Rework ginScanToDelete() to pass Buffers instead of BlockNumbers.

Previously, ginScanToDelete() and ginDeletePage() passed BlockNumbers and
re-read pages that were already pinned and locked during the tree walk.  The
caller ginVacuumPostingTree()) held a cleanup-locked root buffer, yet
ginScanToDelete() re-read it by block number with special-case code to skip
re-locking.

At first, this commit gives both functions more appropriate names,
ginScanPostingTreeToDelete() and ginDeletePostingPage(), indicating they deal
with posting trees/pages.  This is more descriptive and similar to the way we
name other GIN functions, for instance, ginVacuumPostingTree() and
ginVacuumPostingTreeLeaves().

Then rework both functions to pass Buffers directly.  DataPageDeleteStack now
carries buffer, myoff (downlink offset in parent), and isRoot per level,
so ginScanPostingTreeToDelete() takes only GinVacuumState and
DataPageDeleteStack pointers.  Also, ginDeletePostingPage() receives the three
Buffers directly, and no longer reads or releases them itself.  The caller
reads and locks child pages before recursing, and manages buffer lifecycle
afterward.

This eliminates the confusing isRoot special cases in buffer management,
including the apparent (but unreachable) double release of the root
buffer identified by Andres Freund.

Add comments explaining the locking protocol and the DataPageDeleteStack
structure.

Reported-by: Andres Freund <[email protected]>
Discussion: 
https://postgr.es/m/utrlxij43fbguzw4kldte2spc4btoldizutcqyrfakqnbrp3ir@ph3sphpj4asz
Reviewed-by: Pavel Borisov <[email protected]>
Reviewed-by: Xuneng Zhou <[email protected]>
Reviewed-by: Jinbinge <[email protected]>

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/fa6f2f624c01bba6e4c7b6920256afcc1ee17eb2

Modified Files
--------------
src/backend/access/gin/README      |   8 +-
src/backend/access/gin/ginvacuum.c | 193 ++++++++++++++++++++++---------------
2 files changed, 119 insertions(+), 82 deletions(-)

Reply via email to