Re: Atomic ops for unlogged LSN
John Morris <john.morris@crunchydata.com>
From: John Morris <john.morris@crunchydata.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Andres Freund <andres@anarazel.de>, Stephen Frost <sfrost@snowman.net>, Michael Paquier <michael@paquier.xyz>, Robert Haas <robertmhaas@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2023-11-07T00:57:32Z
Lists: pgsql-hackers
Attachments
- atomic_lsn_v5.patch (application/octet-stream) patch v5
I incorporated your suggestions and added a few more. The changes are mainly related to catching potential errors if some basic assumptions aren’t met. There are basically 3 assumptions. Stating them as conditions we want to avoid. * We should not get an unlogged LSN before reading the control file. * We should not get an unlogged LSN when shutting down. * The unlogged LSN written out during a checkpoint shouldn’t be used. Your suggestion addressed the first problem, and it took only minor changes to address the other two. The essential idea is, we set a value of zero in each of the 3 situations. Then we throw an Assert() If somebody tries to allocate an unlogged LSN with the value zero. I found the comment about cache coherency a bit confusing. We are dealing with a single address, so there should be no memory ordering or coherency issues. (Did I misunderstand?) I see it more as a race condition. Rather than merely explaining why it shouldn’t happen, the new version verifies the assumptions and throws an Assert() if something goes wrong.
Commits
-
Convert unloggedLSN to an atomic variable.
- 963d3072af21 17.0 landed
-
Introduce atomic read/write functions with full barrier semantics.
- bd5132db558b 17.0 cited