Re: Refactoring of pg_resetwal/t/001_basic.pl

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 and docs, 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;