Re: Refactoring of pg_resetwal/t/001_basic.pl - Mailing list pgsql-hackers

From Peter Eisentraut
Subject Re: Refactoring of pg_resetwal/t/001_basic.pl
Date
Msg-id 82687a9d-937e-4c4a-a6ee-3748e32a0005@eisentraut.org
Whole thread Raw
In response to Re: Refactoring of pg_resetwal/t/001_basic.pl  (Svetlana Derevyanko <s.derevyanko@postgrespro.ru>)
List pgsql-hackers
On 26.03.24 12:53, Svetlana Derevyanko wrote:
> Peter Eisentraut писал(а) 2024-03-25 17:10:
> 
>> But MXOFF_SIZE doesn't exist anywhere else.  The actual formula uses
>> sizeof(MultiXactOffset), which isn't obvious from your patch.  So this 
>> just moves the magic constants around by one level.
>>
>> I think if we're going to add more symbols, then it has to be done 
>> consistently in the source code, the documentation, and the tests, not 
>> just one of them.
>>
> 
> Hello!
> Thank you for your reply.
> 
> Attached is the updated version of patch for pg_resetwal test. I added 
> definitions for MXOFF_SIZE and MXID_SIZE constants in multixact.c (and 
> replaced use of sizeof(MultiXactId) and sizeof(MultiXactOffset) 
> accordingly). Also changed multipliers for pg_xact/members/offset on 
> CLOG_XACTS_PER_PAGE/MULTIXACT_MEMBERS_PER_PAGE/MULTIXACT_OFFSETS_PER_PAGE both in src/bin/pg_resetwal/t/001_basic.pl
anddocs, since it seems to me that this makes things more clear.
 
> 
> What do you think?

I don't know.  This patch does not fill me with joy.  These additional 
defines ultimately make the code itself harder to comprehend.

Maybe the original request could be satisfied by adding more comments to 
the test files, like

  @files = get_slru_files('pg_xact');
+# SLRU_PAGES_PER_SEGMENT * BLCKSZ * CLOG_XACTS_PER_BYTE
  $mult = 32 * $blcksz * 4;




pgsql-hackers by date:

Previous
From: Etsuro Fujita
Date:
Subject: Re: ExecAppendAsyncEventWait() in REL_14_STABLE can corrupt PG_exception_stack
Next
From: Etsuro Fujita
Date:
Subject: Re: postgres_fdw: Useless test in pgfdw_exec_cleanup_query_end()