Hi, On 2023-11-07 00:57:32 +0000, John Morris wrote: > 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.
IMO cache coherency covers the value a single variable has in different threads / processes. In fact, the only reason there effectively is a guarantee that you're not seeing an outdated unloggedLSN value during shutdown checkpoints, even without the spinlock or full barrier atomic op, is that the LWLockAcquire(), a few lines above this, would prevent both the compiler and CPU from moving the read of unloggedLSN to much earlier. Obviously that lwlock has a different address... If the patch just had done the minimal conversion, it'd already have been committed... Even if there'd be a performance reason to get rid of the memory barrier around reading unloggedLSN in CreateCheckPoint(), I'd do the conversion in a separate commit. Greetings, Andres Freund
