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

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

  1. Convert unloggedLSN to an atomic variable.

  2. Introduce atomic read/write functions with full barrier semantics.