Hi hackers, > Yeah, pg_upgrade will briefly start and stop the old server to make > sure all the WAL is replayed, and won't transfer any of the files > over. AFAIK, major-version WAL changes are fine; it was the previous > claim that we could do it in a minor version that I was unsure about.
OK, here is the patchset v53 where I mostly modified the commit messages. It is explicitly said that 0001 modifies the WAL records and why we decided to do it in this patch. Additionally any mention of 64-bit XIDs is removed since it is not guaranteed that the rest of the patches are going to be accepted. 64-bit SLRU page numbering is a valuable change per se. Changing the status of the CF entry to RfC apparently was a bit premature. It looks like the patchset can use a few more rounds of review. In 0002: ``` -#define TransactionIdToCTsPage(xid) \ - ((xid) / (TransactionId) COMMIT_TS_XACTS_PER_PAGE) +static inline int64 +TransactionIdToCTsPageInternal(TransactionId xid, bool lock) +{ + FullTransactionId fxid, + nextXid; + uint32 epoch; + + if (lock) + LWLockAcquire(XidGenLock, LW_SHARED); + + /* make a local copy */ + nextXid = ShmemVariableCache->nextXid; + + if (lock) + LWLockRelease(XidGenLock); + + epoch = EpochFromFullTransactionId(nextXid); + if (xid > XidFromFullTransactionId(nextXid)) + --epoch; + + fxid = FullTransactionIdFromEpochAndXid(epoch, xid); + + return fxid.value / (uint64) COMMIT_TS_XACTS_PER_PAGE; +} ``` I'm pretty confident that shared memory can't be accessed like this, without taking a lock. Although it may work on x64 generally we can get garbage, unless nextXid is accessed atomically and has a corresponding atomic type. On top of that I'm pretty sure TransactionIds can't be compared with the regular comparison operators. All in all, so far I don't understand why this piece of code should be so complicated. The same applies to: ``` -#define TransactionIdToPage(xid) ((xid) / (TransactionId) SUBTRANS_XACTS_PER_PAGE) +static inline int64 +TransactionIdToPageInternal(TransactionId xid, bool lock) ``` ... in subtrans.c Maxim, perhaps you could share with us what your reasoning was here? -- Best regards, Aleksander Alekseev
v53-0002-Utilize-64-bit-SLRU-page-numbers-in-SLRU-callers.patch
Description: Binary data
v53-0001-Use-64-bit-numbering-of-SLRU-pages.patch
Description: Binary data
v53-0003-pg_upgrade-from-32-bit-to-64-bit-SLRU-page-numbe.patch
Description: Binary data