| From: | Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com> |
|---|---|
| To: | SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Peter Geoghegan <pg(at)bowt(dot)ie> |
| Subject: | Re: Reduce cleanup lock contention on standby replay |
| Date: | 2026-07-17 14:28:52 |
| Message-ID: | CAEze2WgUnbCy3hkWoNQqAs5CMoKVw83QrJn=q9W8nEjXmKVQzw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sat, 11 Jul 2026, 00:40 SATYANARAYANA NARLAPURAM,
<satyanarlapuram(at)gmail(dot)com> wrote:
>
> Hi Hackers,
>
> On standbys, WAL replay of VACUUM-generated XLOG_HEAP2_PRUNE records requires a cleanup lock (exclusive lock + zero pin count) on the target
> buffer. If any backend holds a pin on that buffer, even from a trivial SELECT, replay stalls until max_standby_streaming_delay expires, at which point the query
> is canceled. This can also cause increased replication lag.
>
> Andres and me chatted about this problem offline and he suggested below approaches to improve the current situation:
>
> 1. Shorten the time the queries hold the pins
> 2. Eliminate unnecessary cleanup lock requirements in replay
> 3. Improve diagnosis logging
>
> Additionally, we discussed an approach of redo process performing COW on cleanup lock conflict instead of waiting until the max_standby_streaming_delay under a GUC).
> I am not including it as part of this thread and can start a different one if there is enough interest in that approach.
>
> Attached 3 distinct patches for each of these.
>
> Patch 1: Reduce buffer pin lifetime for unique index point lookups
>
> The most common OLTP pattern is fetching a single row by primary key which holds a heap buffer pin in the scan descriptor (xs_cbuf) until the next fetch or scan end.
> For a point lookup returning exactly one row, this means the pin lives for the entire duration of whatever the executor does with that tuple (evaluating expressions, passing
> through plan nodes, pg_sleep() in a function, nested-loop outer side, etc.).
>
> This patch adds two new ScanOption flags:
> SO_MATERIALIZE_ON_FETCH (IndexScan) - After fetching the heap tuple, materialize it into the slot (heap_copytuple equivalent) and release both the slot's buffer pin and
> the scan descriptor's xs_cbuf pin.
>
> SO_RELEASE_PIN_ON_FETCH (IndexOnlyScan) - Since IOS only visits the heap for visibility checks (output data comes from the index), just release xs_cbuf.
This optimization should only be used when the scan snapshot is an
MVCC snapshot: non-mvcc snapshots can return more than one tuple for
any key value. Please make sure this is Assert()-ed when the flags are
used, and/or checked when the flags are set.
Second, releasing the buffer immediately will impact the performance
of correlated looped join scans. Is that something you've considered?
See also a further comment below.
> Both flags are set only when:
> - The index is unique (rd_index->indisunique)
> - Scan keys cover all key columns (NumScanKeys >= indnkeyatts)
The check you've added in your current patch is insufficient to
guarantee this. The scankey of `col1 > 0 AND col2 > 0` would have
NumScanKeys=2, but doesn't guarantee just one output tuple (indeed, it
could return all the rows in the table, if all values for col1 and
col2 match the given predicate).
You'll have to make sure that every column is mentioned with an
equality predicate for this optimization to be correct (and none of
those equalities can be scalar-array or NULL tests, as Peter also
mentioned below).
> This guarantees at most one heap page visit per scan, so there is no same-page pin reuse benefit to preserve
Please note the comment of heapam_index_fetch_reset, which is used
when IndexRescan() is called to reset the heap lookup part of the
index scan:
/*
* Resets are a no-op.
*
* Deliberately avoid dropping pins now held in xs_cbuf and xs_vmbuffer.
* This saves cycles during certain tight nested loop joins (it can avoid
* repeated pinning and unpinning of the same buffer across rescans).
*/
So, repeated scans of the index do (or at least, can) benefit from
keeping the heap page pinned, even if any one individual index scan
doesn't. Aggressively dropping the page would increase the work for
the next index scan, which could realistically hurt performance.
> Patch 2: Skip cleanup lock on replay for freeze-only prune records
That seems fine with me.
> Patch 3: Add relation info to recovery conflict buffer pin logs
>
> When a recovery conflict on a buffer pin is logged (either resolved or triggering query cancellation), the current message only says "recovery conflict on buffer pin" with timing information.
> It doesn't tell which relation/page is causing the conflict.
>
> This patch adds the buffer tag (spcOid/dbOid/relNumber/blockNum) to the LOG message, so DBAs can identify:
Note that this adds a log message, it does not add to an already
existing log message, and those two things are quite different.
I don't think we should add new logging in LockBufferForCleanup, but
rather extend LogRecoveryConflict() to include this information, and
pass that info from LockBufferForCleanup, like how wait_list is passed
along.
Kind regards,
Matthias van de Meent
Databricks (https://www.databricks.com)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-07-17 14:29:33 | Re: [PATCH] Rename "getdatabaseencoding()" to "pg_database_encoding()", and document |
| Previous Message | Nazir Bilal Yavuz | 2026-07-17 14:22:49 | Re: meson vs. llvm bitcode files |