Hi,
On 2023-04-07 23:04:08 -0700, Andres Freund wrote:
> There were some failures in CI (e.g. [1] (and perhaps also bf, didn't yet
> check), about "no unpinned buffers available". I was worried for a moment
> that this could actually be relation to the bulk extension patch.
>
> But it looks like it's older - and not caused by direct_io support (except by
> way of the test existing). I reproduced the issue locally by setting s_b even
> lower, to 16 and made the ERROR a PANIC.
>
> [backtrace]
>
> If you look at log_newpage_range(), it's not surprising that we get this error
> - it pins up to 32 buffers at once.
>
> Afaics log_newpage_range() originates in 9155580fd5fc, but this caller is from
> c6b92041d385.
>
>
> It doesn't really seem OK to me to unconditionally pin 32 buffers. For the
> relation extension patch I introduced LimitAdditionalPins() to deal with this
> concern. Perhaps it needs to be exposed and log_newpage_buffers() should use
> it?
>
>
> Do we care about fixing this in the backbranches? Probably not, given there
> haven't been user complaints?
Here's a quick prototype of this approach. If we expose LimitAdditionalPins(),
we'd probably want to add "Buffer" to the name, and pass it a relation, so
that it can hand off LimitAdditionalLocalPins() when appropriate? The callsite
in question doesn't need it, but ...
Without the limiting of pins the modified 004_io_direct.pl fails 100% of the
time for me.
Presumably the reason it fails occasionally with 256kB of shared buffers
(i.e. NBuffers=32) is that autovacuum or checkpointer briefly pins a single
buffer. As log_newpage_range() thinks it can just pin 32 buffers
unconditionally, it fails in that case.
Greetings,
Andres Freund
diff --git i/src/include/storage/bufmgr.h w/src/include/storage/bufmgr.h
index 6ab00daa2ea..e5788309c86 100644
--- i/src/include/storage/bufmgr.h
+++ w/src/include/storage/bufmgr.h
@@ -223,6 +223,7 @@ extern void DropRelationBuffers(struct SMgrRelationData *smgr_reln,
extern void DropRelationsAllBuffers(struct SMgrRelationData **smgr_reln,
int nlocators);
extern void DropDatabaseBuffers(Oid dbid);
+extern void LimitAdditionalPins(uint32 *additional_pins);
#define RelationGetNumberOfBlocks(reln) \
RelationGetNumberOfBlocksInFork(reln, MAIN_FORKNUM)
diff --git i/src/backend/access/transam/xloginsert.c w/src/backend/access/transam/xloginsert.c
index e2a5a3d13ba..2189fc9f71f 100644
--- i/src/backend/access/transam/xloginsert.c
+++ w/src/backend/access/transam/xloginsert.c
@@ -1268,8 +1268,8 @@ log_newpage_range(Relation rel, ForkNumber forknum,
/*
* Iterate over all the pages in the range. They are collected into
- * batches of XLR_MAX_BLOCK_ID pages, and a single WAL-record is written
- * for each batch.
+ * batches of up to XLR_MAX_BLOCK_ID pages, and a single WAL-record is
+ * written for each batch.
*/
XLogEnsureRecordSpace(XLR_MAX_BLOCK_ID - 1, 0);
@@ -1278,14 +1278,18 @@ log_newpage_range(Relation rel, ForkNumber forknum,
{
Buffer bufpack[XLR_MAX_BLOCK_ID];
XLogRecPtr recptr;
- int nbufs;
+ uint32 limit = XLR_MAX_BLOCK_ID;
+ int nbufs;
int i;
CHECK_FOR_INTERRUPTS();
+ /* avoid running out pinnable buffers */
+ LimitAdditionalPins(&limit);
+
/* Collect a batch of blocks. */
nbufs = 0;
- while (nbufs < XLR_MAX_BLOCK_ID && blkno < endblk)
+ while (nbufs < limit && blkno < endblk)
{
Buffer buf = ReadBufferExtended(rel, forknum, blkno,
RBM_NORMAL, NULL);
diff --git i/src/backend/storage/buffer/bufmgr.c w/src/backend/storage/buffer/bufmgr.c
index 7778dde3e57..31c75d6240e 100644
--- i/src/backend/storage/buffer/bufmgr.c
+++ w/src/backend/storage/buffer/bufmgr.c
@@ -1742,7 +1742,7 @@ again:
* pessimistic, but outside of toy-sized shared_buffers it should allow
* sufficient pins.
*/
-static void
+void
LimitAdditionalPins(uint32 *additional_pins)
{
uint32 max_backends;
diff --git i/src/test/modules/test_misc/t/004_io_direct.pl w/src/test/modules/test_misc/t/004_io_direct.pl
index f5bf0b11e4e..5791c2ab7bd 100644
--- i/src/test/modules/test_misc/t/004_io_direct.pl
+++ w/src/test/modules/test_misc/t/004_io_direct.pl
@@ -22,7 +22,7 @@ $node->init;
$node->append_conf(
'postgresql.conf', qq{
io_direct = 'data,wal,wal_init'
-shared_buffers = '256kB' # tiny to force I/O
+shared_buffers = '16' # tiny to force I/O
});
$node->start;
