On Fri, 2023-11-03 at 20:23 +0530, Bharath Rupireddy wrote: > > > OldestInitializedPage is introduced in v14-0001 patch. Please have a > look.
I don't see why that's necessary if we move to the algorithm I suggested below that doesn't require a lock. > > > Okay. Current patch doesn't support this [partial hit of newer pages] > case. OK, no need to support it until you see a reason. > > > > > > I think it needs something like: > > > > pg_atomic_write_u64(&XLogCtl->xlblocks[nextidx], > > InvalidXLogRecPtr); > > pg_write_barrier(); > > > > before the MemSet. > > I think it works. First, xlblocks needs to be turned to an array of > 64-bit atomics and then the above change. Does anyone see a reason we shouldn't move to atomics here? > > pg_write_barrier(); > > *((volatile XLogRecPtr *) &XLogCtl->xlblocks[nextidx]) = > NewPageEndPtr; I am confused why the "volatile" is required on that line (not from your patch). I sent a separate message about that: https://www.postgresql.org/message-id/784f72ac09061fe5eaa5335cc347340c367c73ac.ca...@j-davis.com > I think the 3 things that helps read from WAL buffers without > WALBufMappingLock are: 1) couple of the read barriers in > XLogReadFromBuffers, 2) atomically initializing xlblocks[idx] to > InvalidXLogRecPtr plus a write barrier in AdvanceXLInsertBuffer(), 3) > the following sanity check to see if the read page is valid in > XLogReadFromBuffers(). If it sounds sensible, I'll work towards > coding > it up. Thoughts? I like it. I think it will ultimately be a fairly simple loop. And by moving to atomics, we won't need the delicate comment in GetXLogBuffer(). Regards, Jeff Davis
