WIP: Add per-process shadow variables for buffer pool synchronization#9
Draft
palak-chaturvedi wants to merge 2 commits into
Draft
Conversation
4006885 to
82e5693
Compare
Introduce LocalCurrentNBuffers and LocalActiveNBuffers as per-process shadow copies of ShmemCtrl->currentNBuffers and StrategyControl->activeNBuffers respectively. This allows backends to use local copies during normal operation, reducing contention on shared memory, and enables safe mid-operation updates when resize barriers are processed. Key changes: - Add LocalCurrentNBuffers and LocalActiveNBuffers globals, initialized from shared state at backend startup. - Replace direct NBuffers usage with LocalCurrentNBuffers/LocalActiveNBuffers in buffer manager, freelist, pg_buffercache, autoprewarm, and access methods (hash, heap, tableam, xlog). - Coordinator in pg_resize_shared_buffers() updates its own shadows at each phase and asserts consistency at the end. - Each ProcessBarrier*() handler updates the local shadows and logs the transition (old -> new) at DEBUG1 level. - Add PROCSIGNAL_BARRIER_SHBUF_SHMEM_RESIZE_SHRINK barrier so backends update LocalCurrentNBuffers after shared structures are resized during shrink. - Add StrategyGetActiveNBuffers() accessor for freelist.c-private state.
82e5693 to
d1fc645
Compare
- Revert ClockSweepTick() and StrategySyncStart() to use shared atomic reads of activeNBuffers instead of LocalActiveNBuffers. All backends in the CAS loop on nextVictimBuffer must use the same modulus to avoid inconsistent wrap-around during the barrier propagation window. LocalActiveNBuffers is still used for the purely-local trycounter in StrategyGetBuffer() and the bounds check in GetBufferFromRing(). - Fix expand path ordering: update shared state (StrategyReset and ShmemCtrl->currentNBuffers) before coordinator's local shadows, so there is no window where the local view is ahead of shared state. - Fix trailing whitespace in ProcessBarrierShmemShrink(). - Fix comment alignment for PROCSIGNAL_BARRIER_SHBUF_SHMEM_RESIZE_SHRINK in procsignal.h. - Add detailed comment in GetBufferFromRing() explaining why LocalActiveNBuffers is safe for the ring buffer bounds check. - Update ClockSweepTick() comment to explain why the shared atomic is used instead of LocalActiveNBuffers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduce LocalCurrentNBuffers and LocalActiveNBuffers as per-process shadow copies of ShmemCtrl->currentNBuffers and
StrategyControl->activeNBuffers respectively. This allows backends to use local copies during normal operation, reducing contention on shared memory, and enables safe mid-operation updates when resize barriers are processed.
Key changes: