On Thu, Jul 20, 2023 at 12:25 AM John Morris <john.mor...@crunchydata.com> wrote: > > >> Why don't we just use a barrier when around reading the value? It's not > >> like > >> CreateCheckPoint() is frequent? > > One reason is that a barrier isn’t needed, and adding unnecessary barriers > can also be confusing. > > With respect to the “debug only” comment in the original code, whichever > value is written to the structure during a checkpoint will be reset when > restarting. Nobody will ever see that value. We could just as easily write a > zero.
> > Shutting down is a different story. It isn’t stated, but we require the > unlogged LSN be quiescent before shutting down. This patch doesn’t change > that requirement. > > We could also argue that memory ordering doesn’t matter when filling in a > conventional, unlocked structure. But the fact we have only two cases 1) > checkpoint where the value is ignored, and 2) shutdown where it is quiescent, > makes the additional argument unnecessary. > > Would you be more comfortable if I added comments describing the two cases? > My intent was to be minimalistic, but the original code could use better > explanation. Here, the case for unloggedLSN is that there can exist multiple backends incrementing/writing it (via GetFakeLSNForUnloggedRel), and there can exist one reader at a time in CreateCheckPoint with LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);. Is incrementing unloggedLSN atomically (with an implied memory barrier from pg_atomic_fetch_add_u64) helping out synchronize multiple writers and readers? With a spinlock, writers-readers synchronization is guaranteed. With an atomic variable, it is guaranteed that the readers don't see a torn-value, but no synchronization is provided. If the above understanding is right, what happens if readers see an old unloggedLSN value - reader here stores the old unloggedLSN value to control file and the server restarts (clean exit). So, the old value is loaded back to unloggedLSN upon restart and the callers of GetFakeLSNForUnloggedRel() will see an old/repeated value too. Is it a problem? -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
