Horton

Thread

Commits

  1. Sanitize some WAL-logging buffer handling in GIN and GiST code

  1. Release and unpin buffers after leaving CRIT section in GIN.

    Kirill Reshke <reshkekirill@gmail.com> — 2026-02-17T09:58:24Z

    Hi
I have stopped $subj while doing unrelated hacking today.
transam/README suggest following for WAL-logging changes:

...

6. END_CRIT_SECTION()

7. Unlock and unpin the buffer(s).


Few places in GIN code does not follow that. v1 fixes it.

-- 
Best regards,
Kirill Reshke

  2. Re: Release and unpin buffers after leaving CRIT section in GIN.

    Michael Paquier <michael@paquier.xyz> — 2026-02-17T10:07:30Z

    On Tue, Feb 17, 2026 at 02:58:24PM +0500, Kirill Reshke wrote:
> Few places in GIN code does not follow that. v1 fixes it.

Yep.  That sounds pretty much right.  Thanks for the report.
--
Michael

  3. Re: Release and unpin buffers after leaving CRIT section in GIN.

    Kirill Reshke <reshkekirill@gmail.com> — 2026-02-17T10:14:45Z

    On Tue, 17 Feb 2026 at 15:07, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Feb 17, 2026 at 02:58:24PM +0500, Kirill Reshke wrote:
> > Few places in GIN code does not follow that. v1 fixes it.
>
> Yep.  That sounds pretty much right.  Thanks for the report.
> --
> Michael

Looks like there are two more instances of this on HEAD: in gistbuild
and log_newpage_range. Is it?

```
reshke@yezzey-cbdb-bench:~/cpg$ git diff
src/backend/access/gist/gistbuild.c
src/backend/access/transam/xloginsert.c
diff --git a/src/backend/access/gist/gistbuild.c
b/src/backend/access/gist/gistbuild.c
index 7f57c787f4c..8e0c083ffa7 100644
--- a/src/backend/access/gist/gistbuild.c
+++ b/src/backend/access/gist/gistbuild.c
@@ -305,10 +305,10 @@ gistbuild(Relation heap, Relation index,
IndexInfo *indexInfo)
                MarkBufferDirty(buffer);
                PageSetLSN(page, GistBuildLSN);

-               UnlockReleaseBuffer(buffer);
-
                END_CRIT_SECTION();

+               UnlockReleaseBuffer(buffer);
+
                /* Scan the table, inserting all the tuples to the index. */
                reltuples = table_index_build_scan(heap, index,
indexInfo, true, true,

            gistBuildCallback,
diff --git a/src/backend/access/transam/xloginsert.c
b/src/backend/access/transam/xloginsert.c
index d3acaa636c3..a9a1678acc9 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -1355,11 +1355,12 @@ log_newpage_range(Relation rel, ForkNumber forknum,
                recptr = XLogInsert(RM_XLOG_ID, XLOG_FPI);

                for (i = 0; i < nbufs; i++)
-               {
                        PageSetLSN(BufferGetPage(bufpack[i]), recptr);
-                       UnlockReleaseBuffer(bufpack[i]);
-               }
+
                END_CRIT_SECTION();
+
+               for (i = 0; i < nbufs; i++)
+                       UnlockReleaseBuffer(bufpack[i]);
        }
 }

```

-- 
Best regards,
Kirill Reshke

  4. Re: Release and unpin buffers after leaving CRIT section in GIN.

    Michael Paquier <michael@paquier.xyz> — 2026-02-19T07:02:23Z

    On Tue, Feb 17, 2026 at 03:14:45PM +0500, Kirill Reshke wrote:
> Looks like there are two more instances of this on HEAD: in gistbuild
> and log_newpage_range. Is it?

Planting an assertion in UnlockReleaseBuffer(), there are two extra
places that can be spotted:
- Most of the places reported by the regression tests seem to come
from gistplacetopage(), with UnlockReleaseBuffer() called for a
rootsplit.  This case is actually OK to me, where the new buffers are
all unlocked with the root page locked.  I have to admit that I am not
the best specialist ever on this one, so I may be missing something.
- ginHeapTupleFastInsert(), with UnlockReleaseBuffer() called before
GinGetPendingListCleanupSize().  This one is not in line, being
registered as buffer in a XLOG_GIN_UPDATE_META_PAGE record.  We could
do better here.

Saying that, the cases of createPostingTree()@gindatapage.c, the three
in ginfast.c, the one in ginutil.c, the two in ginvacuum.c (one with
only unpins), and xloginsert.c are no-brainers to me, so I have
applied these.

The two proposed cases in gininsert.c and gistbuild.c do nothing
related to WAL-logging, though.
--
Michael