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.

Attachment: atomic_lsn_v5.patch
Description: atomic_lsn_v5.patch

Reply via email to