Hi all, While debugging an issue in AfterTriggerEndQuery [1], I wondered about the batched FK fast-path code added by commit b7b27eb41a5: should ri_FastPathBatchFlush release pk_slot's buffer pin immediately after index_endscan, instead of letting it live until either the next batch's first probe or end-of-statement teardown?
My initial thought was YES -- a stale pin sitting around looks wrong -- but the more I look at it, the more skeptical I become. Currently, RI_FastPathEntry caches pk_rel, idx_rel, pk_slot, fk_slot, and a per-batch scratch MemoryContext. The cache lives until AfterTriggerEndQuery, when ri_FastPathEndBatch flushes the remaining buffered rows and ri_FastPathTeardown drops everything. Inside ri_FastPathBatchFlush, index_getnext_slot stores each matched PK heap tuple into pk_slot. Because pk_slot is a buffer-heap-tuple slot, that pins the buffer and registers the pin on CurrentResourceOwner. After index_endscan() returns, the slot still holds the last-probed tuple; the pin stays live until either: 1) the next probe's index_getnext_slot clears the slot before storing a new tuple, or 2) end-of-statement teardown drops the slot, releasing the pin. So the pin survives for a while but eventually gets cleaned up. Question is whether the gap is worth tightening with an explicit ExecClearTuple(fpentry->pk_slot) right after index_endscan. Reasons to favor the explicit clear: a) Buffer eviction. A pinned buffer is unevictable. Nothing in ri_triggers.c references pk_slot again before teardown, so the pin is functionally orphaned and slightly reduces the effective working set for the remainder of the statement. b) Pin-lifetime discipline. ExecStoreBufferHeapTuple registers the pin on CurrentResourceOwner; ExecDropSingleTupleTableSlot in teardown asks the current CurrentResourceOwner to forget it. Today they coincide, but the gap between probe and teardown spans arbitrary user code. Clearing the slot right after the probe closes that gap for any future or out-of-tree caller (extensions or forks) that ends up invoking ri_FastPathTeardown from a different ResourceOwner context. c) pinning_backends. VACUUM truncation and a few buffer-related operations slow down or skip work when buffers are pinned. Holding a fast-path pin across unrelated execution burns that budget. But none of those seem strictly concerning for today. However, the fix is not problematic or very complicated and seems harmless to include to address these trivial concerns; it would have two ExecClearTuple() calls: one in ri_FastPathBatchFlush after index_endscan, and a defensive pre-clear in ri_FastPathTeardown. Patch attached. Thoughts? 1] http://postgr.es/m/CAAJ_b95p6-qiVpE2Gpr=bUsNAqTcejD_rPgLnfjx9m=fo3r...@mail.gmail.com -- Regards, Amul Sul EDB: http://www.enterprisedb.com
From b4de97c90c2334b3a31b30f90846f546e0984be7 Mon Sep 17 00:00:00 2001 From: Amul Sul <[email protected]> Date: Wed, 6 May 2026 15:52:12 +0530 Subject: [PATCH 1/2] ri_triggers: release FK fast-path pk_slot's buffer pin promptly. The batched FK fast path (commit b7b27eb41a5) leaves pk_slot holding the last-probed PK heap tuple after ri_FastPathBatchFlush()'s index_endscan(), which keeps a buffer pin alive until either the next probe reuses the slot or ri_FastPathTeardown() runs at end of the after-trigger batch. Holding it that long ties a buffer down for no caching benefit and lets the pin's acquire/release straddle unrelated execution that may run under a different ResourceOwner. Tighten the pin's lifetime in two places: 1. ri_FastPathBatchFlush(): clear pk_slot immediately after index_endscan(), before any work that could ereport. 2. ri_FastPathTeardown(): defensively clear each entry's pk_slot before dropping it, in case a future teardown path runs without completing flush first. --- src/backend/utils/adt/ri_triggers.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index dc89c686394..0f1157287b6 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -2956,6 +2956,15 @@ ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel, UnregisterSnapshot(snapshot); index_endscan(scandesc); + /* + * Clear the pk_slot buffer reference now that the scan is finished. This + * prevents the buffer pin from staying active unnecessarily until the next + * probe or the final teardown. Releasing the pin here ensures it doesn't + * block buffer eviction and keeps the pin's lifetime strictly limited to + * the duration of the probe. + */ + ExecClearTuple(fpentry->pk_slot); + if (violation_index >= 0) { ExecStoreHeapTuple(fpentry->batch[violation_index], fk_slot, false); @@ -4170,6 +4179,22 @@ ri_FastPathTeardown(void) if (ri_fastpath_cache == NULL) return; + /* + * Defensively clear pk_slot before dropping it. While + * ri_FastPathBatchFlush normally clears the slot on success, this ensures + * any remaining buffer pin is released if teardown is reached unexpectedly + * (e.g., during an error path). This avoids relying on + * ExecDropSingleTupleTableSlot to handle the release implicitly. fk_slot + * does not need this treatment as it uses TTSOpsHeapTuple and never pins + * buffers. + */ + hash_seq_init(&status, ri_fastpath_cache); + while ((entry = hash_seq_search(&status)) != NULL) + { + if (entry->pk_slot) + ExecClearTuple(entry->pk_slot); + } + hash_seq_init(&status, ri_fastpath_cache); while ((entry = hash_seq_search(&status)) != NULL) { -- 2.47.1
