So I've been wondering about this block at the bottom of XLogWrite: /* * Make sure that the shared 'request' values do not fall behind the * 'result' values. This is not absolutely essential, but it saves some * code in a couple of places. */ { SpinLockAcquire(&XLogCtl->info_lck); if (XLogCtl->LogwrtRqst.Write < LogwrtResult.Write) XLogCtl->LogwrtRqst.Write = LogwrtResult.Write; if (XLogCtl->LogwrtRqst.Flush < LogwrtResult.Flush) XLogCtl->LogwrtRqst.Flush = LogwrtResult.Flush; SpinLockRelease(&XLogCtl->info_lck); }
I just noticed that my 0001 makes the comment a lie: it is now quite possible that 'result' is advanced beyond 'request'. Before the patch that never happened because they were both advanced in the region locked by the spinlock. I think we could still maintain this promise if we just moved this entire block before the first pg_atomic_monotonic_advance_u64 setting XLogCtl->LogwrtResult.Write. Or we could halve the whole block, and put one acquire/test/set/release stanza before each monotonic increase of the corresponding variable. However, I wonder if this is still necessary. This code was added in 4d14fe0048c (March 2001) and while everything else was quite different back then, this hasn't changed at all. I can't quite figure out what are those "couple of places" that would need additional code if this block is just removed. I tried running the tests (including wal_consistency_checking), and nothing breaks. Reading the code surrounding the other accesses of XLogCtl->LogwrtRqst, there's nothing that looks to me like it depends on these values not lagging behind LogwrtResult. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "If you have nothing to say, maybe you need just the right tool to help you not say it." (New York Times, about Microsoft PowerPoint)