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?
Best regards,
Svetlana Derevyanko.
From a1bbaddbc00004e97e95ed15b51e48386be5fb4b Mon Sep 17 00:00:00 2001
From: Svetlana Derevyanko <s.derevya...@postgrespro.ru>
Date: Tue, 26 Mar 2024 13:09:43 +0300
Subject: [PATCH v2] Refactor pg_resetwal/t/001_basic.pl
Use constants instead of magic numbers.
Added MXID_SIZE and MXOFF_SIZE constants.
Changed desciptions for pg_resetwal options in docs.
---
doc/src/sgml/ref/pg_resetwal.sgml | 8 ++++----
src/backend/access/transam/multixact.c | 15 +++++++++------
src/bin/pg_resetwal/t/001_basic.pl | 18 +++++++++++++++---
3 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/ref/pg_resetwal.sgml b/doc/src/sgml/ref/pg_resetwal.sgml
index cf9c7e70f2..d8de5e2e29 100644
--- a/doc/src/sgml/ref/pg_resetwal.sgml
+++ b/doc/src/sgml/ref/pg_resetwal.sgml
@@ -274,7 +274,7 @@ PostgreSQL documentation
names are in hexadecimal, so the easiest way to do this is to specify
the option value in hexadecimal and append four zeroes.
</para>
- <!-- 65536 = SLRU_PAGES_PER_SEGMENT * BLCKSZ / sizeof(MultiXactOffset) -->
+ <!-- 65536 = SLRU_PAGES_PER_SEGMENT * MULTIXACT_OFFSETS_PER_PAGE -->
</listitem>
</varlistentry>
@@ -309,7 +309,7 @@ PostgreSQL documentation
The file names are in hexadecimal. There is no simple recipe such as
the ones for other options of appending zeroes.
</para>
- <!-- 52352 = SLRU_PAGES_PER_SEGMENT * floor(BLCKSZ/20) * 4; see multixact.c -->
+ <!-- 52352 = SLRU_PAGES_PER_SEGMENT * MULTIXACT_MEMBERS_PER_PAGE; see multixact.c -->
</listitem>
</varlistentry>
@@ -358,7 +358,7 @@ PostgreSQL documentation
in <filename>pg_xact</filename>, <literal>-u 0x700000</literal> will work (five
trailing zeroes provide the proper multiplier).
</para>
- <!-- 1048576 = SLRU_PAGES_PER_SEGMENT * BLCKSZ * CLOG_XACTS_PER_BYTE -->
+ <!-- 1048576 = SLRU_PAGES_PER_SEGMENT * CLOG_XACTS_PER_PAGE -->
</listitem>
</varlistentry>
@@ -380,7 +380,7 @@ PostgreSQL documentation
in <filename>pg_xact</filename>, <literal>-x 0x1200000</literal> will work (five
trailing zeroes provide the proper multiplier).
</para>
- <!-- 1048576 = SLRU_PAGES_PER_SEGMENT * BLCKSZ * CLOG_XACTS_PER_BYTE -->
+ <!-- 1048576 = SLRU_PAGES_PER_SEGMENT * CLOG_XACTS_PER_PAGE -->
</listitem>
</varlistentry>
</variablelist>
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 83b578dced..6bda000120 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -104,8 +104,11 @@
* MultiXactOffsetPagePrecedes).
*/
+#define MXOFF_SIZE sizeof(MultiXactOffset)
+#define MXID_SIZE sizeof(MultiXactId)
+
/* We need four bytes per offset */
-#define MULTIXACT_OFFSETS_PER_PAGE (BLCKSZ / sizeof(MultiXactOffset))
+#define MULTIXACT_OFFSETS_PER_PAGE (BLCKSZ / MXOFF_SIZE)
#define MultiXactIdToOffsetPage(xid) \
((xid) / (MultiXactOffset) MULTIXACT_OFFSETS_PER_PAGE)
@@ -1765,7 +1768,7 @@ AtPrepare_MultiXact(void)
if (MultiXactIdIsValid(myOldestMember))
RegisterTwoPhaseRecord(TWOPHASE_RM_MULTIXACT_ID, 0,
- &myOldestMember, sizeof(MultiXactId));
+ &myOldestMember, MXID_SIZE);
}
/*
@@ -1832,7 +1835,7 @@ multixact_twophase_recover(TransactionId xid, uint16 info,
* Get the oldest member XID from the state file record, and set it in the
* OldestMemberMXactId slot reserved for this prepared transaction.
*/
- Assert(len == sizeof(MultiXactId));
+ Assert(len == MXID_SIZE);
oldestMember = *((MultiXactId *) recdata);
OldestMemberMXactId[dummyProcNumber] = oldestMember;
@@ -1848,7 +1851,7 @@ multixact_twophase_postcommit(TransactionId xid, uint16 info,
{
ProcNumber dummyProcNumber = TwoPhaseGetDummyProcNumber(xid, true);
- Assert(len == sizeof(MultiXactId));
+ Assert(len == MXID_SIZE);
OldestMemberMXactId[dummyProcNumber] = InvalidMultiXactId;
}
@@ -1877,7 +1880,7 @@ MultiXactShmemSize(void)
/* We need 2*MaxOldestSlot perBackendXactIds[] entries */
#define SHARED_MULTIXACT_STATE_SIZE \
add_size(offsetof(MultiXactStateData, perBackendXactIds), \
- mul_size(sizeof(MultiXactId) * 2, MaxOldestSlot))
+ mul_size(MXID_SIZE * 2, MaxOldestSlot))
size = SHARED_MULTIXACT_STATE_SIZE;
size = add_size(size, SimpleLruShmemSize(multixact_offset_buffers, 0));
@@ -2146,7 +2149,7 @@ TrimMultiXact(void)
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
offptr += entryno;
- MemSet(offptr, 0, BLCKSZ - (entryno * sizeof(MultiXactOffset)));
+ MemSet(offptr, 0, BLCKSZ - (entryno * MXOFF_SIZE));
MultiXactOffsetCtl->shared->page_dirty[slotno] = true;
LWLockRelease(lock);
diff --git a/src/bin/pg_resetwal/t/001_basic.pl b/src/bin/pg_resetwal/t/001_basic.pl
index 9829e48106..19fb6dc6a2 100644
--- a/src/bin/pg_resetwal/t/001_basic.pl
+++ b/src/bin/pg_resetwal/t/001_basic.pl
@@ -205,8 +205,20 @@ push @cmd,
'-c',
sprintf("%d,%d", hex($files[0]) == 0 ? 3 : hex($files[0]), hex($files[-1]));
+use constant SLRU_PAGES_PER_SEGMENT => 32;
+use constant MXOFF_SIZE => 4;
+use constant MXID_SIZE => 4;
+use constant MULTIXACT_MEMBERS_PER_MEMBERGROUP => 4;
+use constant MULTIXACT_FLAGBYTES_PER_GROUP => 4;
+use constant MULTIXACT_MEMBERGROUP_SIZE => MXID_SIZE * MULTIXACT_MEMBERS_PER_MEMBERGROUP + MULTIXACT_FLAGBYTES_PER_GROUP;
+use constant MULTIXACT_MEMBERGROUPS_PER_PAGE => int($blcksz / MULTIXACT_MEMBERGROUP_SIZE);
+use constant MULTIXACT_MEMBERS_PER_PAGE => MULTIXACT_MEMBERGROUPS_PER_PAGE * MULTIXACT_MEMBERS_PER_MEMBERGROUP;
+use constant MULTIXACT_OFFSETS_PER_PAGE => int($blcksz / MXOFF_SIZE);
+use constant CLOG_XACTS_PER_BYTE => 4;
+use constant CLOG_XACTS_PER_PAGE => $blcksz * CLOG_XACTS_PER_BYTE;
+
@files = get_slru_files('pg_multixact/offsets');
-$mult = 32 * $blcksz / 4;
+$mult = SLRU_PAGES_PER_SEGMENT * MULTIXACT_OFFSETS_PER_PAGE;
# -m argument is "new,old"
push @cmd, '-m',
sprintf("%d,%d",
@@ -214,11 +226,11 @@ push @cmd, '-m',
hex($files[0]) == 0 ? 1 : hex($files[0] * $mult));
@files = get_slru_files('pg_multixact/members');
-$mult = 32 * int($blcksz / 20) * 4;
+$mult = SLRU_PAGES_PER_SEGMENT * MULTIXACT_MEMBERS_PER_PAGE;
push @cmd, '-O', (hex($files[-1]) + 1) * $mult;
@files = get_slru_files('pg_xact');
-$mult = 32 * $blcksz * 4;
+$mult = SLRU_PAGES_PER_SEGMENT * CLOG_XACTS_PER_PAGE;
push @cmd,
'-u', (hex($files[0]) == 0 ? 3 : hex($files[0]) * $mult),
'-x', ((hex($files[-1]) + 1) * $mult);
--
2.34.1
