Horton

Re: XID formatting and SLRU refactorings (was: Add 64-bit XIDs into PostgreSQL 15)

Aleksander Alekseev <aleksander@timescale.com>

View thread View on postgr.es
From: Aleksander Alekseev <aleksander@timescale.com>
To: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Cc: Maxim Orlov <orlovmg@gmail.com>, Jacob Champion <jchampion@timescale.com>, Japin Li <japinli@hotmail.com>, Andres Freund <andres@anarazel.de>, Peter Geoghegan <pg@bowt.ie>, Michael Paquier <michael@paquier.xyz>, Ian Lawrence Barwick <barwick@gmail.com>, Hamid Akhtar <hamid.akhtar@percona.com>, Pavel Borisov <pashkin.elfe@gmail.com>, Justin Pryzby <pryzby@telsasoft.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Alexander Korotkov <aekorotkov@gmail.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Ilya Anfimov <ilan@tzirechnoy.com>
Date: 2023-02-20T15:30:31Z
Lists: pgsql-hackers

Attachments

Hi,

> 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:
>
> [...]
>
> Maxim, perhaps you could share with us what your reasoning was here?

I played with the patch a bit and managed to figure out what you tried
to accomplish. Unfortunately generally you can't derive a
FullTransactionId from a TransactionId, and you can't access
ShmemVariableCache fields without taking a lock unless during the
startup when there are no concurrent processes.

I don't think this patch should do anything but change the SLRU
indexing from 32-bit to 64-bit one. Trying to address the wraparounds
would be nice but I'm afraid we are not quite there yet.

Also I found strage little changes that seemed to be unrelated to the
patch. I believe they ended up here by accident (used to be a part of
64-bit XIDs patchset) and removed them.

PFA the cleaned up version of the patch. I noticed that splitting it
into parts doesn't help much with the review or testing, nor seems it
likely that the patches are going to be merged separately one by one.
For these reasons I merged everything into a single patch.

The convert_pg_xact_segments() function is still obviously
overengineered. As I understand, all it has to do is simply renaming
pg_xact/XXXX to pg_xact/00000000XXXX. Unfortunately I used up all the
mana for today and have to take a long rest in order to continue.

-- 
Best regards,
Aleksander Alekseev

Commits

  1. Fix more holes with SLRU code in need of int64 for segment numbers

  2. Use more consistently int64 for page numbers in SLRU-related code

  3. Improve comments in slru.{c,h} about segment name format

  4. Reorder actions in ProcArrayApplyRecoveryInfo()

  5. Fix typo in 5a1dfde8334b

  6. Fix warning due non-standard inline declaration in 4ed8f0913bfdb5f355

  7. Add SLRU tests for 64-bit page case

  8. Make use FullTransactionId in 2PC filenames

  9. Use larger segment file names for pg_notify

  10. Index SLRUs by 64-bit integers rather than by 32-bit integers

  11. Keep track of transaction commit timestamps

  12. Handle 5-char filenames in SlruScanDirectory