agora inbox for [email protected]
help / color / mirror / Atom feed[PATCH 09/10] Add zstd compression levels
8+ messages / 2 participants
[nested] [flat]
* [PATCH 09/10] Add zstd compression levels
@ 2021-03-14 22:12 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Justin Pryzby @ 2021-03-14 22:12 UTC (permalink / raw)
---
src/backend/access/transam/xlog.c | 6 +++++-
src/backend/access/transam/xloginsert.c | 15 +++++++++++++--
src/backend/access/transam/xlogreader.c | 12 ++++++++++++
src/backend/utils/misc/guc.c | 2 +-
src/include/access/xlog_internal.h | 4 ++++
5 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 92023de9f5..b14c7c5929 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -99,7 +99,7 @@ bool EnableHotStandby = false;
bool fullPageWrites = true;
bool wal_log_hints = false;
bool wal_compression = false;
-int wal_compression_method = WAL_COMPRESSION_ZSTD;
+int wal_compression_method = WAL_COMPRESSION_ZSTD_FAST_10;
char *wal_consistency_checking_string = NULL;
bool *wal_consistency_checking = NULL;
bool wal_init_zero = true;
@@ -192,6 +192,10 @@ const struct config_enum_entry wal_compression_options[] = {
#endif
#ifdef USE_ZSTD
{"zstd", WAL_COMPRESSION_ZSTD, false},
+ {"zstd-1", WAL_COMPRESSION_ZSTD_1, false},
+ {"zstd-fast-10", WAL_COMPRESSION_ZSTD_FAST_10, false},
+ {"zstd-fast-20", WAL_COMPRESSION_ZSTD_FAST_20, false},
+ {"zstd-fast-40", WAL_COMPRESSION_ZSTD_FAST_40, false},
#endif
{NULL, 0, false}
};
diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c
index 4591e476c6..4f11f96373 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -915,11 +915,22 @@ XLogCompressBackupBlock(char *page, uint16 hole_offset, uint16 hole_length,
#ifdef USE_ZSTD
case WAL_COMPRESSION_ZSTD:
- len = ZSTD_compress(dest, COMPRESS_BUFSIZE, source, orig_len,
- ZSTD_CLEVEL_DEFAULT);
+ case WAL_COMPRESSION_ZSTD_1:
+ case WAL_COMPRESSION_ZSTD_FAST_10:
+ case WAL_COMPRESSION_ZSTD_FAST_20:
+ case WAL_COMPRESSION_ZSTD_FAST_40:
+ {
+ int level = compression == WAL_COMPRESSION_ZSTD_1 ? 1 :
+ compression == WAL_COMPRESSION_ZSTD_FAST_10 ? -10 :
+ compression == WAL_COMPRESSION_ZSTD_FAST_20 ? -20 :
+ compression == WAL_COMPRESSION_ZSTD_FAST_40 ? -40 :
+ ZSTD_CLEVEL_DEFAULT;
+
+ len = ZSTD_compress(dest, COMPRESS_BUFSIZE, source, orig_len, level);
if (ZSTD_isError(len))
len = -1;
break;
+ }
#endif
default:
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 0f9d522087..0de61d3073 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1554,6 +1554,10 @@ struct walcompression walmethods[] = {
{"zlib", WAL_COMPRESSION_ZLIB},
{"lz4", WAL_COMPRESSION_LZ4},
{"zstd", WAL_COMPRESSION_ZSTD},
+ {"zstd-1", WAL_COMPRESSION_ZSTD},
+ {"zstd-fast-10",WAL_COMPRESSION_ZSTD},
+ {"zstd-fast-20",WAL_COMPRESSION_ZSTD},
+ {"zstd-fast-40",WAL_COMPRESSION_ZSTD},
};
/*
@@ -1628,6 +1632,14 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page)
#ifdef USE_ZSTD
case WAL_COMPRESSION_ZSTD:
+ /*
+ * There aren't actually written into the header - decompression is the
+ * same.
+ * case WAL_COMPRESSION_ZSTD_1:
+ * case WAL_COMPRESSION_ZSTD_FAST_10:
+ * case WAL_COMPRESSION_ZSTD_FAST_20:
+ * case WAL_COMPRESSION_ZSTD_FAST_40:
+ */
decomp_result = ZSTD_decompress(tmp.data, BLCKSZ-bkpb->hole_length,
ptr, bkpb->bimg_len);
// XXX: ZSTD_getErrorName
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 8031e027aa..667fc4c0c1 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4728,7 +4728,7 @@ static struct config_enum ConfigureNamesEnum[] =
NULL
},
&wal_compression_method,
- WAL_COMPRESSION_ZSTD, wal_compression_options,
+ WAL_COMPRESSION_ZSTD_FAST_10, wal_compression_options,
NULL, NULL, NULL
},
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index fa8146645d..a435b1a654 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -338,6 +338,10 @@ typedef enum WalCompression
WAL_COMPRESSION_ZLIB,
WAL_COMPRESSION_LZ4,
WAL_COMPRESSION_ZSTD,
+ WAL_COMPRESSION_ZSTD_1,
+ WAL_COMPRESSION_ZSTD_FAST_10, /* level = -10 */
+ WAL_COMPRESSION_ZSTD_FAST_20, /* level = -20 */
+ WAL_COMPRESSION_ZSTD_FAST_40, /* level = -40 */
} WalCompression;
extern const char *wal_compression_name(WalCompression compression);
--
2.17.0
--jozmn01XJZjDjM3N--
^ permalink raw reply [nested|flat] 8+ messages in thread
* [PATCH v4 08/15] bufmgr: Support multiple in-progress IOs by using resowner
@ 2022-10-24 23:44 Andres Freund <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Andres Freund @ 2022-10-24 23:44 UTC (permalink / raw)
---
src/include/storage/bufmgr.h | 2 +-
src/include/utils/resowner_private.h | 5 ++
src/backend/access/transam/xact.c | 4 +-
src/backend/postmaster/autovacuum.c | 1 -
src/backend/postmaster/bgwriter.c | 1 -
src/backend/postmaster/checkpointer.c | 1 -
src/backend/postmaster/walwriter.c | 1 -
src/backend/storage/buffer/bufmgr.c | 86 ++++++++++++---------------
src/backend/utils/resowner/resowner.c | 60 +++++++++++++++++++
9 files changed, 105 insertions(+), 56 deletions(-)
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index 973547a8baf..c568336b2b3 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -181,7 +181,7 @@ extern bool ConditionalLockBufferForCleanup(Buffer buffer);
extern bool IsBufferCleanupOK(Buffer buffer);
extern bool HoldingBufferPinThatDelaysRecovery(void);
-extern void AbortBufferIO(void);
+extern void AbortBufferIO(Buffer buffer);
extern void BufmgrCommit(void);
extern bool BgBufferSync(struct WritebackContext *wb_context);
diff --git a/src/include/utils/resowner_private.h b/src/include/utils/resowner_private.h
index 1b1f3181b54..ae58438ec76 100644
--- a/src/include/utils/resowner_private.h
+++ b/src/include/utils/resowner_private.h
@@ -30,6 +30,11 @@ extern void ResourceOwnerEnlargeBuffers(ResourceOwner owner);
extern void ResourceOwnerRememberBuffer(ResourceOwner owner, Buffer buffer);
extern void ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer);
+/* support for IO-in-progress management */
+extern void ResourceOwnerEnlargeBufferIOs(ResourceOwner owner);
+extern void ResourceOwnerRememberBufferIO(ResourceOwner owner, Buffer buffer);
+extern void ResourceOwnerForgetBufferIO(ResourceOwner owner, Buffer buffer);
+
/* support for local lock management */
extern void ResourceOwnerRememberLock(ResourceOwner owner, LOCALLOCK *locallock);
extern void ResourceOwnerForgetLock(ResourceOwner owner, LOCALLOCK *locallock);
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index b8764012607..a0f53e9936a 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2725,8 +2725,7 @@ AbortTransaction(void)
pgstat_report_wait_end();
pgstat_progress_end_command();
- /* Clean up buffer I/O and buffer context locks, too */
- AbortBufferIO();
+ /* Clean up buffer context locks, too */
UnlockBuffers();
/* Reset WAL record construction state */
@@ -5086,7 +5085,6 @@ AbortSubTransaction(void)
pgstat_report_wait_end();
pgstat_progress_end_command();
- AbortBufferIO();
UnlockBuffers();
/* Reset WAL record construction state */
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index ff6149a1793..3540b340b51 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -526,7 +526,6 @@ AutoVacLauncherMain(int argc, char *argv[])
*/
LWLockReleaseAll();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
/* this is probably dead code, but let's be safe: */
if (AuxProcessResourceOwner)
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 9bb47da404d..caad642ec93 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -167,7 +167,6 @@ BackgroundWriterMain(void)
*/
LWLockReleaseAll();
ConditionVariableCancelSleep();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index aaad5c52281..ace9893d957 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -271,7 +271,6 @@ CheckpointerMain(void)
LWLockReleaseAll();
ConditionVariableCancelSleep();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index 513e580c513..65e84be39b9 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -163,7 +163,6 @@ WalWriterMain(void)
LWLockReleaseAll();
ConditionVariableCancelSleep();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 4faa77b1705..c45ad37ec00 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -159,10 +159,6 @@ int checkpoint_flush_after = DEFAULT_CHECKPOINT_FLUSH_AFTER;
int bgwriter_flush_after = DEFAULT_BGWRITER_FLUSH_AFTER;
int backend_flush_after = DEFAULT_BACKEND_FLUSH_AFTER;
-/* local state for StartBufferIO and related functions */
-static BufferDesc *InProgressBuf = NULL;
-static bool IsForInput;
-
/* local state for LockBufferForCleanup */
static BufferDesc *PinCountWaitBuf = NULL;
@@ -2712,7 +2708,6 @@ InitBufferPoolAccess(void)
static void
AtProcExit_Buffers(int code, Datum arg)
{
- AbortBufferIO();
UnlockBuffers();
CheckForBufferLeaks();
@@ -4659,7 +4654,7 @@ StartBufferIO(BufferDesc *buf, bool forInput)
{
uint32 buf_state;
- Assert(!InProgressBuf);
+ ResourceOwnerEnlargeBufferIOs(CurrentResourceOwner);
for (;;)
{
@@ -4683,8 +4678,8 @@ StartBufferIO(BufferDesc *buf, bool forInput)
buf_state |= BM_IO_IN_PROGRESS;
UnlockBufHdr(buf, buf_state);
- InProgressBuf = buf;
- IsForInput = forInput;
+ ResourceOwnerRememberBufferIO(CurrentResourceOwner,
+ BufferDescriptorGetBuffer(buf));
return true;
}
@@ -4710,8 +4705,6 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
{
uint32 buf_state;
- Assert(buf == InProgressBuf);
-
buf_state = LockBufHdr(buf);
Assert(buf_state & BM_IO_IN_PROGRESS);
@@ -4723,13 +4716,14 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
buf_state |= set_flag_bits;
UnlockBufHdr(buf, buf_state);
- InProgressBuf = NULL;
+ ResourceOwnerForgetBufferIO(CurrentResourceOwner,
+ BufferDescriptorGetBuffer(buf));
ConditionVariableBroadcast(BufferDescriptorGetIOCV(buf));
}
/*
- * AbortBufferIO: Clean up any active buffer I/O after an error.
+ * AbortBufferIO: Clean up active buffer I/O after an error.
*
* All LWLocks we might have held have been released,
* but we haven't yet released buffer pins, so the buffer is still pinned.
@@ -4738,46 +4732,42 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
* possible the error condition wasn't related to the I/O.
*/
void
-AbortBufferIO(void)
+AbortBufferIO(Buffer buf)
{
- BufferDesc *buf = InProgressBuf;
+ BufferDesc *buf_hdr = GetBufferDescriptor(buf - 1);
+ uint32 buf_state;
- if (buf)
+ buf_state = LockBufHdr(buf_hdr);
+ Assert(buf_state & (BM_IO_IN_PROGRESS | BM_TAG_VALID));
+
+ if (!(buf_state & BM_VALID))
{
- uint32 buf_state;
-
- buf_state = LockBufHdr(buf);
- Assert(buf_state & BM_IO_IN_PROGRESS);
- if (IsForInput)
- {
- Assert(!(buf_state & BM_DIRTY));
-
- /* We'd better not think buffer is valid yet */
- Assert(!(buf_state & BM_VALID));
- UnlockBufHdr(buf, buf_state);
- }
- else
- {
- Assert(buf_state & BM_DIRTY);
- UnlockBufHdr(buf, buf_state);
- /* Issue notice if this is not the first failure... */
- if (buf_state & BM_IO_ERROR)
- {
- /* Buffer is pinned, so we can read tag without spinlock */
- char *path;
-
- path = relpathperm(BufTagGetRelFileLocator(&buf->tag),
- BufTagGetForkNum(&buf->tag));
- ereport(WARNING,
- (errcode(ERRCODE_IO_ERROR),
- errmsg("could not write block %u of %s",
- buf->tag.blockNum, path),
- errdetail("Multiple failures --- write error might be permanent.")));
- pfree(path);
- }
- }
- TerminateBufferIO(buf, false, BM_IO_ERROR);
+ Assert(!(buf_state & BM_DIRTY));
+ UnlockBufHdr(buf_hdr, buf_state);
}
+ else
+ {
+ Assert(!(buf_state & BM_DIRTY));
+ UnlockBufHdr(buf_hdr, buf_state);
+
+ /* Issue notice if this is not the first failure... */
+ if (buf_state & BM_IO_ERROR)
+ {
+ /* Buffer is pinned, so we can read tag without spinlock */
+ char *path;
+
+ path = relpathperm(BufTagGetRelFileLocator(&buf_hdr->tag),
+ BufTagGetForkNum(&buf_hdr->tag));
+ ereport(WARNING,
+ (errcode(ERRCODE_IO_ERROR),
+ errmsg("could not write block %u of %s",
+ buf_hdr->tag.blockNum, path),
+ errdetail("Multiple failures --- write error might be permanent.")));
+ pfree(path);
+ }
+ }
+
+ TerminateBufferIO(buf_hdr, false, BM_IO_ERROR);
}
/*
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 19b6241e45d..fccc59b39dd 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -121,6 +121,7 @@ typedef struct ResourceOwnerData
/* We have built-in support for remembering: */
ResourceArray bufferarr; /* owned buffers */
+ ResourceArray bufferioarr; /* in-progress buffer IO */
ResourceArray catrefarr; /* catcache references */
ResourceArray catlistrefarr; /* catcache-list pins */
ResourceArray relrefarr; /* relcache references */
@@ -441,6 +442,7 @@ ResourceOwnerCreate(ResourceOwner parent, const char *name)
}
ResourceArrayInit(&(owner->bufferarr), BufferGetDatum(InvalidBuffer));
+ ResourceArrayInit(&(owner->bufferioarr), BufferGetDatum(InvalidBuffer));
ResourceArrayInit(&(owner->catrefarr), PointerGetDatum(NULL));
ResourceArrayInit(&(owner->catlistrefarr), PointerGetDatum(NULL));
ResourceArrayInit(&(owner->relrefarr), PointerGetDatum(NULL));
@@ -517,6 +519,24 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
if (phase == RESOURCE_RELEASE_BEFORE_LOCKS)
{
+ /*
+ * Abort failed buffer IO. AbortBufferIO()->TerminateBufferIO() calls
+ * ResourceOwnerForgetBufferIOs(), so we just have to iterate till
+ * there are none.
+ *
+ * Needs to be before we release buffer pins.
+ *
+ * During a commit, there shouldn't be any in-progress IO.
+ */
+ while (ResourceArrayGetAny(&(owner->bufferioarr), &foundres))
+ {
+ Buffer res = DatumGetBuffer(foundres);
+
+ if (isCommit)
+ elog(PANIC, "lost track of buffer IO on buffer %u", res);
+ AbortBufferIO(res);
+ }
+
/*
* Release buffer pins. Note that ReleaseBuffer will remove the
* buffer entry from our array, so we just have to iterate till there
@@ -746,6 +766,7 @@ ResourceOwnerDelete(ResourceOwner owner)
/* And it better not own any resources, either */
Assert(owner->bufferarr.nitems == 0);
+ Assert(owner->bufferioarr.nitems == 0);
Assert(owner->catrefarr.nitems == 0);
Assert(owner->catlistrefarr.nitems == 0);
Assert(owner->relrefarr.nitems == 0);
@@ -775,6 +796,7 @@ ResourceOwnerDelete(ResourceOwner owner)
/* And free the object. */
ResourceArrayFree(&(owner->bufferarr));
+ ResourceArrayFree(&(owner->bufferioarr));
ResourceArrayFree(&(owner->catrefarr));
ResourceArrayFree(&(owner->catlistrefarr));
ResourceArrayFree(&(owner->relrefarr));
@@ -976,6 +998,44 @@ ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer)
buffer, owner->name);
}
+
+/*
+ * Make sure there is room for at least one more entry in a ResourceOwner's
+ * buffer array.
+ *
+ * This is separate from actually inserting an entry because if we run out
+ * of memory, it's critical to do so *before* acquiring the resource.
+ */
+void
+ResourceOwnerEnlargeBufferIOs(ResourceOwner owner)
+{
+ /* We used to allow pinning buffers without a resowner, but no more */
+ Assert(owner != NULL);
+ ResourceArrayEnlarge(&(owner->bufferioarr));
+}
+
+/*
+ * Remember that a buffer IO is owned by a ResourceOwner
+ *
+ * Caller must have previously done ResourceOwnerEnlargeBufferIOs()
+ */
+void
+ResourceOwnerRememberBufferIO(ResourceOwner owner, Buffer buffer)
+{
+ ResourceArrayAdd(&(owner->bufferioarr), BufferGetDatum(buffer));
+}
+
+/*
+ * Forget that a buffer IO is owned by a ResourceOwner
+ */
+void
+ResourceOwnerForgetBufferIO(ResourceOwner owner, Buffer buffer)
+{
+ if (!ResourceArrayRemove(&(owner->bufferioarr), BufferGetDatum(buffer)))
+ elog(PANIC, "buffer IO %d is not owned by resource owner %s",
+ buffer, owner->name);
+}
+
/*
* Remember that a Local Lock is owned by a ResourceOwner
*
--
2.38.0
--njltjzibte523gwd
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0009-bufmgr-Move-relation-extension-handling-into-Exte.patch"
^ permalink raw reply [nested|flat] 8+ messages in thread
* [PATCH v2 06/14] bufmgr: Support multiple in-progress IOs by using resowner
@ 2022-10-24 23:44 Andres Freund <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Andres Freund @ 2022-10-24 23:44 UTC (permalink / raw)
---
src/include/storage/bufmgr.h | 2 +-
src/include/utils/resowner_private.h | 5 ++
src/backend/access/transam/xact.c | 4 +-
src/backend/postmaster/autovacuum.c | 1 -
src/backend/postmaster/bgwriter.c | 1 -
src/backend/postmaster/checkpointer.c | 1 -
src/backend/postmaster/walwriter.c | 1 -
src/backend/storage/buffer/bufmgr.c | 86 ++++++++++++---------------
src/backend/utils/resowner/resowner.c | 60 +++++++++++++++++++
9 files changed, 105 insertions(+), 56 deletions(-)
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index 3becf32a3c0..2e1d7540fd0 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -176,7 +176,7 @@ extern bool ConditionalLockBufferForCleanup(Buffer buffer);
extern bool IsBufferCleanupOK(Buffer buffer);
extern bool HoldingBufferPinThatDelaysRecovery(void);
-extern void AbortBufferIO(void);
+extern void AbortBufferIO(Buffer buffer);
extern void BufmgrCommit(void);
extern bool BgBufferSync(struct WritebackContext *wb_context);
diff --git a/src/include/utils/resowner_private.h b/src/include/utils/resowner_private.h
index 1b1f3181b54..ae58438ec76 100644
--- a/src/include/utils/resowner_private.h
+++ b/src/include/utils/resowner_private.h
@@ -30,6 +30,11 @@ extern void ResourceOwnerEnlargeBuffers(ResourceOwner owner);
extern void ResourceOwnerRememberBuffer(ResourceOwner owner, Buffer buffer);
extern void ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer);
+/* support for IO-in-progress management */
+extern void ResourceOwnerEnlargeBufferIOs(ResourceOwner owner);
+extern void ResourceOwnerRememberBufferIO(ResourceOwner owner, Buffer buffer);
+extern void ResourceOwnerForgetBufferIO(ResourceOwner owner, Buffer buffer);
+
/* support for local lock management */
extern void ResourceOwnerRememberLock(ResourceOwner owner, LOCALLOCK *locallock);
extern void ResourceOwnerForgetLock(ResourceOwner owner, LOCALLOCK *locallock);
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index d85e3139082..2a91ed64a7a 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2725,8 +2725,7 @@ AbortTransaction(void)
pgstat_report_wait_end();
pgstat_progress_end_command();
- /* Clean up buffer I/O and buffer context locks, too */
- AbortBufferIO();
+ /* Clean up buffer context locks, too */
UnlockBuffers();
/* Reset WAL record construction state */
@@ -5086,7 +5085,6 @@ AbortSubTransaction(void)
pgstat_report_wait_end();
pgstat_progress_end_command();
- AbortBufferIO();
UnlockBuffers();
/* Reset WAL record construction state */
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index f5ea381c53e..2d5d1855439 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -526,7 +526,6 @@ AutoVacLauncherMain(int argc, char *argv[])
*/
LWLockReleaseAll();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
/* this is probably dead code, but let's be safe: */
if (AuxProcessResourceOwner)
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 69667f0eb4b..1c4244b6ec5 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -167,7 +167,6 @@ BackgroundWriterMain(void)
*/
LWLockReleaseAll();
ConditionVariableCancelSleep();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index de0bbbfa791..c1ed6737df5 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -271,7 +271,6 @@ CheckpointerMain(void)
LWLockReleaseAll();
ConditionVariableCancelSleep();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index 3113e8fbdd5..20e0807c60d 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -163,7 +163,6 @@ WalWriterMain(void)
LWLockReleaseAll();
ConditionVariableCancelSleep();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index b9af8a05989..0cdeb644e6e 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -159,10 +159,6 @@ int checkpoint_flush_after = DEFAULT_CHECKPOINT_FLUSH_AFTER;
int bgwriter_flush_after = DEFAULT_BGWRITER_FLUSH_AFTER;
int backend_flush_after = DEFAULT_BACKEND_FLUSH_AFTER;
-/* local state for StartBufferIO and related functions */
-static BufferDesc *InProgressBuf = NULL;
-static bool IsForInput;
-
/* local state for LockBufferForCleanup */
static BufferDesc *PinCountWaitBuf = NULL;
@@ -2689,7 +2685,6 @@ InitBufferPoolAccess(void)
static void
AtProcExit_Buffers(int code, Datum arg)
{
- AbortBufferIO();
UnlockBuffers();
CheckForBufferLeaks();
@@ -4618,7 +4613,7 @@ StartBufferIO(BufferDesc *buf, bool forInput)
{
uint32 buf_state;
- Assert(!InProgressBuf);
+ ResourceOwnerEnlargeBufferIOs(CurrentResourceOwner);
for (;;)
{
@@ -4642,8 +4637,8 @@ StartBufferIO(BufferDesc *buf, bool forInput)
buf_state |= BM_IO_IN_PROGRESS;
UnlockBufHdr(buf, buf_state);
- InProgressBuf = buf;
- IsForInput = forInput;
+ ResourceOwnerRememberBufferIO(CurrentResourceOwner,
+ BufferDescriptorGetBuffer(buf));
return true;
}
@@ -4669,8 +4664,6 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
{
uint32 buf_state;
- Assert(buf == InProgressBuf);
-
buf_state = LockBufHdr(buf);
Assert(buf_state & BM_IO_IN_PROGRESS);
@@ -4682,13 +4675,14 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
buf_state |= set_flag_bits;
UnlockBufHdr(buf, buf_state);
- InProgressBuf = NULL;
+ ResourceOwnerForgetBufferIO(CurrentResourceOwner,
+ BufferDescriptorGetBuffer(buf));
ConditionVariableBroadcast(BufferDescriptorGetIOCV(buf));
}
/*
- * AbortBufferIO: Clean up any active buffer I/O after an error.
+ * AbortBufferIO: Clean up active buffer I/O after an error.
*
* All LWLocks we might have held have been released,
* but we haven't yet released buffer pins, so the buffer is still pinned.
@@ -4697,46 +4691,42 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
* possible the error condition wasn't related to the I/O.
*/
void
-AbortBufferIO(void)
+AbortBufferIO(Buffer buf)
{
- BufferDesc *buf = InProgressBuf;
+ BufferDesc *buf_hdr = GetBufferDescriptor(buf - 1);
+ uint32 buf_state;
- if (buf)
+ buf_state = LockBufHdr(buf_hdr);
+ Assert(buf_state & (BM_IO_IN_PROGRESS | BM_TAG_VALID));
+
+ if (!(buf_state & BM_VALID))
{
- uint32 buf_state;
-
- buf_state = LockBufHdr(buf);
- Assert(buf_state & BM_IO_IN_PROGRESS);
- if (IsForInput)
- {
- Assert(!(buf_state & BM_DIRTY));
-
- /* We'd better not think buffer is valid yet */
- Assert(!(buf_state & BM_VALID));
- UnlockBufHdr(buf, buf_state);
- }
- else
- {
- Assert(buf_state & BM_DIRTY);
- UnlockBufHdr(buf, buf_state);
- /* Issue notice if this is not the first failure... */
- if (buf_state & BM_IO_ERROR)
- {
- /* Buffer is pinned, so we can read tag without spinlock */
- char *path;
-
- path = relpathperm(BufTagGetRelFileLocator(&buf->tag),
- BufTagGetForkNum(&buf->tag));
- ereport(WARNING,
- (errcode(ERRCODE_IO_ERROR),
- errmsg("could not write block %u of %s",
- buf->tag.blockNum, path),
- errdetail("Multiple failures --- write error might be permanent.")));
- pfree(path);
- }
- }
- TerminateBufferIO(buf, false, BM_IO_ERROR);
+ Assert(!(buf_state & BM_DIRTY));
+ UnlockBufHdr(buf_hdr, buf_state);
}
+ else
+ {
+ Assert(!(buf_state & BM_DIRTY));
+ UnlockBufHdr(buf_hdr, buf_state);
+
+ /* Issue notice if this is not the first failure... */
+ if (buf_state & BM_IO_ERROR)
+ {
+ /* Buffer is pinned, so we can read tag without spinlock */
+ char *path;
+
+ path = relpathperm(BufTagGetRelFileLocator(&buf_hdr->tag),
+ BufTagGetForkNum(&buf_hdr->tag));
+ ereport(WARNING,
+ (errcode(ERRCODE_IO_ERROR),
+ errmsg("could not write block %u of %s",
+ buf_hdr->tag.blockNum, path),
+ errdetail("Multiple failures --- write error might be permanent.")));
+ pfree(path);
+ }
+ }
+
+ TerminateBufferIO(buf_hdr, false, BM_IO_ERROR);
}
/*
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 19b6241e45d..fccc59b39dd 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -121,6 +121,7 @@ typedef struct ResourceOwnerData
/* We have built-in support for remembering: */
ResourceArray bufferarr; /* owned buffers */
+ ResourceArray bufferioarr; /* in-progress buffer IO */
ResourceArray catrefarr; /* catcache references */
ResourceArray catlistrefarr; /* catcache-list pins */
ResourceArray relrefarr; /* relcache references */
@@ -441,6 +442,7 @@ ResourceOwnerCreate(ResourceOwner parent, const char *name)
}
ResourceArrayInit(&(owner->bufferarr), BufferGetDatum(InvalidBuffer));
+ ResourceArrayInit(&(owner->bufferioarr), BufferGetDatum(InvalidBuffer));
ResourceArrayInit(&(owner->catrefarr), PointerGetDatum(NULL));
ResourceArrayInit(&(owner->catlistrefarr), PointerGetDatum(NULL));
ResourceArrayInit(&(owner->relrefarr), PointerGetDatum(NULL));
@@ -517,6 +519,24 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
if (phase == RESOURCE_RELEASE_BEFORE_LOCKS)
{
+ /*
+ * Abort failed buffer IO. AbortBufferIO()->TerminateBufferIO() calls
+ * ResourceOwnerForgetBufferIOs(), so we just have to iterate till
+ * there are none.
+ *
+ * Needs to be before we release buffer pins.
+ *
+ * During a commit, there shouldn't be any in-progress IO.
+ */
+ while (ResourceArrayGetAny(&(owner->bufferioarr), &foundres))
+ {
+ Buffer res = DatumGetBuffer(foundres);
+
+ if (isCommit)
+ elog(PANIC, "lost track of buffer IO on buffer %u", res);
+ AbortBufferIO(res);
+ }
+
/*
* Release buffer pins. Note that ReleaseBuffer will remove the
* buffer entry from our array, so we just have to iterate till there
@@ -746,6 +766,7 @@ ResourceOwnerDelete(ResourceOwner owner)
/* And it better not own any resources, either */
Assert(owner->bufferarr.nitems == 0);
+ Assert(owner->bufferioarr.nitems == 0);
Assert(owner->catrefarr.nitems == 0);
Assert(owner->catlistrefarr.nitems == 0);
Assert(owner->relrefarr.nitems == 0);
@@ -775,6 +796,7 @@ ResourceOwnerDelete(ResourceOwner owner)
/* And free the object. */
ResourceArrayFree(&(owner->bufferarr));
+ ResourceArrayFree(&(owner->bufferioarr));
ResourceArrayFree(&(owner->catrefarr));
ResourceArrayFree(&(owner->catlistrefarr));
ResourceArrayFree(&(owner->relrefarr));
@@ -976,6 +998,44 @@ ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer)
buffer, owner->name);
}
+
+/*
+ * Make sure there is room for at least one more entry in a ResourceOwner's
+ * buffer array.
+ *
+ * This is separate from actually inserting an entry because if we run out
+ * of memory, it's critical to do so *before* acquiring the resource.
+ */
+void
+ResourceOwnerEnlargeBufferIOs(ResourceOwner owner)
+{
+ /* We used to allow pinning buffers without a resowner, but no more */
+ Assert(owner != NULL);
+ ResourceArrayEnlarge(&(owner->bufferioarr));
+}
+
+/*
+ * Remember that a buffer IO is owned by a ResourceOwner
+ *
+ * Caller must have previously done ResourceOwnerEnlargeBufferIOs()
+ */
+void
+ResourceOwnerRememberBufferIO(ResourceOwner owner, Buffer buffer)
+{
+ ResourceArrayAdd(&(owner->bufferioarr), BufferGetDatum(buffer));
+}
+
+/*
+ * Forget that a buffer IO is owned by a ResourceOwner
+ */
+void
+ResourceOwnerForgetBufferIO(ResourceOwner owner, Buffer buffer)
+{
+ if (!ResourceArrayRemove(&(owner->bufferioarr), BufferGetDatum(buffer)))
+ elog(PANIC, "buffer IO %d is not owned by resource owner %s",
+ buffer, owner->name);
+}
+
/*
* Remember that a Local Lock is owned by a ResourceOwner
*
--
2.38.0
--wsjhb5zouqhvcv7q
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v2-0007-bufmgr-Move-relation-extension-handling-into-Bulk.patch"
^ permalink raw reply [nested|flat] 8+ messages in thread
* [PATCH v7 06/14] bufmgr: Support multiple in-progress IOs by using resowner
@ 2022-10-24 23:44 Andres Freund <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Andres Freund @ 2022-10-24 23:44 UTC (permalink / raw)
A future patch will add support for extending relations by multiple blocks at
once. To be concurrency safe, the buffers for those blocks need to be marked
as BM_IO_IN_PROGRESS. Until now we only had infrastructure for recovering from
an IO error for a single buffer. This commit extends that infrastructure to
multiple buffers by using the resource owner infrastructure.
This commit increases the size of the ResourceOwnerData struct, which appears
to have a just about measurable overhead in very extreme workloads. Medium
term we are planning to substantially shrink the size of
ResourceOwnerData. Short term the increase is small enough to not worry about
it for now.
Reviewed-by: Melanie Plageman <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Discussion: https://postgr.es/m/[email protected]
---
src/include/storage/bufmgr.h | 2 +-
src/include/utils/resowner_private.h | 5 ++
src/backend/access/transam/xact.c | 4 +-
src/backend/postmaster/autovacuum.c | 1 -
src/backend/postmaster/bgwriter.c | 1 -
src/backend/postmaster/checkpointer.c | 1 -
src/backend/postmaster/walwriter.c | 1 -
src/backend/storage/buffer/bufmgr.c | 86 ++++++++++++---------------
src/backend/utils/resowner/resowner.c | 60 +++++++++++++++++++
9 files changed, 105 insertions(+), 56 deletions(-)
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index f96dc080211..537b89e8774 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -181,7 +181,7 @@ extern bool ConditionalLockBufferForCleanup(Buffer buffer);
extern bool IsBufferCleanupOK(Buffer buffer);
extern bool HoldingBufferPinThatDelaysRecovery(void);
-extern void AbortBufferIO(void);
+extern void AbortBufferIO(Buffer buffer);
extern bool BgBufferSync(struct WritebackContext *wb_context);
diff --git a/src/include/utils/resowner_private.h b/src/include/utils/resowner_private.h
index 1b1f3181b54..ae58438ec76 100644
--- a/src/include/utils/resowner_private.h
+++ b/src/include/utils/resowner_private.h
@@ -30,6 +30,11 @@ extern void ResourceOwnerEnlargeBuffers(ResourceOwner owner);
extern void ResourceOwnerRememberBuffer(ResourceOwner owner, Buffer buffer);
extern void ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer);
+/* support for IO-in-progress management */
+extern void ResourceOwnerEnlargeBufferIOs(ResourceOwner owner);
+extern void ResourceOwnerRememberBufferIO(ResourceOwner owner, Buffer buffer);
+extern void ResourceOwnerForgetBufferIO(ResourceOwner owner, Buffer buffer);
+
/* support for local lock management */
extern void ResourceOwnerRememberLock(ResourceOwner owner, LOCALLOCK *locallock);
extern void ResourceOwnerForgetLock(ResourceOwner owner, LOCALLOCK *locallock);
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 231af52cc92..6a837e1539d 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2719,8 +2719,7 @@ AbortTransaction(void)
pgstat_report_wait_end();
pgstat_progress_end_command();
- /* Clean up buffer I/O and buffer context locks, too */
- AbortBufferIO();
+ /* Clean up buffer context locks, too */
UnlockBuffers();
/* Reset WAL record construction state */
@@ -5080,7 +5079,6 @@ AbortSubTransaction(void)
pgstat_report_wait_end();
pgstat_progress_end_command();
- AbortBufferIO();
UnlockBuffers();
/* Reset WAL record construction state */
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 585d28148ca..e9ba0dc17cd 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -526,7 +526,6 @@ AutoVacLauncherMain(int argc, char *argv[])
*/
LWLockReleaseAll();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
/* this is probably dead code, but let's be safe: */
if (AuxProcessResourceOwner)
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 9bb47da404d..caad642ec93 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -167,7 +167,6 @@ BackgroundWriterMain(void)
*/
LWLockReleaseAll();
ConditionVariableCancelSleep();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index aaad5c52281..ace9893d957 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -271,7 +271,6 @@ CheckpointerMain(void)
LWLockReleaseAll();
ConditionVariableCancelSleep();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index 513e580c513..65e84be39b9 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -163,7 +163,6 @@ WalWriterMain(void)
LWLockReleaseAll();
ConditionVariableCancelSleep();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 497050c83d4..8de02c44aba 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -159,10 +159,6 @@ int checkpoint_flush_after = DEFAULT_CHECKPOINT_FLUSH_AFTER;
int bgwriter_flush_after = DEFAULT_BGWRITER_FLUSH_AFTER;
int backend_flush_after = DEFAULT_BACKEND_FLUSH_AFTER;
-/* local state for StartBufferIO and related functions */
-static BufferDesc *InProgressBuf = NULL;
-static bool IsForInput;
-
/* local state for LockBufferForCleanup */
static BufferDesc *PinCountWaitBuf = NULL;
@@ -2702,7 +2698,6 @@ InitBufferPoolAccess(void)
static void
AtProcExit_Buffers(int code, Datum arg)
{
- AbortBufferIO();
UnlockBuffers();
CheckForBufferLeaks();
@@ -4644,7 +4639,7 @@ StartBufferIO(BufferDesc *buf, bool forInput)
{
uint32 buf_state;
- Assert(!InProgressBuf);
+ ResourceOwnerEnlargeBufferIOs(CurrentResourceOwner);
for (;;)
{
@@ -4668,8 +4663,8 @@ StartBufferIO(BufferDesc *buf, bool forInput)
buf_state |= BM_IO_IN_PROGRESS;
UnlockBufHdr(buf, buf_state);
- InProgressBuf = buf;
- IsForInput = forInput;
+ ResourceOwnerRememberBufferIO(CurrentResourceOwner,
+ BufferDescriptorGetBuffer(buf));
return true;
}
@@ -4695,8 +4690,6 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
{
uint32 buf_state;
- Assert(buf == InProgressBuf);
-
buf_state = LockBufHdr(buf);
Assert(buf_state & BM_IO_IN_PROGRESS);
@@ -4708,13 +4701,14 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
buf_state |= set_flag_bits;
UnlockBufHdr(buf, buf_state);
- InProgressBuf = NULL;
+ ResourceOwnerForgetBufferIO(CurrentResourceOwner,
+ BufferDescriptorGetBuffer(buf));
ConditionVariableBroadcast(BufferDescriptorGetIOCV(buf));
}
/*
- * AbortBufferIO: Clean up any active buffer I/O after an error.
+ * AbortBufferIO: Clean up active buffer I/O after an error.
*
* All LWLocks we might have held have been released,
* but we haven't yet released buffer pins, so the buffer is still pinned.
@@ -4723,46 +4717,42 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
* possible the error condition wasn't related to the I/O.
*/
void
-AbortBufferIO(void)
+AbortBufferIO(Buffer buf)
{
- BufferDesc *buf = InProgressBuf;
+ BufferDesc *buf_hdr = GetBufferDescriptor(buf - 1);
+ uint32 buf_state;
- if (buf)
+ buf_state = LockBufHdr(buf_hdr);
+ Assert(buf_state & (BM_IO_IN_PROGRESS | BM_TAG_VALID));
+
+ if (!(buf_state & BM_VALID))
{
- uint32 buf_state;
-
- buf_state = LockBufHdr(buf);
- Assert(buf_state & BM_IO_IN_PROGRESS);
- if (IsForInput)
- {
- Assert(!(buf_state & BM_DIRTY));
-
- /* We'd better not think buffer is valid yet */
- Assert(!(buf_state & BM_VALID));
- UnlockBufHdr(buf, buf_state);
- }
- else
- {
- Assert(buf_state & BM_DIRTY);
- UnlockBufHdr(buf, buf_state);
- /* Issue notice if this is not the first failure... */
- if (buf_state & BM_IO_ERROR)
- {
- /* Buffer is pinned, so we can read tag without spinlock */
- char *path;
-
- path = relpathperm(BufTagGetRelFileLocator(&buf->tag),
- BufTagGetForkNum(&buf->tag));
- ereport(WARNING,
- (errcode(ERRCODE_IO_ERROR),
- errmsg("could not write block %u of %s",
- buf->tag.blockNum, path),
- errdetail("Multiple failures --- write error might be permanent.")));
- pfree(path);
- }
- }
- TerminateBufferIO(buf, false, BM_IO_ERROR);
+ Assert(!(buf_state & BM_DIRTY));
+ UnlockBufHdr(buf_hdr, buf_state);
}
+ else
+ {
+ Assert(!(buf_state & BM_DIRTY));
+ UnlockBufHdr(buf_hdr, buf_state);
+
+ /* Issue notice if this is not the first failure... */
+ if (buf_state & BM_IO_ERROR)
+ {
+ /* Buffer is pinned, so we can read tag without spinlock */
+ char *path;
+
+ path = relpathperm(BufTagGetRelFileLocator(&buf_hdr->tag),
+ BufTagGetForkNum(&buf_hdr->tag));
+ ereport(WARNING,
+ (errcode(ERRCODE_IO_ERROR),
+ errmsg("could not write block %u of %s",
+ buf_hdr->tag.blockNum, path),
+ errdetail("Multiple failures --- write error might be permanent.")));
+ pfree(path);
+ }
+ }
+
+ TerminateBufferIO(buf_hdr, false, BM_IO_ERROR);
}
/*
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 19b6241e45d..fccc59b39dd 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -121,6 +121,7 @@ typedef struct ResourceOwnerData
/* We have built-in support for remembering: */
ResourceArray bufferarr; /* owned buffers */
+ ResourceArray bufferioarr; /* in-progress buffer IO */
ResourceArray catrefarr; /* catcache references */
ResourceArray catlistrefarr; /* catcache-list pins */
ResourceArray relrefarr; /* relcache references */
@@ -441,6 +442,7 @@ ResourceOwnerCreate(ResourceOwner parent, const char *name)
}
ResourceArrayInit(&(owner->bufferarr), BufferGetDatum(InvalidBuffer));
+ ResourceArrayInit(&(owner->bufferioarr), BufferGetDatum(InvalidBuffer));
ResourceArrayInit(&(owner->catrefarr), PointerGetDatum(NULL));
ResourceArrayInit(&(owner->catlistrefarr), PointerGetDatum(NULL));
ResourceArrayInit(&(owner->relrefarr), PointerGetDatum(NULL));
@@ -517,6 +519,24 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
if (phase == RESOURCE_RELEASE_BEFORE_LOCKS)
{
+ /*
+ * Abort failed buffer IO. AbortBufferIO()->TerminateBufferIO() calls
+ * ResourceOwnerForgetBufferIOs(), so we just have to iterate till
+ * there are none.
+ *
+ * Needs to be before we release buffer pins.
+ *
+ * During a commit, there shouldn't be any in-progress IO.
+ */
+ while (ResourceArrayGetAny(&(owner->bufferioarr), &foundres))
+ {
+ Buffer res = DatumGetBuffer(foundres);
+
+ if (isCommit)
+ elog(PANIC, "lost track of buffer IO on buffer %u", res);
+ AbortBufferIO(res);
+ }
+
/*
* Release buffer pins. Note that ReleaseBuffer will remove the
* buffer entry from our array, so we just have to iterate till there
@@ -746,6 +766,7 @@ ResourceOwnerDelete(ResourceOwner owner)
/* And it better not own any resources, either */
Assert(owner->bufferarr.nitems == 0);
+ Assert(owner->bufferioarr.nitems == 0);
Assert(owner->catrefarr.nitems == 0);
Assert(owner->catlistrefarr.nitems == 0);
Assert(owner->relrefarr.nitems == 0);
@@ -775,6 +796,7 @@ ResourceOwnerDelete(ResourceOwner owner)
/* And free the object. */
ResourceArrayFree(&(owner->bufferarr));
+ ResourceArrayFree(&(owner->bufferioarr));
ResourceArrayFree(&(owner->catrefarr));
ResourceArrayFree(&(owner->catlistrefarr));
ResourceArrayFree(&(owner->relrefarr));
@@ -976,6 +998,44 @@ ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer)
buffer, owner->name);
}
+
+/*
+ * Make sure there is room for at least one more entry in a ResourceOwner's
+ * buffer array.
+ *
+ * This is separate from actually inserting an entry because if we run out
+ * of memory, it's critical to do so *before* acquiring the resource.
+ */
+void
+ResourceOwnerEnlargeBufferIOs(ResourceOwner owner)
+{
+ /* We used to allow pinning buffers without a resowner, but no more */
+ Assert(owner != NULL);
+ ResourceArrayEnlarge(&(owner->bufferioarr));
+}
+
+/*
+ * Remember that a buffer IO is owned by a ResourceOwner
+ *
+ * Caller must have previously done ResourceOwnerEnlargeBufferIOs()
+ */
+void
+ResourceOwnerRememberBufferIO(ResourceOwner owner, Buffer buffer)
+{
+ ResourceArrayAdd(&(owner->bufferioarr), BufferGetDatum(buffer));
+}
+
+/*
+ * Forget that a buffer IO is owned by a ResourceOwner
+ */
+void
+ResourceOwnerForgetBufferIO(ResourceOwner owner, Buffer buffer)
+{
+ if (!ResourceArrayRemove(&(owner->bufferioarr), BufferGetDatum(buffer)))
+ elog(PANIC, "buffer IO %d is not owned by resource owner %s",
+ buffer, owner->name);
+}
+
/*
* Remember that a Local Lock is owned by a ResourceOwner
*
--
2.38.0
--ju7ntqqtbf66a3ug
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0007-bufmgr-Introduce-infrastructure-for-faster-relati.patch"
^ permalink raw reply [nested|flat] 8+ messages in thread
* [PATCH v5 08/15] bufmgr: Support multiple in-progress IOs by using resowner
@ 2022-10-24 23:44 Andres Freund <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Andres Freund @ 2022-10-24 23:44 UTC (permalink / raw)
Commit message should describe why we couldn't support multiple
in-progress IOs before, I think (e.g. we couldn't be sure that we
cleared IO_IN_PROGRESS if something happened).
@@ -4709,8 +4704,6 @@ TerminateBufferIO(BufferDesc *buf, bool
clear_dirty, uint32 set_flag_bits)
{
uint32 buf_state;
I noticed that the comment above TermianteBufferIO() says
* TerminateBufferIO: release a buffer we were doing I/O on
* (Assumptions)
* My process is executing IO for the buffer
Can we still say this is an assumption? What about when it is being
cleaned up after being called from AbortBufferIO()
diff --git a/src/backend/utils/resowner/resowner.c
b/src/backend/utils/resowner/resowner.c
index 19b6241e45..fccc59b39d 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -121,6 +121,7 @@ typedef struct ResourceOwnerData
/* We have built-in support for remembering: */
ResourceArray bufferarr; /* owned buffers */
+ ResourceArray bufferioarr; /* in-progress buffer IO */
ResourceArray catrefarr; /* catcache references */
ResourceArray catlistrefarr; /* catcache-list pins */
ResourceArray relrefarr; /* relcache references */
@@ -441,6 +442,7 @@ ResourceOwnerCreate(ResourceOwner parent, const char *name)
Maybe worth mentioning in-progress buffer IO in resowner README? I know
it doesn't claim to be exhaustive, so, up to you.
Also, I realize that existing code in this file has the extraneous
parantheses, but maybe it isn't worth staying consistent with that?
as in: &(owner->bufferioarr)
+ */
+void
+ResourceOwnerRememberBufferIO(ResourceOwner owner, Buffer buffer)
+{
+ ResourceArrayAdd(&(owner->bufferioarr), BufferGetDatum(buffer));
+}
+
^ permalink raw reply [nested|flat] 8+ messages in thread
* [PATCH v6 10/17] bufmgr: Support multiple in-progress IOs by using resowner
@ 2022-10-24 23:44 Andres Freund <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Andres Freund @ 2022-10-24 23:44 UTC (permalink / raw)
A future patch will add support for extending relations by multiple blocks at
once. To be concurrency safe, the buffers for those blocks need to be marked
as BM_IO_IN_PROGRESS. Until now we only had infrastructure for recovering from
an IO error for a single buffer. This commit extends that infrastructure to
multiple buffers by using the resource owner infrastructure.
This commit increases the size of the ResourceOwnerData struct, which appears
to have a just about measurable overhead in very extreme workloads. Medium
term we are planning to substantially shrink the size of
ResourceOwnerData. Short term the increase is small enough to not worry about
it for now.
Reviewed-by: Melanie Plageman <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Discussion: https://postgr.es/m/[email protected]
IO on a single b
---
src/include/storage/bufmgr.h | 2 +-
src/include/utils/resowner_private.h | 5 ++
src/backend/access/transam/xact.c | 4 +-
src/backend/postmaster/autovacuum.c | 1 -
src/backend/postmaster/bgwriter.c | 1 -
src/backend/postmaster/checkpointer.c | 1 -
src/backend/postmaster/walwriter.c | 1 -
src/backend/storage/buffer/bufmgr.c | 86 ++++++++++++---------------
src/backend/utils/resowner/resowner.c | 60 +++++++++++++++++++
9 files changed, 105 insertions(+), 56 deletions(-)
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index d30733c65a1..9ba25521ea1 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -181,7 +181,7 @@ extern bool ConditionalLockBufferForCleanup(Buffer buffer);
extern bool IsBufferCleanupOK(Buffer buffer);
extern bool HoldingBufferPinThatDelaysRecovery(void);
-extern void AbortBufferIO(void);
+extern void AbortBufferIO(Buffer buffer);
extern void BufmgrCommit(void);
extern bool BgBufferSync(struct WritebackContext *wb_context);
diff --git a/src/include/utils/resowner_private.h b/src/include/utils/resowner_private.h
index 1b1f3181b54..ae58438ec76 100644
--- a/src/include/utils/resowner_private.h
+++ b/src/include/utils/resowner_private.h
@@ -30,6 +30,11 @@ extern void ResourceOwnerEnlargeBuffers(ResourceOwner owner);
extern void ResourceOwnerRememberBuffer(ResourceOwner owner, Buffer buffer);
extern void ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer);
+/* support for IO-in-progress management */
+extern void ResourceOwnerEnlargeBufferIOs(ResourceOwner owner);
+extern void ResourceOwnerRememberBufferIO(ResourceOwner owner, Buffer buffer);
+extern void ResourceOwnerForgetBufferIO(ResourceOwner owner, Buffer buffer);
+
/* support for local lock management */
extern void ResourceOwnerRememberLock(ResourceOwner owner, LOCALLOCK *locallock);
extern void ResourceOwnerForgetLock(ResourceOwner owner, LOCALLOCK *locallock);
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index b8764012607..a0f53e9936a 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2725,8 +2725,7 @@ AbortTransaction(void)
pgstat_report_wait_end();
pgstat_progress_end_command();
- /* Clean up buffer I/O and buffer context locks, too */
- AbortBufferIO();
+ /* Clean up buffer context locks, too */
UnlockBuffers();
/* Reset WAL record construction state */
@@ -5086,7 +5085,6 @@ AbortSubTransaction(void)
pgstat_report_wait_end();
pgstat_progress_end_command();
- AbortBufferIO();
UnlockBuffers();
/* Reset WAL record construction state */
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 585d28148ca..e9ba0dc17cd 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -526,7 +526,6 @@ AutoVacLauncherMain(int argc, char *argv[])
*/
LWLockReleaseAll();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
/* this is probably dead code, but let's be safe: */
if (AuxProcessResourceOwner)
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 9bb47da404d..caad642ec93 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -167,7 +167,6 @@ BackgroundWriterMain(void)
*/
LWLockReleaseAll();
ConditionVariableCancelSleep();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index aaad5c52281..ace9893d957 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -271,7 +271,6 @@ CheckpointerMain(void)
LWLockReleaseAll();
ConditionVariableCancelSleep();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index 513e580c513..65e84be39b9 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -163,7 +163,6 @@ WalWriterMain(void)
LWLockReleaseAll();
ConditionVariableCancelSleep();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 02281303440..41153f99bc2 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -159,10 +159,6 @@ int checkpoint_flush_after = DEFAULT_CHECKPOINT_FLUSH_AFTER;
int bgwriter_flush_after = DEFAULT_BGWRITER_FLUSH_AFTER;
int backend_flush_after = DEFAULT_BACKEND_FLUSH_AFTER;
-/* local state for StartBufferIO and related functions */
-static BufferDesc *InProgressBuf = NULL;
-static bool IsForInput;
-
/* local state for LockBufferForCleanup */
static BufferDesc *PinCountWaitBuf = NULL;
@@ -2712,7 +2708,6 @@ InitBufferPoolAccess(void)
static void
AtProcExit_Buffers(int code, Datum arg)
{
- AbortBufferIO();
UnlockBuffers();
CheckForBufferLeaks();
@@ -4658,7 +4653,7 @@ StartBufferIO(BufferDesc *buf, bool forInput)
{
uint32 buf_state;
- Assert(!InProgressBuf);
+ ResourceOwnerEnlargeBufferIOs(CurrentResourceOwner);
for (;;)
{
@@ -4682,8 +4677,8 @@ StartBufferIO(BufferDesc *buf, bool forInput)
buf_state |= BM_IO_IN_PROGRESS;
UnlockBufHdr(buf, buf_state);
- InProgressBuf = buf;
- IsForInput = forInput;
+ ResourceOwnerRememberBufferIO(CurrentResourceOwner,
+ BufferDescriptorGetBuffer(buf));
return true;
}
@@ -4709,8 +4704,6 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
{
uint32 buf_state;
- Assert(buf == InProgressBuf);
-
buf_state = LockBufHdr(buf);
Assert(buf_state & BM_IO_IN_PROGRESS);
@@ -4722,13 +4715,14 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
buf_state |= set_flag_bits;
UnlockBufHdr(buf, buf_state);
- InProgressBuf = NULL;
+ ResourceOwnerForgetBufferIO(CurrentResourceOwner,
+ BufferDescriptorGetBuffer(buf));
ConditionVariableBroadcast(BufferDescriptorGetIOCV(buf));
}
/*
- * AbortBufferIO: Clean up any active buffer I/O after an error.
+ * AbortBufferIO: Clean up active buffer I/O after an error.
*
* All LWLocks we might have held have been released,
* but we haven't yet released buffer pins, so the buffer is still pinned.
@@ -4737,46 +4731,42 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
* possible the error condition wasn't related to the I/O.
*/
void
-AbortBufferIO(void)
+AbortBufferIO(Buffer buf)
{
- BufferDesc *buf = InProgressBuf;
+ BufferDesc *buf_hdr = GetBufferDescriptor(buf - 1);
+ uint32 buf_state;
- if (buf)
+ buf_state = LockBufHdr(buf_hdr);
+ Assert(buf_state & (BM_IO_IN_PROGRESS | BM_TAG_VALID));
+
+ if (!(buf_state & BM_VALID))
{
- uint32 buf_state;
-
- buf_state = LockBufHdr(buf);
- Assert(buf_state & BM_IO_IN_PROGRESS);
- if (IsForInput)
- {
- Assert(!(buf_state & BM_DIRTY));
-
- /* We'd better not think buffer is valid yet */
- Assert(!(buf_state & BM_VALID));
- UnlockBufHdr(buf, buf_state);
- }
- else
- {
- Assert(buf_state & BM_DIRTY);
- UnlockBufHdr(buf, buf_state);
- /* Issue notice if this is not the first failure... */
- if (buf_state & BM_IO_ERROR)
- {
- /* Buffer is pinned, so we can read tag without spinlock */
- char *path;
-
- path = relpathperm(BufTagGetRelFileLocator(&buf->tag),
- BufTagGetForkNum(&buf->tag));
- ereport(WARNING,
- (errcode(ERRCODE_IO_ERROR),
- errmsg("could not write block %u of %s",
- buf->tag.blockNum, path),
- errdetail("Multiple failures --- write error might be permanent.")));
- pfree(path);
- }
- }
- TerminateBufferIO(buf, false, BM_IO_ERROR);
+ Assert(!(buf_state & BM_DIRTY));
+ UnlockBufHdr(buf_hdr, buf_state);
}
+ else
+ {
+ Assert(!(buf_state & BM_DIRTY));
+ UnlockBufHdr(buf_hdr, buf_state);
+
+ /* Issue notice if this is not the first failure... */
+ if (buf_state & BM_IO_ERROR)
+ {
+ /* Buffer is pinned, so we can read tag without spinlock */
+ char *path;
+
+ path = relpathperm(BufTagGetRelFileLocator(&buf_hdr->tag),
+ BufTagGetForkNum(&buf_hdr->tag));
+ ereport(WARNING,
+ (errcode(ERRCODE_IO_ERROR),
+ errmsg("could not write block %u of %s",
+ buf_hdr->tag.blockNum, path),
+ errdetail("Multiple failures --- write error might be permanent.")));
+ pfree(path);
+ }
+ }
+
+ TerminateBufferIO(buf_hdr, false, BM_IO_ERROR);
}
/*
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 19b6241e45d..fccc59b39dd 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -121,6 +121,7 @@ typedef struct ResourceOwnerData
/* We have built-in support for remembering: */
ResourceArray bufferarr; /* owned buffers */
+ ResourceArray bufferioarr; /* in-progress buffer IO */
ResourceArray catrefarr; /* catcache references */
ResourceArray catlistrefarr; /* catcache-list pins */
ResourceArray relrefarr; /* relcache references */
@@ -441,6 +442,7 @@ ResourceOwnerCreate(ResourceOwner parent, const char *name)
}
ResourceArrayInit(&(owner->bufferarr), BufferGetDatum(InvalidBuffer));
+ ResourceArrayInit(&(owner->bufferioarr), BufferGetDatum(InvalidBuffer));
ResourceArrayInit(&(owner->catrefarr), PointerGetDatum(NULL));
ResourceArrayInit(&(owner->catlistrefarr), PointerGetDatum(NULL));
ResourceArrayInit(&(owner->relrefarr), PointerGetDatum(NULL));
@@ -517,6 +519,24 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
if (phase == RESOURCE_RELEASE_BEFORE_LOCKS)
{
+ /*
+ * Abort failed buffer IO. AbortBufferIO()->TerminateBufferIO() calls
+ * ResourceOwnerForgetBufferIOs(), so we just have to iterate till
+ * there are none.
+ *
+ * Needs to be before we release buffer pins.
+ *
+ * During a commit, there shouldn't be any in-progress IO.
+ */
+ while (ResourceArrayGetAny(&(owner->bufferioarr), &foundres))
+ {
+ Buffer res = DatumGetBuffer(foundres);
+
+ if (isCommit)
+ elog(PANIC, "lost track of buffer IO on buffer %u", res);
+ AbortBufferIO(res);
+ }
+
/*
* Release buffer pins. Note that ReleaseBuffer will remove the
* buffer entry from our array, so we just have to iterate till there
@@ -746,6 +766,7 @@ ResourceOwnerDelete(ResourceOwner owner)
/* And it better not own any resources, either */
Assert(owner->bufferarr.nitems == 0);
+ Assert(owner->bufferioarr.nitems == 0);
Assert(owner->catrefarr.nitems == 0);
Assert(owner->catlistrefarr.nitems == 0);
Assert(owner->relrefarr.nitems == 0);
@@ -775,6 +796,7 @@ ResourceOwnerDelete(ResourceOwner owner)
/* And free the object. */
ResourceArrayFree(&(owner->bufferarr));
+ ResourceArrayFree(&(owner->bufferioarr));
ResourceArrayFree(&(owner->catrefarr));
ResourceArrayFree(&(owner->catlistrefarr));
ResourceArrayFree(&(owner->relrefarr));
@@ -976,6 +998,44 @@ ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer)
buffer, owner->name);
}
+
+/*
+ * Make sure there is room for at least one more entry in a ResourceOwner's
+ * buffer array.
+ *
+ * This is separate from actually inserting an entry because if we run out
+ * of memory, it's critical to do so *before* acquiring the resource.
+ */
+void
+ResourceOwnerEnlargeBufferIOs(ResourceOwner owner)
+{
+ /* We used to allow pinning buffers without a resowner, but no more */
+ Assert(owner != NULL);
+ ResourceArrayEnlarge(&(owner->bufferioarr));
+}
+
+/*
+ * Remember that a buffer IO is owned by a ResourceOwner
+ *
+ * Caller must have previously done ResourceOwnerEnlargeBufferIOs()
+ */
+void
+ResourceOwnerRememberBufferIO(ResourceOwner owner, Buffer buffer)
+{
+ ResourceArrayAdd(&(owner->bufferioarr), BufferGetDatum(buffer));
+}
+
+/*
+ * Forget that a buffer IO is owned by a ResourceOwner
+ */
+void
+ResourceOwnerForgetBufferIO(ResourceOwner owner, Buffer buffer)
+{
+ if (!ResourceArrayRemove(&(owner->bufferioarr), BufferGetDatum(buffer)))
+ elog(PANIC, "buffer IO %d is not owned by resource owner %s",
+ buffer, owner->name);
+}
+
/*
* Remember that a Local Lock is owned by a ResourceOwner
*
--
2.38.0
--cfn72vqnlbycypta
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0011-bufmgr-Move-relation-extension-handling-into-Exte.patch"
^ permalink raw reply [nested|flat] 8+ messages in thread
* [PATCH v5 07/14] bufmgr: Support multiple in-progress IOs by using resowner
@ 2022-10-24 23:44 Andres Freund <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Andres Freund @ 2022-10-24 23:44 UTC (permalink / raw)
---
src/include/storage/bufmgr.h | 2 +-
src/include/utils/resowner_private.h | 5 ++
src/backend/access/transam/xact.c | 4 +-
src/backend/postmaster/autovacuum.c | 1 -
src/backend/postmaster/bgwriter.c | 1 -
src/backend/postmaster/checkpointer.c | 1 -
src/backend/postmaster/walwriter.c | 1 -
src/backend/storage/buffer/bufmgr.c | 86 ++++++++++++---------------
src/backend/utils/resowner/resowner.c | 60 +++++++++++++++++++
9 files changed, 105 insertions(+), 56 deletions(-)
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index d30733c65a1..9ba25521ea1 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -181,7 +181,7 @@ extern bool ConditionalLockBufferForCleanup(Buffer buffer);
extern bool IsBufferCleanupOK(Buffer buffer);
extern bool HoldingBufferPinThatDelaysRecovery(void);
-extern void AbortBufferIO(void);
+extern void AbortBufferIO(Buffer buffer);
extern void BufmgrCommit(void);
extern bool BgBufferSync(struct WritebackContext *wb_context);
diff --git a/src/include/utils/resowner_private.h b/src/include/utils/resowner_private.h
index 1b1f3181b54..ae58438ec76 100644
--- a/src/include/utils/resowner_private.h
+++ b/src/include/utils/resowner_private.h
@@ -30,6 +30,11 @@ extern void ResourceOwnerEnlargeBuffers(ResourceOwner owner);
extern void ResourceOwnerRememberBuffer(ResourceOwner owner, Buffer buffer);
extern void ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer);
+/* support for IO-in-progress management */
+extern void ResourceOwnerEnlargeBufferIOs(ResourceOwner owner);
+extern void ResourceOwnerRememberBufferIO(ResourceOwner owner, Buffer buffer);
+extern void ResourceOwnerForgetBufferIO(ResourceOwner owner, Buffer buffer);
+
/* support for local lock management */
extern void ResourceOwnerRememberLock(ResourceOwner owner, LOCALLOCK *locallock);
extern void ResourceOwnerForgetLock(ResourceOwner owner, LOCALLOCK *locallock);
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index b8764012607..a0f53e9936a 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2725,8 +2725,7 @@ AbortTransaction(void)
pgstat_report_wait_end();
pgstat_progress_end_command();
- /* Clean up buffer I/O and buffer context locks, too */
- AbortBufferIO();
+ /* Clean up buffer context locks, too */
UnlockBuffers();
/* Reset WAL record construction state */
@@ -5086,7 +5085,6 @@ AbortSubTransaction(void)
pgstat_report_wait_end();
pgstat_progress_end_command();
- AbortBufferIO();
UnlockBuffers();
/* Reset WAL record construction state */
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 585d28148ca..e9ba0dc17cd 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -526,7 +526,6 @@ AutoVacLauncherMain(int argc, char *argv[])
*/
LWLockReleaseAll();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
/* this is probably dead code, but let's be safe: */
if (AuxProcessResourceOwner)
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 9bb47da404d..caad642ec93 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -167,7 +167,6 @@ BackgroundWriterMain(void)
*/
LWLockReleaseAll();
ConditionVariableCancelSleep();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index aaad5c52281..ace9893d957 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -271,7 +271,6 @@ CheckpointerMain(void)
LWLockReleaseAll();
ConditionVariableCancelSleep();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index 513e580c513..65e84be39b9 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -163,7 +163,6 @@ WalWriterMain(void)
LWLockReleaseAll();
ConditionVariableCancelSleep();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 02281303440..41153f99bc2 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -159,10 +159,6 @@ int checkpoint_flush_after = DEFAULT_CHECKPOINT_FLUSH_AFTER;
int bgwriter_flush_after = DEFAULT_BGWRITER_FLUSH_AFTER;
int backend_flush_after = DEFAULT_BACKEND_FLUSH_AFTER;
-/* local state for StartBufferIO and related functions */
-static BufferDesc *InProgressBuf = NULL;
-static bool IsForInput;
-
/* local state for LockBufferForCleanup */
static BufferDesc *PinCountWaitBuf = NULL;
@@ -2712,7 +2708,6 @@ InitBufferPoolAccess(void)
static void
AtProcExit_Buffers(int code, Datum arg)
{
- AbortBufferIO();
UnlockBuffers();
CheckForBufferLeaks();
@@ -4658,7 +4653,7 @@ StartBufferIO(BufferDesc *buf, bool forInput)
{
uint32 buf_state;
- Assert(!InProgressBuf);
+ ResourceOwnerEnlargeBufferIOs(CurrentResourceOwner);
for (;;)
{
@@ -4682,8 +4677,8 @@ StartBufferIO(BufferDesc *buf, bool forInput)
buf_state |= BM_IO_IN_PROGRESS;
UnlockBufHdr(buf, buf_state);
- InProgressBuf = buf;
- IsForInput = forInput;
+ ResourceOwnerRememberBufferIO(CurrentResourceOwner,
+ BufferDescriptorGetBuffer(buf));
return true;
}
@@ -4709,8 +4704,6 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
{
uint32 buf_state;
- Assert(buf == InProgressBuf);
-
buf_state = LockBufHdr(buf);
Assert(buf_state & BM_IO_IN_PROGRESS);
@@ -4722,13 +4715,14 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
buf_state |= set_flag_bits;
UnlockBufHdr(buf, buf_state);
- InProgressBuf = NULL;
+ ResourceOwnerForgetBufferIO(CurrentResourceOwner,
+ BufferDescriptorGetBuffer(buf));
ConditionVariableBroadcast(BufferDescriptorGetIOCV(buf));
}
/*
- * AbortBufferIO: Clean up any active buffer I/O after an error.
+ * AbortBufferIO: Clean up active buffer I/O after an error.
*
* All LWLocks we might have held have been released,
* but we haven't yet released buffer pins, so the buffer is still pinned.
@@ -4737,46 +4731,42 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
* possible the error condition wasn't related to the I/O.
*/
void
-AbortBufferIO(void)
+AbortBufferIO(Buffer buf)
{
- BufferDesc *buf = InProgressBuf;
+ BufferDesc *buf_hdr = GetBufferDescriptor(buf - 1);
+ uint32 buf_state;
- if (buf)
+ buf_state = LockBufHdr(buf_hdr);
+ Assert(buf_state & (BM_IO_IN_PROGRESS | BM_TAG_VALID));
+
+ if (!(buf_state & BM_VALID))
{
- uint32 buf_state;
-
- buf_state = LockBufHdr(buf);
- Assert(buf_state & BM_IO_IN_PROGRESS);
- if (IsForInput)
- {
- Assert(!(buf_state & BM_DIRTY));
-
- /* We'd better not think buffer is valid yet */
- Assert(!(buf_state & BM_VALID));
- UnlockBufHdr(buf, buf_state);
- }
- else
- {
- Assert(buf_state & BM_DIRTY);
- UnlockBufHdr(buf, buf_state);
- /* Issue notice if this is not the first failure... */
- if (buf_state & BM_IO_ERROR)
- {
- /* Buffer is pinned, so we can read tag without spinlock */
- char *path;
-
- path = relpathperm(BufTagGetRelFileLocator(&buf->tag),
- BufTagGetForkNum(&buf->tag));
- ereport(WARNING,
- (errcode(ERRCODE_IO_ERROR),
- errmsg("could not write block %u of %s",
- buf->tag.blockNum, path),
- errdetail("Multiple failures --- write error might be permanent.")));
- pfree(path);
- }
- }
- TerminateBufferIO(buf, false, BM_IO_ERROR);
+ Assert(!(buf_state & BM_DIRTY));
+ UnlockBufHdr(buf_hdr, buf_state);
}
+ else
+ {
+ Assert(!(buf_state & BM_DIRTY));
+ UnlockBufHdr(buf_hdr, buf_state);
+
+ /* Issue notice if this is not the first failure... */
+ if (buf_state & BM_IO_ERROR)
+ {
+ /* Buffer is pinned, so we can read tag without spinlock */
+ char *path;
+
+ path = relpathperm(BufTagGetRelFileLocator(&buf_hdr->tag),
+ BufTagGetForkNum(&buf_hdr->tag));
+ ereport(WARNING,
+ (errcode(ERRCODE_IO_ERROR),
+ errmsg("could not write block %u of %s",
+ buf_hdr->tag.blockNum, path),
+ errdetail("Multiple failures --- write error might be permanent.")));
+ pfree(path);
+ }
+ }
+
+ TerminateBufferIO(buf_hdr, false, BM_IO_ERROR);
}
/*
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 19b6241e45d..fccc59b39dd 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -121,6 +121,7 @@ typedef struct ResourceOwnerData
/* We have built-in support for remembering: */
ResourceArray bufferarr; /* owned buffers */
+ ResourceArray bufferioarr; /* in-progress buffer IO */
ResourceArray catrefarr; /* catcache references */
ResourceArray catlistrefarr; /* catcache-list pins */
ResourceArray relrefarr; /* relcache references */
@@ -441,6 +442,7 @@ ResourceOwnerCreate(ResourceOwner parent, const char *name)
}
ResourceArrayInit(&(owner->bufferarr), BufferGetDatum(InvalidBuffer));
+ ResourceArrayInit(&(owner->bufferioarr), BufferGetDatum(InvalidBuffer));
ResourceArrayInit(&(owner->catrefarr), PointerGetDatum(NULL));
ResourceArrayInit(&(owner->catlistrefarr), PointerGetDatum(NULL));
ResourceArrayInit(&(owner->relrefarr), PointerGetDatum(NULL));
@@ -517,6 +519,24 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
if (phase == RESOURCE_RELEASE_BEFORE_LOCKS)
{
+ /*
+ * Abort failed buffer IO. AbortBufferIO()->TerminateBufferIO() calls
+ * ResourceOwnerForgetBufferIOs(), so we just have to iterate till
+ * there are none.
+ *
+ * Needs to be before we release buffer pins.
+ *
+ * During a commit, there shouldn't be any in-progress IO.
+ */
+ while (ResourceArrayGetAny(&(owner->bufferioarr), &foundres))
+ {
+ Buffer res = DatumGetBuffer(foundres);
+
+ if (isCommit)
+ elog(PANIC, "lost track of buffer IO on buffer %u", res);
+ AbortBufferIO(res);
+ }
+
/*
* Release buffer pins. Note that ReleaseBuffer will remove the
* buffer entry from our array, so we just have to iterate till there
@@ -746,6 +766,7 @@ ResourceOwnerDelete(ResourceOwner owner)
/* And it better not own any resources, either */
Assert(owner->bufferarr.nitems == 0);
+ Assert(owner->bufferioarr.nitems == 0);
Assert(owner->catrefarr.nitems == 0);
Assert(owner->catlistrefarr.nitems == 0);
Assert(owner->relrefarr.nitems == 0);
@@ -775,6 +796,7 @@ ResourceOwnerDelete(ResourceOwner owner)
/* And free the object. */
ResourceArrayFree(&(owner->bufferarr));
+ ResourceArrayFree(&(owner->bufferioarr));
ResourceArrayFree(&(owner->catrefarr));
ResourceArrayFree(&(owner->catlistrefarr));
ResourceArrayFree(&(owner->relrefarr));
@@ -976,6 +998,44 @@ ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer)
buffer, owner->name);
}
+
+/*
+ * Make sure there is room for at least one more entry in a ResourceOwner's
+ * buffer array.
+ *
+ * This is separate from actually inserting an entry because if we run out
+ * of memory, it's critical to do so *before* acquiring the resource.
+ */
+void
+ResourceOwnerEnlargeBufferIOs(ResourceOwner owner)
+{
+ /* We used to allow pinning buffers without a resowner, but no more */
+ Assert(owner != NULL);
+ ResourceArrayEnlarge(&(owner->bufferioarr));
+}
+
+/*
+ * Remember that a buffer IO is owned by a ResourceOwner
+ *
+ * Caller must have previously done ResourceOwnerEnlargeBufferIOs()
+ */
+void
+ResourceOwnerRememberBufferIO(ResourceOwner owner, Buffer buffer)
+{
+ ResourceArrayAdd(&(owner->bufferioarr), BufferGetDatum(buffer));
+}
+
+/*
+ * Forget that a buffer IO is owned by a ResourceOwner
+ */
+void
+ResourceOwnerForgetBufferIO(ResourceOwner owner, Buffer buffer)
+{
+ if (!ResourceArrayRemove(&(owner->bufferioarr), BufferGetDatum(buffer)))
+ elog(PANIC, "buffer IO %d is not owned by resource owner %s",
+ buffer, owner->name);
+}
+
/*
* Remember that a Local Lock is owned by a ResourceOwner
*
--
2.38.0
--wwosng5vofi3dmcs
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0008-bufmgr-Move-relation-extension-handling-into-Exte.patch"
^ permalink raw reply [nested|flat] 8+ messages in thread
* [PATCH v1 07/12] bufmgr: Support multiple in-progress IOs by using resowner
@ 2022-10-24 23:44 Andres Freund <[email protected]>
0 siblings, 0 replies; 8+ messages in thread
From: Andres Freund @ 2022-10-24 23:44 UTC (permalink / raw)
---
src/include/storage/bufmgr.h | 2 +-
src/include/utils/resowner_private.h | 5 ++
src/backend/access/transam/xact.c | 4 +-
src/backend/postmaster/autovacuum.c | 1 -
src/backend/postmaster/bgwriter.c | 1 -
src/backend/postmaster/checkpointer.c | 1 -
src/backend/postmaster/walwriter.c | 1 -
src/backend/storage/buffer/bufmgr.c | 86 ++++++++++++---------------
src/backend/utils/resowner/resowner.c | 58 ++++++++++++++++++
9 files changed, 103 insertions(+), 56 deletions(-)
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index 2c1f12d3066..723243aeb96 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -167,7 +167,7 @@ extern bool ConditionalLockBufferForCleanup(Buffer buffer);
extern bool IsBufferCleanupOK(Buffer buffer);
extern bool HoldingBufferPinThatDelaysRecovery(void);
-extern void AbortBufferIO(void);
+extern void AbortBufferIO(Buffer buffer);
extern void BufmgrCommit(void);
extern bool BgBufferSync(struct WritebackContext *wb_context);
diff --git a/src/include/utils/resowner_private.h b/src/include/utils/resowner_private.h
index d01cccc27c1..61a72ed52f5 100644
--- a/src/include/utils/resowner_private.h
+++ b/src/include/utils/resowner_private.h
@@ -30,6 +30,11 @@ extern void ResourceOwnerEnlargeBuffers(ResourceOwner owner);
extern void ResourceOwnerRememberBuffer(ResourceOwner owner, Buffer buffer);
extern void ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer);
+/* support for IO-in-progress management */
+extern void ResourceOwnerEnlargeBufferIOs(ResourceOwner owner);
+extern void ResourceOwnerRememberBufferIO(ResourceOwner owner, Buffer buffer);
+extern void ResourceOwnerForgetBufferIO(ResourceOwner owner, Buffer buffer);
+
/* support for local lock management */
extern void ResourceOwnerRememberLock(ResourceOwner owner, LOCALLOCK *locallock);
extern void ResourceOwnerForgetLock(ResourceOwner owner, LOCALLOCK *locallock);
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index fd5103a78e2..71b298d38fe 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2707,8 +2707,7 @@ AbortTransaction(void)
pgstat_report_wait_end();
pgstat_progress_end_command();
- /* Clean up buffer I/O and buffer context locks, too */
- AbortBufferIO();
+ /* Clean up buffer context locks, too */
UnlockBuffers();
/* Reset WAL record construction state */
@@ -5051,7 +5050,6 @@ AbortSubTransaction(void)
pgstat_report_wait_end();
pgstat_progress_end_command();
- AbortBufferIO();
UnlockBuffers();
/* Reset WAL record construction state */
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 1e90b72b740..09b8986597a 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -526,7 +526,6 @@ AutoVacLauncherMain(int argc, char *argv[])
*/
LWLockReleaseAll();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
/* this is probably dead code, but let's be safe: */
if (AuxProcessResourceOwner)
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 91e6f6ea18a..372cf21464e 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -167,7 +167,6 @@ BackgroundWriterMain(void)
*/
LWLockReleaseAll();
ConditionVariableCancelSleep();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 5fc076fc149..a3954e7cc0e 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -271,7 +271,6 @@ CheckpointerMain(void)
LWLockReleaseAll();
ConditionVariableCancelSleep();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index beb46dcb55c..2bea4947c1b 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -163,7 +163,6 @@ WalWriterMain(void)
LWLockReleaseAll();
ConditionVariableCancelSleep();
pgstat_report_wait_end();
- AbortBufferIO();
UnlockBuffers();
ReleaseAuxProcessResources(false);
AtEOXact_Buffers(false);
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 50550a8a70e..b1404bd77dd 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -159,10 +159,6 @@ int checkpoint_flush_after = 0;
int bgwriter_flush_after = 0;
int backend_flush_after = 0;
-/* local state for StartBufferIO and related functions */
-static BufferDesc *InProgressBuf = NULL;
-static bool IsForInput;
-
/* local state for LockBufferForCleanup */
static BufferDesc *PinCountWaitBuf = NULL;
@@ -2689,7 +2685,6 @@ InitBufferPoolAccess(void)
static void
AtProcExit_Buffers(int code, Datum arg)
{
- AbortBufferIO();
UnlockBuffers();
CheckForBufferLeaks();
@@ -4618,7 +4613,7 @@ StartBufferIO(BufferDesc *buf, bool forInput)
{
uint32 buf_state;
- Assert(!InProgressBuf);
+ ResourceOwnerEnlargeBufferIOs(CurrentResourceOwner);
for (;;)
{
@@ -4642,8 +4637,8 @@ StartBufferIO(BufferDesc *buf, bool forInput)
buf_state |= BM_IO_IN_PROGRESS;
UnlockBufHdr(buf, buf_state);
- InProgressBuf = buf;
- IsForInput = forInput;
+ ResourceOwnerRememberBufferIO(CurrentResourceOwner,
+ BufferDescriptorGetBuffer(buf));
return true;
}
@@ -4669,8 +4664,6 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
{
uint32 buf_state;
- Assert(buf == InProgressBuf);
-
buf_state = LockBufHdr(buf);
Assert(buf_state & BM_IO_IN_PROGRESS);
@@ -4682,13 +4675,14 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
buf_state |= set_flag_bits;
UnlockBufHdr(buf, buf_state);
- InProgressBuf = NULL;
+ ResourceOwnerForgetBufferIO(CurrentResourceOwner,
+ BufferDescriptorGetBuffer(buf));
ConditionVariableBroadcast(BufferDescriptorGetIOCV(buf));
}
/*
- * AbortBufferIO: Clean up any active buffer I/O after an error.
+ * AbortBufferIO: Clean up active buffer I/O after an error.
*
* All LWLocks we might have held have been released,
* but we haven't yet released buffer pins, so the buffer is still pinned.
@@ -4697,46 +4691,42 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint32 set_flag_bits)
* possible the error condition wasn't related to the I/O.
*/
void
-AbortBufferIO(void)
+AbortBufferIO(Buffer buf)
{
- BufferDesc *buf = InProgressBuf;
+ BufferDesc *buf_hdr = GetBufferDescriptor(buf - 1);
+ uint32 buf_state;
- if (buf)
+ buf_state = LockBufHdr(buf_hdr);
+ Assert(buf_state & (BM_IO_IN_PROGRESS | BM_TAG_VALID));
+
+ if (!(buf_state & BM_VALID))
{
- uint32 buf_state;
-
- buf_state = LockBufHdr(buf);
- Assert(buf_state & BM_IO_IN_PROGRESS);
- if (IsForInput)
- {
- Assert(!(buf_state & BM_DIRTY));
-
- /* We'd better not think buffer is valid yet */
- Assert(!(buf_state & BM_VALID));
- UnlockBufHdr(buf, buf_state);
- }
- else
- {
- Assert(buf_state & BM_DIRTY);
- UnlockBufHdr(buf, buf_state);
- /* Issue notice if this is not the first failure... */
- if (buf_state & BM_IO_ERROR)
- {
- /* Buffer is pinned, so we can read tag without spinlock */
- char *path;
-
- path = relpathperm(BufTagGetRelFileLocator(&buf->tag),
- BufTagGetForkNum(&buf->tag));
- ereport(WARNING,
- (errcode(ERRCODE_IO_ERROR),
- errmsg("could not write block %u of %s",
- buf->tag.blockNum, path),
- errdetail("Multiple failures --- write error might be permanent.")));
- pfree(path);
- }
- }
- TerminateBufferIO(buf, false, BM_IO_ERROR);
+ Assert(!(buf_state & BM_DIRTY));
+ UnlockBufHdr(buf_hdr, buf_state);
}
+ else
+ {
+ Assert(!(buf_state & BM_DIRTY));
+ UnlockBufHdr(buf_hdr, buf_state);
+
+ /* Issue notice if this is not the first failure... */
+ if (buf_state & BM_IO_ERROR)
+ {
+ /* Buffer is pinned, so we can read tag without spinlock */
+ char *path;
+
+ path = relpathperm(BufTagGetRelFileLocator(&buf_hdr->tag),
+ BufTagGetForkNum(&buf_hdr->tag));
+ ereport(WARNING,
+ (errcode(ERRCODE_IO_ERROR),
+ errmsg("could not write block %u of %s",
+ buf_hdr->tag.blockNum, path),
+ errdetail("Multiple failures --- write error might be permanent.")));
+ pfree(path);
+ }
+ }
+
+ TerminateBufferIO(buf_hdr, false, BM_IO_ERROR);
}
/*
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index 37b43ee1f8c..62311831348 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -121,6 +121,7 @@ typedef struct ResourceOwnerData
/* We have built-in support for remembering: */
ResourceArray bufferarr; /* owned buffers */
+ ResourceArray bufferioarr; /* in-progress buffer IO */
ResourceArray catrefarr; /* catcache references */
ResourceArray catlistrefarr; /* catcache-list pins */
ResourceArray relrefarr; /* relcache references */
@@ -441,6 +442,7 @@ ResourceOwnerCreate(ResourceOwner parent, const char *name)
}
ResourceArrayInit(&(owner->bufferarr), BufferGetDatum(InvalidBuffer));
+ ResourceArrayInit(&(owner->bufferioarr), BufferGetDatum(InvalidBuffer));
ResourceArrayInit(&(owner->catrefarr), PointerGetDatum(NULL));
ResourceArrayInit(&(owner->catlistrefarr), PointerGetDatum(NULL));
ResourceArrayInit(&(owner->relrefarr), PointerGetDatum(NULL));
@@ -517,6 +519,24 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
if (phase == RESOURCE_RELEASE_BEFORE_LOCKS)
{
+ /*
+ * Abort failed buffer IO. AbortBufferIO()->TerminateBufferIO() calls
+ * ResourceOwnerForgetBufferIOs(), so we just have to iterate till
+ * there are none.
+ *
+ * Needs to be before we release buffer pins.
+ *
+ * During a commit, there shouldn't be any in-progress IO.
+ */
+ while (ResourceArrayGetAny(&(owner->bufferioarr), &foundres))
+ {
+ Buffer res = DatumGetBuffer(foundres);
+
+ if (isCommit)
+ elog(PANIC, "lost track of buffer IO on buffer %u", res);
+ AbortBufferIO(res);
+ }
+
/*
* Release buffer pins. Note that ReleaseBuffer will remove the
* buffer entry from our array, so we just have to iterate till there
@@ -976,6 +996,44 @@ ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer)
buffer, owner->name);
}
+
+/*
+ * Make sure there is room for at least one more entry in a ResourceOwner's
+ * buffer array.
+ *
+ * This is separate from actually inserting an entry because if we run out
+ * of memory, it's critical to do so *before* acquiring the resource.
+ */
+void
+ResourceOwnerEnlargeBufferIOs(ResourceOwner owner)
+{
+ /* We used to allow pinning buffers without a resowner, but no more */
+ Assert(owner != NULL);
+ ResourceArrayEnlarge(&(owner->bufferioarr));
+}
+
+/*
+ * Remember that a buffer IO is owned by a ResourceOwner
+ *
+ * Caller must have previously done ResourceOwnerEnlargeBufferIOs()
+ */
+void
+ResourceOwnerRememberBufferIO(ResourceOwner owner, Buffer buffer)
+{
+ ResourceArrayAdd(&(owner->bufferioarr), BufferGetDatum(buffer));
+}
+
+/*
+ * Forget that a buffer IO is owned by a ResourceOwner
+ */
+void
+ResourceOwnerForgetBufferIO(ResourceOwner owner, Buffer buffer)
+{
+ if (!ResourceArrayRemove(&(owner->bufferioarr), BufferGetDatum(buffer)))
+ elog(PANIC, "buffer IO %d is not owned by resource owner %s",
+ buffer, owner->name);
+}
+
/*
* Remember that a Local Lock is owned by a ResourceOwner
*
--
2.38.0
--pdbwq5asyrs2p56f
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0008-bufmgr-Move-relation-extension-handling-into-Bulk.patch"
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2022-10-24 23:44 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-14 22:12 [PATCH 09/10] Add zstd compression levels Justin Pryzby <[email protected]>
2022-10-24 23:44 [PATCH v7 06/14] bufmgr: Support multiple in-progress IOs by using resowner Andres Freund <[email protected]>
2022-10-24 23:44 [PATCH v5 08/15] bufmgr: Support multiple in-progress IOs by using resowner Andres Freund <[email protected]>
2022-10-24 23:44 [PATCH v6 10/17] bufmgr: Support multiple in-progress IOs by using resowner Andres Freund <[email protected]>
2022-10-24 23:44 [PATCH v5 07/14] bufmgr: Support multiple in-progress IOs by using resowner Andres Freund <[email protected]>
2022-10-24 23:44 [PATCH v1 07/12] bufmgr: Support multiple in-progress IOs by using resowner Andres Freund <[email protected]>
2022-10-24 23:44 [PATCH v4 08/15] bufmgr: Support multiple in-progress IOs by using resowner Andres Freund <[email protected]>
2022-10-24 23:44 [PATCH v2 06/14] bufmgr: Support multiple in-progress IOs by using resowner Andres Freund <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox