Make local buffers pin limit more conservative

GetLocalPinLimit() and GetAdditionalLocalPinLimit(), currently in use
only by the read stream, previously allowed a backend to pin all
num_temp_buffers local buffers. This meant that the read stream could
use every available local buffer for read-ahead, leaving none for other
concurrent pin-holders.

In 18, this was reported as a sequential scan of a temporary table
failing with "no empty local buffer available": with
effective_io_concurrency >= 64 and default io_combine_limit and
temp_buffers, the scan's read stream budget,
(effective_io_concurrency + 1) * io_combine_limit, meets or exceeds
num_temp_buffers, so look-ahead could pin the entire local buffer pool.
Any additional pin request made while returning tuples -- such as
fetching TOASTed values from the table's (also temporary) TOAST table --
then found no unpinned buffer to evict.

Cap the local pin limit to num_temp_buffers / 4, providing some
headroom. This doesn't guarantee that all needed pins will be available
-- for example, a backend can still open more cursors than there are
buffers -- but it makes it less likely that read-ahead will exhaust the
pool.

This is a backpatch of commit da6874635db, which was applied to master
only during the 19 development cycle, where the issue surfaced as a
regression test failure after on-access pruning began setting the
visibility map. Reports of the failure above on 18 prompted
backpatching it now.

No backpatch to 17, even though its read stream also allows a single
stream to pin the whole pool in principle. There, sequential scans pass
READ_STREAM_SEQUENTIAL, which disables fadvise-based look-ahead, so the
look-ahead distance cannot grow past io_combine_limit (at most 32
buffers, versus a minimum temp_buffers of 100) and the reported failure
is unreachable. The remaining theoretical paths require adversarial
settings and have never been reported. Moreover, 17 predates these
functions; the fix would have to be reimplemented in
LimitAdditionalLocalPins(), making the change not a simple cherry-pick.

Reported-by: Induja Sreekanthan <[email protected]>
Reported-by: Eduard Stepanov <[email protected]>
Reported-by: Feike Steenbergen <[email protected]>
Reported-by: Alexander Lakhin <[email protected]>
Reviewed-by: Xuneng Zhou <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/97529f5a-ec10-46b1-ab50-4653126c6889%40gmail.com
Discussion: 
https://postgr.es/m/flat/CAFMO8-rYPSJbXsDdWDzDdpNi-fQ%2B6bKvgbXwE%2BR%3DsGko4epq0Q%40mail.gmail.com
Backpatch-through: 18

Branch
------
REL_18_STABLE

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

Modified Files
--------------
src/backend/storage/buffer/localbuf.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)

Reply via email to