public inbox for [email protected]
help / color / mirror / Atom feedRework SLRU I/O errors handle
7+ messages / 2 participants
[nested] [flat]
* Rework SLRU I/O errors handle
@ 2026-02-26 14:26 Maxim Orlov <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Maxim Orlov @ 2026-02-26 14:26 UTC (permalink / raw)
To: Postgres hackers <[email protected]>
Hi!
Beginning of the discussion is here [0].
Historically, the SLRU module was designed to handle 32-bit
transactions. However, it is now utilised for handling a variety of
object types, like TransactionId, MultixactId, MultiXactOffset,
QueuePosition, and so on. But the IO error reporting system is still
designed to support 32-bit XIDs exclusively.
The proposed patchset allows us to define a "custom" callback to
improve error messages.
The first two commits add a callback and test case. The subsequent ones
improve I/O error messages. The last one adds the XID epoch to the error
message. It's purely optional, but I think it would be useful.
[0]
https://www.postgresql.org/message-id/CACG%3Dezbwy1zargXDNPeYXxZwRW3jXu_aD%3DrcG-7dc4fw7Y9Ojw%40mail...
--
Best regards,
Maxim Orlov.
Attachments:
[application/octet-stream] v3-0003-Use-custom-SLRU-IO-error-msg-for-an-asynchronous-.patch (3.1K, 3-v3-0003-Use-custom-SLRU-IO-error-msg-for-an-asynchronous-.patch)
download | inline diff:
From 4433671e0b338412216d21ac2572f51fcb07a4b2 Mon Sep 17 00:00:00 2001
From: Maxim Orlov <[email protected]>
Date: Wed, 25 Feb 2026 18:04:13 +0300
Subject: [PATCH v3 3/5] Use custom SLRU IO error msg for an asynchronous
notification
---
src/backend/commands/async.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 8afd1315a9c..254fdc398ce 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -569,6 +569,7 @@ bool Trace_notify = false;
int max_notify_queue_pages = 1048576;
/* local function prototypes */
+static inline int asyncQueueErrmsgForIoError(const void *opaque_data);
static inline int64 asyncQueuePageDiff(int64 p, int64 q);
static inline bool asyncQueuePagePrecedes(int64 p, int64 q);
static inline void GlobalChannelKeyInit(GlobalChannelKey *key, Oid dboid,
@@ -609,6 +610,17 @@ static uint32 notification_hash(const void *key, Size keysize);
static int notification_match(const void *key1, const void *key2, Size keysize);
static void ClearPendingActionsAndNotifies(void);
+static inline int
+asyncQueueErrmsgForIoError(const void *opaque_data)
+{
+ const QueuePosition *position = opaque_data;
+
+ Assert(position != NULL);
+
+ return errmsg("could not access status of async queue position (page=%" PRId64", offset=%d)",
+ position->page, position->offset);
+}
+
/*
* Compute the difference between two queue page numbers.
* Previously this function accounted for a wraparound.
@@ -829,7 +841,7 @@ AsyncShmemInit(void)
* names are used in order to avoid wraparound.
*/
NotifyCtl->PagePrecedes = asyncQueuePagePrecedes;
- NotifyCtl->errmsg_for_io_error = xact_errmsg_for_io_error;
+ NotifyCtl->errmsg_for_io_error = asyncQueueErrmsgForIoError;
SimpleLruInit(NotifyCtl, "notify", notify_buffers, 0,
"pg_notify", LWTRANCHE_NOTIFY_BUFFER, LWTRANCHE_NOTIFY_SLRU,
SYNC_HANDLER_NONE, true);
@@ -2068,7 +2080,7 @@ asyncQueueAddEntries(ListCell *nextNotify)
if (QUEUE_POS_IS_ZERO(queue_head))
slotno = SimpleLruZeroPage(NotifyCtl, pageno);
else
- slotno = SimpleLruReadPage(NotifyCtl, pageno, true, NULL);
+ slotno = SimpleLruReadPage(NotifyCtl, pageno, true, &queue_head);
/* Note we mark the page dirty before writing in it */
NotifyCtl->shared->page_dirty[slotno] = true;
@@ -2738,7 +2750,7 @@ asyncQueueProcessPageEntries(QueuePosition *current,
alignas(AsyncQueueEntry) char local_buf[QUEUE_PAGESIZE];
char *local_buf_end = local_buf;
- slotno = SimpleLruReadPage_ReadOnly(NotifyCtl, curpage, NULL);
+ slotno = SimpleLruReadPage_ReadOnly(NotifyCtl, curpage, current);
page_buffer = NotifyCtl->shared->page_buffer[slotno];
do
@@ -2996,7 +3008,7 @@ AsyncNotifyFreezeXids(TransactionId newFrozenXid)
lock = SimpleLruGetBankLock(NotifyCtl, pageno);
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(NotifyCtl, pageno, true, NULL);
+ slotno = SimpleLruReadPage(NotifyCtl, pageno, true, &pos);
page_buffer = NotifyCtl->shared->page_buffer[slotno];
curpage = pageno;
}
--
2.43.0
[application/octet-stream] v3-0005-Avoid-misleading-user-about-status-of-InvalidTran.patch (1.3K, 4-v3-0005-Avoid-misleading-user-about-status-of-InvalidTran.patch)
download | inline diff:
From bf50b3db60cc6c9603b5c6ff26ea2d8a79d9965d Mon Sep 17 00:00:00 2001
From: Maxim Orlov <[email protected]>
Date: Wed, 25 Feb 2026 18:20:32 +0300
Subject: [PATCH v3 5/5] Avoid misleading user about status of
InvalidTransactionId
In some cases, we use the access SLRU page without specifying the XID.
If an error occurs, you may receive a message about the inability to
obtain status of transaction 0, even though the page appears to be sane.
To avoid this, use a more general formulation in case XID is invalid.
---
src/include/access/slru.h | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 1e0beb26628..78ee36c05a6 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -166,10 +166,11 @@ typedef SlruCtlData *SlruCtl;
static inline int
xact_errmsg_for_io_error(const void *opaque_data)
{
- TransactionId xid = opaque_data ? (*(TransactionId *) opaque_data) :
- InvalidTransactionId;
+ if (opaque_data)
+ return errmsg("could not access status of transaction %u",
+ *(TransactionId *) opaque_data);
- return errmsg("could not access status of transaction %u", xid);
+ return errmsg("could not access slru entry"); /* InvalidTransactionId */
}
/*
--
2.43.0
[application/octet-stream] v3-0001-Add-a-callback-for-generating-an-I-O-message-in-t.patch (21.7K, 5-v3-0001-Add-a-callback-for-generating-an-I-O-message-in-t.patch)
download | inline diff:
From 4964b19d495e94050efd37454eff7dc89145a748 Mon Sep 17 00:00:00 2001
From: Maxim Orlov <[email protected]>
Date: Wed, 25 Feb 2026 16:59:38 +0300
Subject: [PATCH v3 1/5] Add a callback for generating an I/O message in the
SLRU
Historically, the SLRU module was designed to work with transaction IDs.
But now we use it to work with different objects, and even of different
types. However, I/O errors continued to be output in the
corresponding XIDs format.
This commit adds a callback that will allow to create custom IO error
messages for modules that don't work with transaction IDs.
No user-visible behavior change is expected in this commit.
---
src/backend/access/transam/clog.c | 8 +++---
src/backend/access/transam/commit_ts.c | 5 ++--
src/backend/access/transam/multixact.c | 21 ++++++++------
src/backend/access/transam/slru.c | 38 +++++++++++++++-----------
src/backend/access/transam/subtrans.c | 5 ++--
src/backend/commands/async.c | 10 +++----
src/backend/storage/lmgr/predicate.c | 5 ++--
src/include/access/slru.h | 26 ++++++++++++++++--
src/test/modules/test_slru/test_slru.c | 5 ++--
9 files changed, 78 insertions(+), 45 deletions(-)
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index b5c38bbb162..899fa7b41e1 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -381,8 +381,7 @@ TransactionIdSetPageStatusInternal(TransactionId xid, int nsubxids,
* write-busy, since we don't care if the update reaches disk sooner than
* we think.
*/
- slotno = SimpleLruReadPage(XactCtl, pageno, !XLogRecPtrIsValid(lsn),
- xid);
+ slotno = SimpleLruReadPage(XactCtl, pageno, !XLogRecPtrIsValid(lsn), &xid);
/*
* Set the main transaction id, if any.
@@ -743,7 +742,7 @@ TransactionIdGetStatus(TransactionId xid, XLogRecPtr *lsn)
/* lock is acquired by SimpleLruReadPage_ReadOnly */
- slotno = SimpleLruReadPage_ReadOnly(XactCtl, pageno, xid);
+ slotno = SimpleLruReadPage_ReadOnly(XactCtl, pageno, &xid);
byteptr = XactCtl->shared->page_buffer[slotno] + byteno;
status = (*byteptr >> bshift) & CLOG_XACT_BITMASK;
@@ -807,6 +806,7 @@ CLOGShmemInit(void)
Assert(transaction_buffers != 0);
XactCtl->PagePrecedes = CLOGPagePrecedes;
+ XactCtl->errmsg_for_io_error = xact_errmsg_for_io_error;
SimpleLruInit(XactCtl, "transaction", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE,
"pg_xact", LWTRANCHE_XACT_BUFFER,
LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG, false);
@@ -882,7 +882,7 @@ TrimCLOG(void)
int slotno;
char *byteptr;
- slotno = SimpleLruReadPage(XactCtl, pageno, false, xid);
+ slotno = SimpleLruReadPage(XactCtl, pageno, false, &xid);
byteptr = XactCtl->shared->page_buffer[slotno] + byteno;
/* Zero so-far-unused positions in the current byte */
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 6fa2178f1dd..9563e87d9b2 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -227,7 +227,7 @@ SetXidCommitTsInPage(TransactionId xid, int nsubxids,
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(CommitTsCtl, pageno, true, xid);
+ slotno = SimpleLruReadPage(CommitTsCtl, pageno, true, &xid);
TransactionIdSetCommitTs(xid, ts, nodeid, slotno);
for (i = 0; i < nsubxids; i++)
@@ -332,7 +332,7 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts,
}
/* lock is acquired by SimpleLruReadPage_ReadOnly */
- slotno = SimpleLruReadPage_ReadOnly(CommitTsCtl, pageno, xid);
+ slotno = SimpleLruReadPage_ReadOnly(CommitTsCtl, pageno, &xid);
memcpy(&entry,
CommitTsCtl->shared->page_buffer[slotno] +
SizeOfCommitTimestampEntry * entryno,
@@ -551,6 +551,7 @@ CommitTsShmemInit(void)
Assert(commit_timestamp_buffers != 0);
CommitTsCtl->PagePrecedes = CommitTsPagePrecedes;
+ CommitTsCtl->errmsg_for_io_error = xact_errmsg_for_io_error;
SimpleLruInit(CommitTsCtl, "commit_timestamp", CommitTsShmemBuffers(), 0,
"pg_commit_ts", LWTRANCHE_COMMITTS_BUFFER,
LWTRANCHE_COMMITTS_SLRU,
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 90ec87d9dd6..816fb50fa4b 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -798,7 +798,7 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
* enough that a MultiXactId is really involved. Perhaps someday we'll
* take the trouble to generalize the slru.c error reporting code.
*/
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, multi);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, &multi);
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
offptr += entryno;
@@ -827,7 +827,7 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
lock = SimpleLruGetBankLock(MultiXactOffsetCtl, next_pageno);
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, next_pageno, true, next);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, next_pageno, true, &next);
next_offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
next_offptr += next_entryno;
}
@@ -881,7 +881,7 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
LWLockAcquire(lock, LW_EXCLUSIVE);
prevlock = lock;
}
- slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, multi);
+ slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, &multi);
prev_pageno = pageno;
}
@@ -1206,7 +1206,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
LWLockAcquire(lock, LW_EXCLUSIVE);
/* read this multi's offset */
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, multi);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, &multi);
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
offptr += entryno;
offset = *offptr;
@@ -1244,7 +1244,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
LWLockAcquire(newlock, LW_EXCLUSIVE);
lock = newlock;
}
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, tmpMXact);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, &tmpMXact);
}
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
@@ -1309,7 +1309,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
lock = newlock;
}
- slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, multi);
+ slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, &multi);
prev_pageno = pageno;
}
@@ -1730,6 +1730,9 @@ MultiXactShmemInit(void)
MultiXactOffsetCtl->PagePrecedes = MultiXactOffsetPagePrecedes;
MultiXactMemberCtl->PagePrecedes = MultiXactMemberPagePrecedes;
+ MultiXactOffsetCtl->errmsg_for_io_error = xact_errmsg_for_io_error;
+ MultiXactMemberCtl->errmsg_for_io_error = xact_errmsg_for_io_error;
+
SimpleLruInit(MultiXactOffsetCtl,
"multixact_offset", multixact_offset_buffers, 0,
"pg_multixact/offsets", LWTRANCHE_MULTIXACTOFFSET_BUFFER,
@@ -1879,7 +1882,7 @@ TrimMultiXact(void)
if (entryno == 0 || nextMXact == FirstMultiXactId)
slotno = SimpleLruZeroPage(MultiXactOffsetCtl, pageno);
else
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, nextMXact);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, &nextMXact);
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
offptr += entryno;
@@ -1914,7 +1917,7 @@ TrimMultiXact(void)
LWLockAcquire(lock, LW_EXCLUSIVE);
memberoff = MXOffsetToMemberOffset(offset);
- slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, offset);
+ slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, &offset);
xidptr = (TransactionId *)
(MultiXactMemberCtl->shared->page_buffer[slotno] + memberoff);
@@ -2444,7 +2447,7 @@ find_multixact_start(MultiXactId multi, MultiXactOffset *result)
return false;
/* lock is acquired by SimpleLruReadPage_ReadOnly */
- slotno = SimpleLruReadPage_ReadOnly(MultiXactOffsetCtl, pageno, multi);
+ slotno = SimpleLruReadPage_ReadOnly(MultiXactOffsetCtl, pageno, &multi);
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
offptr += entryno;
offset = *offptr;
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 549c7e3e64b..0b5bb11a18d 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -181,7 +181,8 @@ static void SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata);
static bool SlruPhysicalReadPage(SlruCtl ctl, int64 pageno, int slotno);
static bool SlruPhysicalWritePage(SlruCtl ctl, int64 pageno, int slotno,
SlruWriteAll fdata);
-static void SlruReportIOError(SlruCtl ctl, int64 pageno, TransactionId xid);
+static void SlruReportIOError(SlruCtl ctl, int64 pageno,
+ const void *opaque_data);
static int SlruSelectLRUPage(SlruCtl ctl, int64 pageno);
static bool SlruScanDirCbDeleteCutoff(SlruCtl ctl, char *filename,
@@ -257,6 +258,10 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
bool found;
int nbanks = nslots / SLRU_BANK_SIZE;
+ /* Make sure callbacks are set up */
+ Assert(ctl->PagePrecedes != NULL);
+ Assert(ctl->errmsg_for_io_error != NULL);
+
Assert(nslots <= SLRU_MAX_ALLOWED_BUFFERS);
shared = (SlruShared) ShmemInitStruct(name,
@@ -525,7 +530,7 @@ SimpleLruWaitIO(SlruCtl ctl, int slotno)
*/
int
SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
- TransactionId xid)
+ const void *opaque_data)
{
SlruShared shared = ctl->shared;
LWLock *banklock = SimpleLruGetBankLock(ctl, pageno);
@@ -601,7 +606,7 @@ SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
/* Now it's okay to ereport if we failed */
if (!ok)
- SlruReportIOError(ctl, pageno, xid);
+ SlruReportIOError(ctl, pageno, opaque_data);
SlruRecentlyUsed(shared, slotno);
@@ -627,7 +632,7 @@ SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
* It is unspecified whether the lock will be shared or exclusive.
*/
int
-SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno, TransactionId xid)
+SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno, const void *opaque_data)
{
SlruShared shared = ctl->shared;
LWLock *banklock = SimpleLruGetBankLock(ctl, pageno);
@@ -659,7 +664,7 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno, TransactionId xid)
LWLockRelease(banklock);
LWLockAcquire(banklock, LW_EXCLUSIVE);
- return SimpleLruReadPage(ctl, pageno, true, xid);
+ return SimpleLruReadPage(ctl, pageno, true, opaque_data);
}
/*
@@ -682,6 +687,7 @@ SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata)
bool ok;
Assert(shared->page_status[slotno] != SLRU_PAGE_EMPTY);
+
Assert(LWLockHeldByMeInMode(SimpleLruGetBankLock(ctl, pageno), LW_EXCLUSIVE));
/* If a write is in progress, wait for it to finish */
@@ -739,7 +745,7 @@ SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata)
/* Now it's okay to ereport if we failed */
if (!ok)
- SlruReportIOError(ctl, pageno, InvalidTransactionId);
+ SlruReportIOError(ctl, pageno, NULL);
/* If part of a checkpoint, count this as a SLRU buffer written. */
if (fdata)
@@ -1070,7 +1076,7 @@ SlruPhysicalWritePage(SlruCtl ctl, int64 pageno, int slotno, SlruWriteAll fdata)
* SlruPhysicalWritePage. Call this after cleaning up shared-memory state.
*/
static void
-SlruReportIOError(SlruCtl ctl, int64 pageno, TransactionId xid)
+SlruReportIOError(SlruCtl ctl, int64 pageno, const void *opaque_data)
{
int64 segno = pageno / SLRU_PAGES_PER_SEGMENT;
int rpageno = pageno % SLRU_PAGES_PER_SEGMENT;
@@ -1084,13 +1090,13 @@ SlruReportIOError(SlruCtl ctl, int64 pageno, TransactionId xid)
case SLRU_OPEN_FAILED:
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
+ ctl->errmsg_for_io_error(opaque_data),
errdetail("Could not open file \"%s\": %m.", path)));
break;
case SLRU_SEEK_FAILED:
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
+ ctl->errmsg_for_io_error(opaque_data),
errdetail("Could not seek in file \"%s\" to offset %d: %m.",
path, offset)));
break;
@@ -1098,38 +1104,38 @@ SlruReportIOError(SlruCtl ctl, int64 pageno, TransactionId xid)
if (errno)
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
+ ctl->errmsg_for_io_error(opaque_data),
errdetail("Could not read from file \"%s\" at offset %d: %m.",
path, offset)));
else
ereport(ERROR,
- (errmsg("could not access status of transaction %u", xid),
+ (ctl->errmsg_for_io_error(opaque_data),
errdetail("Could not read from file \"%s\" at offset %d: read too few bytes.", path, offset)));
break;
case SLRU_WRITE_FAILED:
if (errno)
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
+ ctl->errmsg_for_io_error(opaque_data),
errdetail("Could not write to file \"%s\" at offset %d: %m.",
path, offset)));
else
ereport(ERROR,
- (errmsg("could not access status of transaction %u", xid),
+ (ctl->errmsg_for_io_error(opaque_data),
errdetail("Could not write to file \"%s\" at offset %d: wrote too few bytes.",
path, offset)));
break;
case SLRU_FSYNC_FAILED:
ereport(data_sync_elevel(ERROR),
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
+ ctl->errmsg_for_io_error(opaque_data),
errdetail("Could not fsync file \"%s\": %m.",
path)));
break;
case SLRU_CLOSE_FAILED:
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
+ ctl->errmsg_for_io_error(opaque_data),
errdetail("Could not close file \"%s\": %m.",
path)));
break;
@@ -1411,7 +1417,7 @@ SimpleLruWriteAll(SlruCtl ctl, bool allow_redirtied)
}
}
if (!ok)
- SlruReportIOError(ctl, pageno, InvalidTransactionId);
+ SlruReportIOError(ctl, pageno, NULL);
/* Ensure that directory entries for new files are on disk. */
if (ctl->sync_handler != SYNC_HANDLER_NONE)
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index c0987f43f11..18b7da7fca1 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -95,7 +95,7 @@ SubTransSetParent(TransactionId xid, TransactionId parent)
lock = SimpleLruGetBankLock(SubTransCtl, pageno);
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(SubTransCtl, pageno, true, xid);
+ slotno = SimpleLruReadPage(SubTransCtl, pageno, true, &xid);
ptr = (TransactionId *) SubTransCtl->shared->page_buffer[slotno];
ptr += entryno;
@@ -135,7 +135,7 @@ SubTransGetParent(TransactionId xid)
/* lock is acquired by SimpleLruReadPage_ReadOnly */
- slotno = SimpleLruReadPage_ReadOnly(SubTransCtl, pageno, xid);
+ slotno = SimpleLruReadPage_ReadOnly(SubTransCtl, pageno, &xid);
ptr = (TransactionId *) SubTransCtl->shared->page_buffer[slotno];
ptr += entryno;
@@ -240,6 +240,7 @@ SUBTRANSShmemInit(void)
Assert(subtransaction_buffers != 0);
SubTransCtl->PagePrecedes = SubTransPagePrecedes;
+ SubTransCtl->errmsg_for_io_error = xact_errmsg_for_io_error;
SimpleLruInit(SubTransCtl, "subtransaction", SUBTRANSShmemBuffers(), 0,
"pg_subtrans", LWTRANCHE_SUBTRANS_BUFFER,
LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE, false);
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 657c591618d..8afd1315a9c 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -829,6 +829,7 @@ AsyncShmemInit(void)
* names are used in order to avoid wraparound.
*/
NotifyCtl->PagePrecedes = asyncQueuePagePrecedes;
+ NotifyCtl->errmsg_for_io_error = xact_errmsg_for_io_error;
SimpleLruInit(NotifyCtl, "notify", notify_buffers, 0,
"pg_notify", LWTRANCHE_NOTIFY_BUFFER, LWTRANCHE_NOTIFY_SLRU,
SYNC_HANDLER_NONE, true);
@@ -2067,8 +2068,7 @@ asyncQueueAddEntries(ListCell *nextNotify)
if (QUEUE_POS_IS_ZERO(queue_head))
slotno = SimpleLruZeroPage(NotifyCtl, pageno);
else
- slotno = SimpleLruReadPage(NotifyCtl, pageno, true,
- InvalidTransactionId);
+ slotno = SimpleLruReadPage(NotifyCtl, pageno, true, NULL);
/* Note we mark the page dirty before writing in it */
NotifyCtl->shared->page_dirty[slotno] = true;
@@ -2738,8 +2738,7 @@ asyncQueueProcessPageEntries(QueuePosition *current,
alignas(AsyncQueueEntry) char local_buf[QUEUE_PAGESIZE];
char *local_buf_end = local_buf;
- slotno = SimpleLruReadPage_ReadOnly(NotifyCtl, curpage,
- InvalidTransactionId);
+ slotno = SimpleLruReadPage_ReadOnly(NotifyCtl, curpage, NULL);
page_buffer = NotifyCtl->shared->page_buffer[slotno];
do
@@ -2997,8 +2996,7 @@ AsyncNotifyFreezeXids(TransactionId newFrozenXid)
lock = SimpleLruGetBankLock(NotifyCtl, pageno);
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(NotifyCtl, pageno, true,
- InvalidTransactionId);
+ slotno = SimpleLruReadPage(NotifyCtl, pageno, true, NULL);
page_buffer = NotifyCtl->shared->page_buffer[slotno];
curpage = pageno;
}
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index fe75ead3501..66eb1c9d6b1 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -811,6 +811,7 @@ SerialInit(void)
* Set up SLRU management of the pg_serial data.
*/
SerialSlruCtl->PagePrecedes = SerialPagePrecedesLogically;
+ SerialSlruCtl->errmsg_for_io_error = xact_errmsg_for_io_error;
SimpleLruInit(SerialSlruCtl, "serializable",
serializable_buffers, 0, "pg_serial",
LWTRANCHE_SERIAL_BUFFER, LWTRANCHE_SERIAL_SLRU,
@@ -930,7 +931,7 @@ SerialAdd(TransactionId xid, SerCommitSeqNo minConflictCommitSeqNo)
else
{
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(SerialSlruCtl, targetPage, true, xid);
+ slotno = SimpleLruReadPage(SerialSlruCtl, targetPage, true, &xid);
}
SerialValue(slotno, xid) = minConflictCommitSeqNo;
@@ -974,7 +975,7 @@ SerialGetMinConflictCommitSeqNo(TransactionId xid)
* but will return with that lock held, which must then be released.
*/
slotno = SimpleLruReadPage_ReadOnly(SerialSlruCtl,
- SerialPage(xid), xid);
+ SerialPage(xid), &xid);
val = SerialValue(slotno, xid);
LWLockRelease(SimpleLruGetBankLock(SerialSlruCtl, SerialPage(xid)));
return val;
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 4cb8f478fce..1e0beb26628 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -13,6 +13,7 @@
#ifndef SLRU_H
#define SLRU_H
+#include "access/transam.h"
#include "access/xlogdefs.h"
#include "storage/lwlock.h"
#include "storage/sync.h"
@@ -146,10 +147,31 @@ typedef struct SlruCtlData
* it's always the same, it doesn't need to be in shared memory.
*/
char Dir[64];
+
+ /*
+ * Callback for creating an I/O error message.
+ *
+ * The opaque_data argument here is the same one that is passed to the
+ * SimpleLruReadPage* calls.
+ */
+ int (*errmsg_for_io_error)(const void *opaque_data);
} SlruCtlData;
typedef SlruCtlData *SlruCtl;
+/*
+ * Historically, this module was designed for handling transaction IDs,
+ * therefore this is the most common use case. Thus, make it publicly available.
+ */
+static inline int
+xact_errmsg_for_io_error(const void *opaque_data)
+{
+ TransactionId xid = opaque_data ? (*(TransactionId *) opaque_data) :
+ InvalidTransactionId;
+
+ return errmsg("could not access status of transaction %u", xid);
+}
+
/*
* Get the SLRU bank lock for given SlruCtl and the pageno.
*
@@ -174,9 +196,9 @@ extern void SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
extern int SimpleLruZeroPage(SlruCtl ctl, int64 pageno);
extern void SimpleLruZeroAndWritePage(SlruCtl ctl, int64 pageno);
extern int SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
- TransactionId xid);
+ const void *opaque_data);
extern int SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno,
- TransactionId xid);
+ const void *opaque_data);
extern void SimpleLruWritePage(SlruCtl ctl, int slotno);
extern void SimpleLruWriteAll(SlruCtl ctl, bool allow_redirtied);
#ifdef USE_ASSERT_CHECKING
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 4dc74e19620..a19129c366b 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -100,7 +100,7 @@ test_slru_page_read(PG_FUNCTION_ARGS)
/* find page in buffers, reading it if necessary */
LWLockAcquire(lock, LW_EXCLUSIVE);
slotno = SimpleLruReadPage(TestSlruCtl, pageno,
- write_ok, InvalidTransactionId);
+ write_ok, NULL);
data = (char *) TestSlruCtl->shared->page_buffer[slotno];
LWLockRelease(lock);
@@ -118,7 +118,7 @@ test_slru_page_readonly(PG_FUNCTION_ARGS)
/* find page in buffers, reading it if necessary */
slotno = SimpleLruReadPage_ReadOnly(TestSlruCtl,
pageno,
- InvalidTransactionId);
+ NULL);
Assert(LWLockHeldByMe(lock));
data = (char *) TestSlruCtl->shared->page_buffer[slotno];
LWLockRelease(lock);
@@ -245,6 +245,7 @@ test_slru_shmem_startup(void)
}
TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically;
+ TestSlruCtl->errmsg_for_io_error = xact_errmsg_for_io_error;
SimpleLruInit(TestSlruCtl, "TestSLRU",
NUM_TEST_BUFFERS, 0, slru_dir_name,
test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE,
--
2.43.0
[application/octet-stream] v3-0002-Add-test-case-for-custom-SLRU-IO-error.patch (4.5K, 6-v3-0002-Add-test-case-for-custom-SLRU-IO-error.patch)
download | inline diff:
From a86db1088a104d8ecf29fc5e762f5949c5d2e57c Mon Sep 17 00:00:00 2001
From: Maxim Orlov <[email protected]>
Date: Wed, 25 Feb 2026 17:48:42 +0300
Subject: [PATCH v3 2/5] Add test case for custom SLRU IO error
---
src/test/modules/test_slru/expected/test_slru.out | 7 +++++++
src/test/modules/test_slru/sql/test_slru.sql | 4 ++++
src/test/modules/test_slru/test_slru--1.0.sql | 2 +-
src/test/modules/test_slru/test_slru.c | 15 +++++++++++++--
4 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/src/test/modules/test_slru/expected/test_slru.out b/src/test/modules/test_slru/expected/test_slru.out
index 185c56e5d62..0dda6b60f0b 100644
--- a/src/test/modules/test_slru/expected/test_slru.out
+++ b/src/test/modules/test_slru/expected/test_slru.out
@@ -23,6 +23,13 @@ SELECT test_slru_page_exists(12345);
t
(1 row)
+-- should fail with custom error msg
+SELECT test_slru_page_read(54321);
+ERROR: could not access test_slru entry
+DETAIL: Could not open file "pg_test_slru/0000000000006A1": No such file or directory.
+SELECT test_slru_page_read(54321, false, '123'::xid);
+ERROR: could not access test_slru entry 123
+DETAIL: Could not open file "pg_test_slru/0000000000006A1": No such file or directory.
-- 48 extra pages
SELECT count(test_slru_page_write(a, 'Test SLRU'))
FROM generate_series(12346, 12393, 1) as a;
diff --git a/src/test/modules/test_slru/sql/test_slru.sql b/src/test/modules/test_slru/sql/test_slru.sql
index b1b376581ab..4f66f4207b7 100644
--- a/src/test/modules/test_slru/sql/test_slru.sql
+++ b/src/test/modules/test_slru/sql/test_slru.sql
@@ -5,6 +5,10 @@ SELECT test_slru_page_write(12345, 'Test SLRU');
SELECT test_slru_page_read(12345);
SELECT test_slru_page_exists(12345);
+-- should fail with custom error msg
+SELECT test_slru_page_read(54321);
+SELECT test_slru_page_read(54321, false, '123'::xid);
+
-- 48 extra pages
SELECT count(test_slru_page_write(a, 'Test SLRU'))
FROM generate_series(12346, 12393, 1) as a;
diff --git a/src/test/modules/test_slru/test_slru--1.0.sql b/src/test/modules/test_slru/test_slru--1.0.sql
index abecb5e2183..22f4f64b988 100644
--- a/src/test/modules/test_slru/test_slru--1.0.sql
+++ b/src/test/modules/test_slru/test_slru--1.0.sql
@@ -7,7 +7,7 @@ CREATE OR REPLACE FUNCTION test_slru_page_writeall() RETURNS VOID
AS 'MODULE_PATHNAME', 'test_slru_page_writeall' LANGUAGE C;
CREATE OR REPLACE FUNCTION test_slru_page_sync(bigint) RETURNS VOID
AS 'MODULE_PATHNAME', 'test_slru_page_sync' LANGUAGE C;
-CREATE OR REPLACE FUNCTION test_slru_page_read(bigint, bool DEFAULT true) RETURNS text
+CREATE OR REPLACE FUNCTION test_slru_page_read(bigint, bool DEFAULT true, xid DEFAULT NULL) RETURNS text
AS 'MODULE_PATHNAME', 'test_slru_page_read' LANGUAGE C;
CREATE OR REPLACE FUNCTION test_slru_page_readonly(bigint) RETURNS text
AS 'MODULE_PATHNAME', 'test_slru_page_readonly' LANGUAGE C;
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index a19129c366b..9ecf74882ff 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -93,6 +93,7 @@ test_slru_page_read(PG_FUNCTION_ARGS)
{
int64 pageno = PG_GETARG_INT64(0);
bool write_ok = PG_GETARG_BOOL(1);
+ TransactionId xid = PG_GETARG_TRANSACTIONID(2);
char *data = NULL;
int slotno;
LWLock *lock = SimpleLruGetBankLock(TestSlruCtl, pageno);
@@ -100,7 +101,7 @@ test_slru_page_read(PG_FUNCTION_ARGS)
/* find page in buffers, reading it if necessary */
LWLockAcquire(lock, LW_EXCLUSIVE);
slotno = SimpleLruReadPage(TestSlruCtl, pageno,
- write_ok, NULL);
+ write_ok, PG_ARGISNULL(2) ? NULL : &xid);
data = (char *) TestSlruCtl->shared->page_buffer[slotno];
LWLockRelease(lock);
@@ -210,6 +211,16 @@ test_slru_page_precedes_logically(int64 page1, int64 page2)
return page1 < page2;
}
+static inline int
+test_slru_errmsg_for_io_error(const void *opaque_data)
+{
+ if (opaque_data)
+ return errmsg("could not access test_slru entry %u",
+ *(TransactionId *) opaque_data);
+
+ return errmsg("could not access test_slru entry");
+}
+
static void
test_slru_shmem_startup(void)
{
@@ -245,7 +256,7 @@ test_slru_shmem_startup(void)
}
TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically;
- TestSlruCtl->errmsg_for_io_error = xact_errmsg_for_io_error;
+ TestSlruCtl->errmsg_for_io_error = test_slru_errmsg_for_io_error;
SimpleLruInit(TestSlruCtl, "TestSLRU",
NUM_TEST_BUFFERS, 0, slru_dir_name,
test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE,
--
2.43.0
[application/octet-stream] v3-0004-Use-custom-SLRU-IO-error-msg-for-multixact.patch (3.0K, 7-v3-0004-Use-custom-SLRU-IO-error-msg-for-multixact.patch)
download | inline diff:
From b94b2c17e07e5b9c8df1510da0f687ca2b52a5c3 Mon Sep 17 00:00:00 2001
From: Maxim Orlov <[email protected]>
Date: Wed, 25 Feb 2026 18:20:01 +0300
Subject: [PATCH v3 4/5] Use custom SLRU IO error msg for multixact
---
src/backend/access/transam/multixact.c | 36 +++++++++++++++++++++++---
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 816fb50fa4b..b78cd5fe41e 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -277,6 +277,8 @@ static void mXactCachePut(MultiXactId multi, int nmembers,
/* management of SLRU infrastructure */
static bool MultiXactOffsetPagePrecedes(int64 page1, int64 page2);
static bool MultiXactMemberPagePrecedes(int64 page1, int64 page2);
+static inline int MultiXactOffsetIoErrorMsg(const void *opaque_data);
+static inline int MultiXactMemberIoErrorMsg(const void *opaque_data);
static void ExtendMultiXactOffset(MultiXactId multi);
static void ExtendMultiXactMember(MultiXactOffset offset, int nmembers);
static void SetOldestOffset(void);
@@ -881,7 +883,8 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
LWLockAcquire(lock, LW_EXCLUSIVE);
prevlock = lock;
}
- slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, &multi);
+ slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true,
+ &offset);
prev_pageno = pageno;
}
@@ -1309,7 +1312,8 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
lock = newlock;
}
- slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, &multi);
+ slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true,
+ &offset);
prev_pageno = pageno;
}
@@ -1730,8 +1734,8 @@ MultiXactShmemInit(void)
MultiXactOffsetCtl->PagePrecedes = MultiXactOffsetPagePrecedes;
MultiXactMemberCtl->PagePrecedes = MultiXactMemberPagePrecedes;
- MultiXactOffsetCtl->errmsg_for_io_error = xact_errmsg_for_io_error;
- MultiXactMemberCtl->errmsg_for_io_error = xact_errmsg_for_io_error;
+ MultiXactOffsetCtl->errmsg_for_io_error = MultiXactOffsetIoErrorMsg;
+ MultiXactMemberCtl->errmsg_for_io_error = MultiXactMemberIoErrorMsg;
SimpleLruInit(MultiXactOffsetCtl,
"multixact_offset", multixact_offset_buffers, 0,
@@ -2758,6 +2762,30 @@ MultiXactMemberPagePrecedes(int64 page1, int64 page2)
return page1 < page2;
}
+/*
+ * Custom IO errmsg for MultiXactOffset.
+ */
+static inline int
+MultiXactOffsetIoErrorMsg(const void *opaque_data)
+{
+ Assert(opaque_data != NULL);
+
+ return errmsg("could not access status of multixact offset %u",
+ *(MultiXactId *) opaque_data);
+}
+
+/*
+ * Custom IO errmsg for MultiXactMember.
+ */
+static inline int
+MultiXactMemberIoErrorMsg(const void *opaque_data)
+{
+ Assert(opaque_data != NULL);
+
+ return errmsg("could not access status of multixact member %" PRIu64,
+ *(MultiXactOffset *) opaque_data);
+}
+
/*
* Decide which of two MultiXactIds is earlier.
*
--
2.43.0
[application/octet-stream] v3-0006-Expand-xact-SLRU-IO-error-to-show-epoch.patch (1.1K, 8-v3-0006-Expand-xact-SLRU-IO-error-to-show-epoch.patch)
download | inline diff:
From 0929999480cbcf901681c34bf3d7c18ae1c82acc Mon Sep 17 00:00:00 2001
From: Maxim Orlov <[email protected]>
Date: Thu, 26 Feb 2026 16:55:45 +0300
Subject: [PATCH v3 6/6] Expand xact SLRU IO-error to show epoch
---
src/include/access/slru.h | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 78ee36c05a6..3ca2e4e92f4 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -167,8 +167,16 @@ static inline int
xact_errmsg_for_io_error(const void *opaque_data)
{
if (opaque_data)
- return errmsg("could not access status of transaction %u",
- *(TransactionId *) opaque_data);
+ {
+ FullTransactionId fxid;
+
+ fxid = FullTransactionIdFromAllowableAt(ReadNextFullTransactionId(),
+ *(TransactionId *) opaque_data);
+
+ return errmsg("could not access status of transaction %u:%u",
+ EpochFromFullTransactionId(fxid),
+ XidFromFullTransactionId(fxid));
+ }
return errmsg("could not access slru entry"); /* InvalidTransactionId */
}
--
2.43.0
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Rework SLRU I/O errors handle
@ 2026-03-06 14:43 Heikki Linnakangas <[email protected]>
parent: Maxim Orlov <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Heikki Linnakangas @ 2026-03-06 14:43 UTC (permalink / raw)
To: Maxim Orlov <[email protected]>; Postgres hackers <[email protected]>
On 26/02/2026 16:26, Maxim Orlov wrote:
> Beginning of the discussion is here [0].
>
> Historically, the SLRU module was designed to handle 32-bit
> transactions. However, it is now utilised for handling a variety of
> object types, like TransactionId, MultixactId, MultiXactOffset,
> QueuePosition, and so on. But the IO error reporting system is still
> designed to support 32-bit XIDs exclusively.
>
> The proposed patchset allows us to define a "custom" callback to
> improve error messages.
>
> The first two commits add a callback and test case. The subsequent ones
> improve I/O error messages. The last one adds the XID epoch to the error
> message. It's purely optional, but I think it would be useful.
>
> [0] https://www.postgresql.org/message-id/
> CACG%3Dezbwy1zargXDNPeYXxZwRW3jXu_aD%3DrcG-7dc4fw7Y9Ojw%40mail.gmail.com
> <https://www.postgresql.org/message-id/
> CACG%3Dezbwy1zargXDNPeYXxZwRW3jXu_aD%3DrcG-7dc4fw7Y9Ojw%40mail.gmail.com>
Thanks, looks reasonable.
I'm -1 on the last patch, "Expand xact SLRU IO-error to show epoch"
though. The epoch isn't used in addressing the SLRU, the patch just
expands the 32-bit XID into a full 64-bit XID using the current epoch.
That seems misleading.
- Heikki
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Rework SLRU I/O errors handle
@ 2026-03-06 16:22 Heikki Linnakangas <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Heikki Linnakangas @ 2026-03-06 16:22 UTC (permalink / raw)
To: Álvaro Herrera <[email protected]>; +Cc: Maxim Orlov <[email protected]>; Postgres hackers <[email protected]>
On 06/03/2026 17:33, Álvaro Herrera wrote:
> I'm not a fan of the split. I think it all these patches should be
> pushed as a single commit, and avoid introducing xact_errmsg_for_io_error
> as an exposed function. I think that doesn't make a lot of sense. Each
> SLRU should have a correct and appropriate error reporting callback.
Agreed.
> The comment added in 0005 is bogus too. It mentions InvalidTransactionId,
> but the problem is not that the value is 0 but rather that we get no
> pointer. Also, in all other callbacks the pointer is asserted to not be
> NULL, so why don't we do the same here, and avoid an error message
> that's not going to help anyone? I see however that in the patch we're
> passing a NULL to SlruReportIOError(), which means if you get an IO
> error with any SLRU other than xact, you're going to get either a crash
> on the assertion, or (on non-debug builds) a crash on dereferencing the
> NULL pointer.
Hmm, I thought we could just never pass a NULL pointer, but if a Write
fails, slru.c has no context where to pull that opaque_data. Perhaps we
should just not call the callback in that case.
I'm starting to feel that what SlruReportIOError() currently uses as the
errdetail, could well be the main error message, and the callback could
provide the errdetail. I.e. swap the errmsg and errdetail we have now.
That way, we can just leave out the errdetail for Write failures. The
current errmsg we have for Write failures is pretty bad: "ERROR: could
not access status of transaction 0". What's currently the errdetail,
e.g. "Could not read from file \"%s\" at offset %d: %m", is a lot more
informative.
- Heikki
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Rework SLRU I/O errors handle
@ 2026-03-06 16:38 Heikki Linnakangas <[email protected]>
parent: Heikki Linnakangas <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Heikki Linnakangas @ 2026-03-06 16:38 UTC (permalink / raw)
To: Álvaro Herrera <[email protected]>; +Cc: Maxim Orlov <[email protected]>; Postgres hackers <[email protected]>
On 06/03/2026 18:22, Heikki Linnakangas wrote:
> On 06/03/2026 17:33, Álvaro Herrera wrote:
>> I'm not a fan of the split. I think it all these patches should be
>> pushed as a single commit, and avoid introducing xact_errmsg_for_io_error
>> as an exposed function. I think that doesn't make a lot of sense. Each
>> SLRU should have a correct and appropriate error reporting callback.
>
> Agreed.
>
>> The comment added in 0005 is bogus too. It mentions
>> InvalidTransactionId,
>> but the problem is not that the value is 0 but rather that we get no
>> pointer. Also, in all other callbacks the pointer is asserted to not be
>> NULL, so why don't we do the same here, and avoid an error message
>> that's not going to help anyone? I see however that in the patch we're
>> passing a NULL to SlruReportIOError(), which means if you get an IO
>> error with any SLRU other than xact, you're going to get either a crash
>> on the assertion, or (on non-debug builds) a crash on dereferencing the
>> NULL pointer.
>
> Hmm, I thought we could just never pass a NULL pointer, but if a Write
> fails, slru.c has no context where to pull that opaque_data. Perhaps we
> should just not call the callback in that case.
>
> I'm starting to feel that what SlruReportIOError() currently uses as the
> errdetail, could well be the main error message, and the callback could
> provide the errdetail. I.e. swap the errmsg and errdetail we have now.
> That way, we can just leave out the errdetail for Write failures. The
> current errmsg we have for Write failures is pretty bad: "ERROR: could
> not access status of transaction 0". What's currently the errdetail,
> e.g. "Could not read from file \"%s\" at offset %d: %m", is a lot more
> informative.
Attached version that does that.
As another data point, we have a lot of error messages like "could not
open file \"%s\": %m" in the source tree. slru.c was the only place
where that was in the errdetail() part.
- Heikki
Attachments:
[text/x-patch] v2-0001-Add-a-callback-for-I-O-error-messages-in-SLRUs.patch (32.5K, 2-v2-0001-Add-a-callback-for-I-O-error-messages-in-SLRUs.patch)
download | inline diff:
From f93661167b501e7ade94976d3743dccadd3bbed0 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Fri, 6 Mar 2026 18:34:29 +0200
Subject: [PATCH v2 1/1] Add a callback for I/O error messages in SLRUs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Historically, all SLRUs were addressed by transaction IDs, but that
hasn't been true for long time. However, the error message on I/O
error still always talked about accessing a transaction ID.
This commit adds a callback that allow subsystems to construct their
own error messages, which can then correctly refer to a transaction
ID, a multixid or whatever else is used to address the SLRU.
Author: Maxim Orlov <[email protected]>
Reviewed-by: Álvaro Herrera <[email protected]>
Discussion: https://www.postgresql.org/message-id/CACG=ezZZfurhYV+66ceubxQAyWqv9vaUi0yoO4-t48OE5xc0DQ@mail.gmail.com
---
src/backend/access/transam/clog.c | 17 +++-
src/backend/access/transam/commit_ts.c | 13 ++-
src/backend/access/transam/multixact.c | 48 ++++++++---
src/backend/access/transam/slru.c | 80 ++++++++++---------
src/backend/access/transam/subtrans.c | 14 +++-
src/backend/commands/async.c | 20 +++--
src/backend/storage/lmgr/predicate.c | 14 +++-
src/include/access/slru.h | 15 +++-
.../modules/test_slru/expected/test_slru.out | 4 +
src/test/modules/test_slru/sql/test_slru.sql | 3 +
src/test/modules/test_slru/test_slru--1.0.sql | 2 +-
src/test/modules/test_slru/test_slru.c | 15 +++-
12 files changed, 178 insertions(+), 67 deletions(-)
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index b5c38bbb162..68189de4645 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -111,6 +111,7 @@ static SlruCtlData XactCtlData;
static bool CLOGPagePrecedes(int64 page1, int64 page2);
+static int clog_errdetail_for_io_error(const void *opaque_data);
static void WriteTruncateXlogRec(int64 pageno, TransactionId oldestXact,
Oid oldestXactDb);
static void TransactionIdSetPageStatus(TransactionId xid, int nsubxids,
@@ -381,8 +382,7 @@ TransactionIdSetPageStatusInternal(TransactionId xid, int nsubxids,
* write-busy, since we don't care if the update reaches disk sooner than
* we think.
*/
- slotno = SimpleLruReadPage(XactCtl, pageno, !XLogRecPtrIsValid(lsn),
- xid);
+ slotno = SimpleLruReadPage(XactCtl, pageno, !XLogRecPtrIsValid(lsn), &xid);
/*
* Set the main transaction id, if any.
@@ -743,7 +743,7 @@ TransactionIdGetStatus(TransactionId xid, XLogRecPtr *lsn)
/* lock is acquired by SimpleLruReadPage_ReadOnly */
- slotno = SimpleLruReadPage_ReadOnly(XactCtl, pageno, xid);
+ slotno = SimpleLruReadPage_ReadOnly(XactCtl, pageno, &xid);
byteptr = XactCtl->shared->page_buffer[slotno] + byteno;
status = (*byteptr >> bshift) & CLOG_XACT_BITMASK;
@@ -807,6 +807,7 @@ CLOGShmemInit(void)
Assert(transaction_buffers != 0);
XactCtl->PagePrecedes = CLOGPagePrecedes;
+ XactCtl->errdetail_for_io_error = clog_errdetail_for_io_error;
SimpleLruInit(XactCtl, "transaction", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE,
"pg_xact", LWTRANCHE_XACT_BUFFER,
LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG, false);
@@ -882,7 +883,7 @@ TrimCLOG(void)
int slotno;
char *byteptr;
- slotno = SimpleLruReadPage(XactCtl, pageno, false, xid);
+ slotno = SimpleLruReadPage(XactCtl, pageno, false, &xid);
byteptr = XactCtl->shared->page_buffer[slotno] + byteno;
/* Zero so-far-unused positions in the current byte */
@@ -1033,6 +1034,14 @@ CLOGPagePrecedes(int64 page1, int64 page2)
TransactionIdPrecedes(xid1, xid2 + CLOG_XACTS_PER_PAGE - 1));
}
+static int
+clog_errdetail_for_io_error(const void *opaque_data)
+{
+ TransactionId xid = *(const TransactionId *) opaque_data;
+
+ return errdetail("Could not access commit status of transaction %u.", xid);
+}
+
/*
* Write a TRUNCATE xlog record
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 6fa2178f1dd..6e1c80b44b3 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -115,6 +115,7 @@ static void TransactionIdSetCommitTs(TransactionId xid, TimestampTz ts,
ReplOriginId nodeid, int slotno);
static void error_commit_ts_disabled(void);
static bool CommitTsPagePrecedes(int64 page1, int64 page2);
+static int commit_ts_errdetail_for_io_error(const void *opaque_data);
static void ActivateCommitTs(void);
static void DeactivateCommitTs(void);
static void WriteTruncateXlogRec(int64 pageno, TransactionId oldestXid);
@@ -227,7 +228,7 @@ SetXidCommitTsInPage(TransactionId xid, int nsubxids,
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(CommitTsCtl, pageno, true, xid);
+ slotno = SimpleLruReadPage(CommitTsCtl, pageno, true, &xid);
TransactionIdSetCommitTs(xid, ts, nodeid, slotno);
for (i = 0; i < nsubxids; i++)
@@ -332,7 +333,7 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts,
}
/* lock is acquired by SimpleLruReadPage_ReadOnly */
- slotno = SimpleLruReadPage_ReadOnly(CommitTsCtl, pageno, xid);
+ slotno = SimpleLruReadPage_ReadOnly(CommitTsCtl, pageno, &xid);
memcpy(&entry,
CommitTsCtl->shared->page_buffer[slotno] +
SizeOfCommitTimestampEntry * entryno,
@@ -551,6 +552,7 @@ CommitTsShmemInit(void)
Assert(commit_timestamp_buffers != 0);
CommitTsCtl->PagePrecedes = CommitTsPagePrecedes;
+ CommitTsCtl->errdetail_for_io_error = commit_ts_errdetail_for_io_error;
SimpleLruInit(CommitTsCtl, "commit_timestamp", CommitTsShmemBuffers(), 0,
"pg_commit_ts", LWTRANCHE_COMMITTS_BUFFER,
LWTRANCHE_COMMITTS_SLRU,
@@ -959,6 +961,13 @@ CommitTsPagePrecedes(int64 page1, int64 page2)
TransactionIdPrecedes(xid1, xid2 + COMMIT_TS_XACTS_PER_PAGE - 1));
}
+static int
+commit_ts_errdetail_for_io_error(const void *opaque_data)
+{
+ TransactionId xid = *(const TransactionId *) opaque_data;
+
+ return errdetail("Could not access commit timestamp of transaction %u.", xid);
+}
/*
* Write a TRUNCATE xlog record
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 8a5c9818ed6..6f7f39d4e53 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -315,6 +315,8 @@ static void mXactCachePut(MultiXactId multi, int nmembers,
/* management of SLRU infrastructure */
static bool MultiXactOffsetPagePrecedes(int64 page1, int64 page2);
static bool MultiXactMemberPagePrecedes(int64 page1, int64 page2);
+static int MultiXactOffsetIoErrorDetail(const void *opaque_data);
+static int MultiXactMemberIoErrorDetail(const void *opaque_data);
static void ExtendMultiXactOffset(MultiXactId multi);
static void ExtendMultiXactMember(MultiXactOffset offset, int nmembers);
static void SetOldestOffset(void);
@@ -836,7 +838,7 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
* enough that a MultiXactId is really involved. Perhaps someday we'll
* take the trouble to generalize the slru.c error reporting code.
*/
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, multi);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, &multi);
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
offptr += entryno;
@@ -865,7 +867,7 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
lock = SimpleLruGetBankLock(MultiXactOffsetCtl, next_pageno);
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, next_pageno, true, next);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, next_pageno, true, &next);
next_offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
next_offptr += next_entryno;
}
@@ -919,7 +921,8 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
LWLockAcquire(lock, LW_EXCLUSIVE);
prevlock = lock;
}
- slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, multi);
+ slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true,
+ &offset);
prev_pageno = pageno;
}
@@ -1244,7 +1247,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
LWLockAcquire(lock, LW_EXCLUSIVE);
/* read this multi's offset */
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, multi);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, &multi);
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
offptr += entryno;
offset = *offptr;
@@ -1282,7 +1285,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
LWLockAcquire(newlock, LW_EXCLUSIVE);
lock = newlock;
}
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, tmpMXact);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, &tmpMXact);
}
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
@@ -1347,7 +1350,8 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
lock = newlock;
}
- slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, multi);
+ slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true,
+ &offset);
prev_pageno = pageno;
}
@@ -1779,6 +1783,9 @@ MultiXactShmemInit(void)
MultiXactOffsetCtl->PagePrecedes = MultiXactOffsetPagePrecedes;
MultiXactMemberCtl->PagePrecedes = MultiXactMemberPagePrecedes;
+ MultiXactOffsetCtl->errdetail_for_io_error = MultiXactOffsetIoErrorDetail;
+ MultiXactMemberCtl->errdetail_for_io_error = MultiXactMemberIoErrorDetail;
+
SimpleLruInit(MultiXactOffsetCtl,
"multixact_offset", multixact_offset_buffers, 0,
"pg_multixact/offsets", LWTRANCHE_MULTIXACTOFFSET_BUFFER,
@@ -1928,7 +1935,7 @@ TrimMultiXact(void)
if (entryno == 0 || nextMXact == FirstMultiXactId)
slotno = SimpleLruZeroPage(MultiXactOffsetCtl, pageno);
else
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, nextMXact);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, &nextMXact);
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
offptr += entryno;
@@ -1963,7 +1970,7 @@ TrimMultiXact(void)
LWLockAcquire(lock, LW_EXCLUSIVE);
memberoff = MXOffsetToMemberOffset(offset);
- slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, offset);
+ slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, &offset);
xidptr = (TransactionId *)
(MultiXactMemberCtl->shared->page_buffer[slotno] + memberoff);
@@ -2497,7 +2504,7 @@ find_multixact_start(MultiXactId multi, MultiXactOffset *result)
return false;
/* lock is acquired by SimpleLruReadPage_ReadOnly */
- slotno = SimpleLruReadPage_ReadOnly(MultiXactOffsetCtl, pageno, multi);
+ slotno = SimpleLruReadPage_ReadOnly(MultiXactOffsetCtl, pageno, &multi);
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
offptr += entryno;
offset = *offptr;
@@ -2808,6 +2815,29 @@ MultiXactMemberPagePrecedes(int64 page1, int64 page2)
return page1 < page2;
}
+/*
+ * Custom IO errmsg for MultiXactOffset.
+ */
+static inline int
+MultiXactOffsetIoErrorDetail(const void *opaque_data)
+{
+ MultiXactId multixid = *(const MultiXactId *) opaque_data;
+
+ return errdetail("Could not access offset of multixact %u.", multixid);
+}
+
+/*
+ * Custom IO errmsg for MultiXactMember.
+ */
+static inline int
+MultiXactMemberIoErrorDetail(const void *opaque_data)
+{
+ MultiXactOffset mxoff = *(const MultiXactOffset *) opaque_data;
+
+ return errdetail("could not access multixact member at offset %" PRIu64 ".",
+ mxoff);
+}
+
/*
* Decide which of two MultiXactIds is earlier.
*
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 549c7e3e64b..8ba3eaee01e 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -181,7 +181,8 @@ static void SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata);
static bool SlruPhysicalReadPage(SlruCtl ctl, int64 pageno, int slotno);
static bool SlruPhysicalWritePage(SlruCtl ctl, int64 pageno, int slotno,
SlruWriteAll fdata);
-static void SlruReportIOError(SlruCtl ctl, int64 pageno, TransactionId xid);
+static void SlruReportIOError(SlruCtl ctl, int64 pageno,
+ const void *opaque_data);
static int SlruSelectLRUPage(SlruCtl ctl, int64 pageno);
static bool SlruScanDirCbDeleteCutoff(SlruCtl ctl, char *filename,
@@ -257,6 +258,9 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
bool found;
int nbanks = nslots / SLRU_BANK_SIZE;
+ /* Make sure callbacks are set up */
+ Assert(ctl->PagePrecedes != NULL);
+
Assert(nslots <= SLRU_MAX_ALLOWED_BUFFERS);
shared = (SlruShared) ShmemInitStruct(name,
@@ -515,8 +519,9 @@ SimpleLruWaitIO(SlruCtl ctl, int slotno)
* that modification of the page is safe. If write_ok is false then we
* will not return the page until it is not undergoing active I/O.
*
- * The passed-in xid is used only for error reporting, and may be
- * InvalidTransactionId if no specific xid is associated with the action.
+ * On error, the passed-in 'opaque_data' is passed to the
+ * 'errmsg_for_io_error' callback, to provide details on the operation that
+ * failed. It is only used for error reporting.
*
* Return value is the shared-buffer slot number now holding the page.
* The buffer's LRU access info is updated.
@@ -525,7 +530,7 @@ SimpleLruWaitIO(SlruCtl ctl, int slotno)
*/
int
SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
- TransactionId xid)
+ const void *opaque_data)
{
SlruShared shared = ctl->shared;
LWLock *banklock = SimpleLruGetBankLock(ctl, pageno);
@@ -601,7 +606,7 @@ SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
/* Now it's okay to ereport if we failed */
if (!ok)
- SlruReportIOError(ctl, pageno, xid);
+ SlruReportIOError(ctl, pageno, opaque_data);
SlruRecentlyUsed(shared, slotno);
@@ -617,8 +622,9 @@ SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
* The page number must correspond to an already-initialized page.
* The caller must intend only read-only access to the page.
*
- * The passed-in xid is used only for error reporting, and may be
- * InvalidTransactionId if no specific xid is associated with the action.
+ * On error, the passed-in 'opaque_data' is passed to the
+ * 'errmsg_for_io_error' callback, to provide details on the operation that
+ * failed. It is only used for error reporting.
*
* Return value is the shared-buffer slot number now holding the page.
* The buffer's LRU access info is updated.
@@ -627,7 +633,7 @@ SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
* It is unspecified whether the lock will be shared or exclusive.
*/
int
-SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno, TransactionId xid)
+SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno, const void *opaque_data)
{
SlruShared shared = ctl->shared;
LWLock *banklock = SimpleLruGetBankLock(ctl, pageno);
@@ -659,7 +665,7 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno, TransactionId xid)
LWLockRelease(banklock);
LWLockAcquire(banklock, LW_EXCLUSIVE);
- return SimpleLruReadPage(ctl, pageno, true, xid);
+ return SimpleLruReadPage(ctl, pageno, true, opaque_data);
}
/*
@@ -682,6 +688,7 @@ SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata)
bool ok;
Assert(shared->page_status[slotno] != SLRU_PAGE_EMPTY);
+
Assert(LWLockHeldByMeInMode(SimpleLruGetBankLock(ctl, pageno), LW_EXCLUSIVE));
/* If a write is in progress, wait for it to finish */
@@ -739,7 +746,7 @@ SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata)
/* Now it's okay to ereport if we failed */
if (!ok)
- SlruReportIOError(ctl, pageno, InvalidTransactionId);
+ SlruReportIOError(ctl, pageno, NULL);
/* If part of a checkpoint, count this as a SLRU buffer written. */
if (fdata)
@@ -793,14 +800,14 @@ SimpleLruDoesPhysicalPageExist(SlruCtl ctl, int64 pageno)
/* report error normally */
slru_errcause = SLRU_OPEN_FAILED;
slru_errno = errno;
- SlruReportIOError(ctl, pageno, 0);
+ SlruReportIOError(ctl, pageno, NULL);
}
if ((endpos = lseek(fd, 0, SEEK_END)) < 0)
{
slru_errcause = SLRU_SEEK_FAILED;
slru_errno = errno;
- SlruReportIOError(ctl, pageno, 0);
+ SlruReportIOError(ctl, pageno, NULL);
}
result = endpos >= (off_t) (offset + BLCKSZ);
@@ -1070,7 +1077,7 @@ SlruPhysicalWritePage(SlruCtl ctl, int64 pageno, int slotno, SlruWriteAll fdata)
* SlruPhysicalWritePage. Call this after cleaning up shared-memory state.
*/
static void
-SlruReportIOError(SlruCtl ctl, int64 pageno, TransactionId xid)
+SlruReportIOError(SlruCtl ctl, int64 pageno, const void *opaque_data)
{
int64 segno = pageno / SLRU_PAGES_PER_SEGMENT;
int rpageno = pageno % SLRU_PAGES_PER_SEGMENT;
@@ -1084,54 +1091,55 @@ SlruReportIOError(SlruCtl ctl, int64 pageno, TransactionId xid)
case SLRU_OPEN_FAILED:
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
- errdetail("Could not open file \"%s\": %m.", path)));
+ errmsg("could not open file \"%s\": %m", path),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
break;
case SLRU_SEEK_FAILED:
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
- errdetail("Could not seek in file \"%s\" to offset %d: %m.",
- path, offset)));
+ errmsg("could not seek in file \"%s\" to offset %d: %m",
+ path, offset),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
break;
case SLRU_READ_FAILED:
if (errno)
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
- errdetail("Could not read from file \"%s\" at offset %d: %m.",
- path, offset)));
+ errmsg("could not read from file \"%s\" at offset %d: %m",
+ path, offset),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
else
ereport(ERROR,
- (errmsg("could not access status of transaction %u", xid),
- errdetail("Could not read from file \"%s\" at offset %d: read too few bytes.", path, offset)));
+ (errmsg("could not read from file \"%s\" at offset %d: read too few bytes",
+ path, offset),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
break;
case SLRU_WRITE_FAILED:
if (errno)
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
- errdetail("Could not write to file \"%s\" at offset %d: %m.",
- path, offset)));
+ errmsg("Could not write to file \"%s\" at offset %d: %m",
+ path, offset),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
else
ereport(ERROR,
- (errmsg("could not access status of transaction %u", xid),
- errdetail("Could not write to file \"%s\" at offset %d: wrote too few bytes.",
- path, offset)));
+ (errmsg("Could not write to file \"%s\" at offset %d: wrote too few bytes.",
+ path, offset),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
break;
case SLRU_FSYNC_FAILED:
ereport(data_sync_elevel(ERROR),
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
- errdetail("Could not fsync file \"%s\": %m.",
- path)));
+ errmsg("could not fsync file \"%s\": %m",
+ path),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
break;
case SLRU_CLOSE_FAILED:
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
- errdetail("Could not close file \"%s\": %m.",
- path)));
+ errmsg("could not close file \"%s\": %m",
+ path),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
break;
default:
/* can't get here, we trust */
@@ -1411,7 +1419,7 @@ SimpleLruWriteAll(SlruCtl ctl, bool allow_redirtied)
}
}
if (!ok)
- SlruReportIOError(ctl, pageno, InvalidTransactionId);
+ SlruReportIOError(ctl, pageno, NULL);
/* Ensure that directory entries for new files are on disk. */
if (ctl->sync_handler != SYNC_HANDLER_NONE)
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index c0987f43f11..86c5b5c05bd 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -75,6 +75,7 @@ static SlruCtlData SubTransCtlData;
static bool SubTransPagePrecedes(int64 page1, int64 page2);
+static int subtrans_errdetail_for_io_error(const void *opaque_data);
/*
@@ -95,7 +96,7 @@ SubTransSetParent(TransactionId xid, TransactionId parent)
lock = SimpleLruGetBankLock(SubTransCtl, pageno);
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(SubTransCtl, pageno, true, xid);
+ slotno = SimpleLruReadPage(SubTransCtl, pageno, true, &xid);
ptr = (TransactionId *) SubTransCtl->shared->page_buffer[slotno];
ptr += entryno;
@@ -135,7 +136,7 @@ SubTransGetParent(TransactionId xid)
/* lock is acquired by SimpleLruReadPage_ReadOnly */
- slotno = SimpleLruReadPage_ReadOnly(SubTransCtl, pageno, xid);
+ slotno = SimpleLruReadPage_ReadOnly(SubTransCtl, pageno, &xid);
ptr = (TransactionId *) SubTransCtl->shared->page_buffer[slotno];
ptr += entryno;
@@ -240,6 +241,7 @@ SUBTRANSShmemInit(void)
Assert(subtransaction_buffers != 0);
SubTransCtl->PagePrecedes = SubTransPagePrecedes;
+ SubTransCtl->errdetail_for_io_error = subtrans_errdetail_for_io_error;
SimpleLruInit(SubTransCtl, "subtransaction", SUBTRANSShmemBuffers(), 0,
"pg_subtrans", LWTRANCHE_SUBTRANS_BUFFER,
LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE, false);
@@ -419,3 +421,11 @@ SubTransPagePrecedes(int64 page1, int64 page2)
return (TransactionIdPrecedes(xid1, xid2) &&
TransactionIdPrecedes(xid1, xid2 + SUBTRANS_XACTS_PER_PAGE - 1));
}
+
+static int
+subtrans_errdetail_for_io_error(const void *opaque_data)
+{
+ TransactionId xid = *(const TransactionId *) opaque_data;
+
+ return errdetail("Could not access subtransaction status of transaction %u.", xid);
+}
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 0b6d119dad0..093c91ea7f6 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -570,6 +570,7 @@ bool Trace_notify = false;
int max_notify_queue_pages = 1048576;
/* local function prototypes */
+static int asyncQueueErrdetailForIoError(const void *opaque_data);
static inline int64 asyncQueuePageDiff(int64 p, int64 q);
static inline bool asyncQueuePagePrecedes(int64 p, int64 q);
static inline void GlobalChannelKeyInit(GlobalChannelKey *key, Oid dboid,
@@ -610,6 +611,15 @@ static uint32 notification_hash(const void *key, Size keysize);
static int notification_match(const void *key1, const void *key2, Size keysize);
static void ClearPendingActionsAndNotifies(void);
+static int
+asyncQueueErrdetailForIoError(const void *opaque_data)
+{
+ const QueuePosition *position = opaque_data;
+
+ return errmsg("could not access async queue at page %" PRId64", offset %d",
+ position->page, position->offset);
+}
+
/*
* Compute the difference between two queue page numbers.
* Previously this function accounted for a wraparound.
@@ -830,6 +840,7 @@ AsyncShmemInit(void)
* names are used in order to avoid wraparound.
*/
NotifyCtl->PagePrecedes = asyncQueuePagePrecedes;
+ NotifyCtl->errdetail_for_io_error = asyncQueueErrdetailForIoError;
SimpleLruInit(NotifyCtl, "notify", notify_buffers, 0,
"pg_notify", LWTRANCHE_NOTIFY_BUFFER, LWTRANCHE_NOTIFY_SLRU,
SYNC_HANDLER_NONE, true);
@@ -2068,8 +2079,7 @@ asyncQueueAddEntries(ListCell *nextNotify)
if (QUEUE_POS_IS_ZERO(queue_head))
slotno = SimpleLruZeroPage(NotifyCtl, pageno);
else
- slotno = SimpleLruReadPage(NotifyCtl, pageno, true,
- InvalidTransactionId);
+ slotno = SimpleLruReadPage(NotifyCtl, pageno, true, &queue_head);
/* Note we mark the page dirty before writing in it */
NotifyCtl->shared->page_dirty[slotno] = true;
@@ -2739,8 +2749,7 @@ asyncQueueProcessPageEntries(QueuePosition *current,
alignas(AsyncQueueEntry) char local_buf[QUEUE_PAGESIZE];
char *local_buf_end = local_buf;
- slotno = SimpleLruReadPage_ReadOnly(NotifyCtl, curpage,
- InvalidTransactionId);
+ slotno = SimpleLruReadPage_ReadOnly(NotifyCtl, curpage, current);
page_buffer = NotifyCtl->shared->page_buffer[slotno];
do
@@ -2998,8 +3007,7 @@ AsyncNotifyFreezeXids(TransactionId newFrozenXid)
lock = SimpleLruGetBankLock(NotifyCtl, pageno);
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(NotifyCtl, pageno, true,
- InvalidTransactionId);
+ slotno = SimpleLruReadPage(NotifyCtl, pageno, true, &pos);
page_buffer = NotifyCtl->shared->page_buffer[slotno];
curpage = pageno;
}
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index fe75ead3501..a548e01f890 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -442,6 +442,7 @@ static void ReleaseRWConflict(RWConflict conflict);
static void FlagSxactUnsafe(SERIALIZABLEXACT *sxact);
static bool SerialPagePrecedesLogically(int64 page1, int64 page2);
+static int serial_errdetail_for_io_error(const void *opaque_data);
static void SerialInit(void);
static void SerialAdd(TransactionId xid, SerCommitSeqNo minConflictCommitSeqNo);
static SerCommitSeqNo SerialGetMinConflictCommitSeqNo(TransactionId xid);
@@ -742,6 +743,14 @@ SerialPagePrecedesLogically(int64 page1, int64 page2)
TransactionIdPrecedes(xid1, xid2 + SERIAL_ENTRIESPERPAGE - 1));
}
+static int
+serial_errdetail_for_io_error(const void *opaque_data)
+{
+ TransactionId xid = *(const TransactionId *) opaque_data;
+
+ return errdetail("Could not access serializable CSN of transaction %u.", xid);
+}
+
#ifdef USE_ASSERT_CHECKING
static void
SerialPagePrecedesLogicallyUnitTests(void)
@@ -811,6 +820,7 @@ SerialInit(void)
* Set up SLRU management of the pg_serial data.
*/
SerialSlruCtl->PagePrecedes = SerialPagePrecedesLogically;
+ SerialSlruCtl->errdetail_for_io_error = serial_errdetail_for_io_error;
SimpleLruInit(SerialSlruCtl, "serializable",
serializable_buffers, 0, "pg_serial",
LWTRANCHE_SERIAL_BUFFER, LWTRANCHE_SERIAL_SLRU,
@@ -930,7 +940,7 @@ SerialAdd(TransactionId xid, SerCommitSeqNo minConflictCommitSeqNo)
else
{
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(SerialSlruCtl, targetPage, true, xid);
+ slotno = SimpleLruReadPage(SerialSlruCtl, targetPage, true, &xid);
}
SerialValue(slotno, xid) = minConflictCommitSeqNo;
@@ -974,7 +984,7 @@ SerialGetMinConflictCommitSeqNo(TransactionId xid)
* but will return with that lock held, which must then be released.
*/
slotno = SimpleLruReadPage_ReadOnly(SerialSlruCtl,
- SerialPage(xid), xid);
+ SerialPage(xid), &xid);
val = SerialValue(slotno, xid);
LWLockRelease(SimpleLruGetBankLock(SerialSlruCtl, SerialPage(xid)));
return val;
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 4cb8f478fce..8d3fe59299f 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -13,6 +13,7 @@
#ifndef SLRU_H
#define SLRU_H
+#include "access/transam.h"
#include "access/xlogdefs.h"
#include "storage/lwlock.h"
#include "storage/sync.h"
@@ -141,6 +142,16 @@ typedef struct SlruCtlData
*/
bool (*PagePrecedes) (int64, int64);
+ /*
+ * Callback to provide more details on an I/O error. This is called as
+ * part of ereport(), and the callback function is expected to call
+ * errdetail() to provide more details.
+ *
+ * The opaque_data argument here is the argument passed to the
+ * SimpleLruReadPage() function.
+ */
+ int (*errdetail_for_io_error)(const void *opaque_data);
+
/*
* Dir is set during SimpleLruInit and does not change thereafter. Since
* it's always the same, it doesn't need to be in shared memory.
@@ -174,9 +185,9 @@ extern void SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
extern int SimpleLruZeroPage(SlruCtl ctl, int64 pageno);
extern void SimpleLruZeroAndWritePage(SlruCtl ctl, int64 pageno);
extern int SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
- TransactionId xid);
+ const void *opaque_data);
extern int SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno,
- TransactionId xid);
+ const void *opaque_data);
extern void SimpleLruWritePage(SlruCtl ctl, int slotno);
extern void SimpleLruWriteAll(SlruCtl ctl, bool allow_redirtied);
#ifdef USE_ASSERT_CHECKING
diff --git a/src/test/modules/test_slru/expected/test_slru.out b/src/test/modules/test_slru/expected/test_slru.out
index 185c56e5d62..7ae1b0d4a98 100644
--- a/src/test/modules/test_slru/expected/test_slru.out
+++ b/src/test/modules/test_slru/expected/test_slru.out
@@ -23,6 +23,10 @@ SELECT test_slru_page_exists(12345);
t
(1 row)
+-- Test read failure
+SELECT test_slru_page_read(54321, false, '123'::xid);
+ERROR: could not open file "pg_test_slru/0000000000006A1": No such file or directory
+DETAIL: Could not access test_slru entry 123.
-- 48 extra pages
SELECT count(test_slru_page_write(a, 'Test SLRU'))
FROM generate_series(12346, 12393, 1) as a;
diff --git a/src/test/modules/test_slru/sql/test_slru.sql b/src/test/modules/test_slru/sql/test_slru.sql
index b1b376581ab..c6454c5bf82 100644
--- a/src/test/modules/test_slru/sql/test_slru.sql
+++ b/src/test/modules/test_slru/sql/test_slru.sql
@@ -5,6 +5,9 @@ SELECT test_slru_page_write(12345, 'Test SLRU');
SELECT test_slru_page_read(12345);
SELECT test_slru_page_exists(12345);
+-- Test read failure
+SELECT test_slru_page_read(54321, false, '123'::xid);
+
-- 48 extra pages
SELECT count(test_slru_page_write(a, 'Test SLRU'))
FROM generate_series(12346, 12393, 1) as a;
diff --git a/src/test/modules/test_slru/test_slru--1.0.sql b/src/test/modules/test_slru/test_slru--1.0.sql
index abecb5e2183..2148e7204ec 100644
--- a/src/test/modules/test_slru/test_slru--1.0.sql
+++ b/src/test/modules/test_slru/test_slru--1.0.sql
@@ -7,7 +7,7 @@ CREATE OR REPLACE FUNCTION test_slru_page_writeall() RETURNS VOID
AS 'MODULE_PATHNAME', 'test_slru_page_writeall' LANGUAGE C;
CREATE OR REPLACE FUNCTION test_slru_page_sync(bigint) RETURNS VOID
AS 'MODULE_PATHNAME', 'test_slru_page_sync' LANGUAGE C;
-CREATE OR REPLACE FUNCTION test_slru_page_read(bigint, bool DEFAULT true) RETURNS text
+CREATE OR REPLACE FUNCTION test_slru_page_read(bigint, bool DEFAULT true, xid DEFAULT '0') RETURNS text
AS 'MODULE_PATHNAME', 'test_slru_page_read' LANGUAGE C;
CREATE OR REPLACE FUNCTION test_slru_page_readonly(bigint) RETURNS text
AS 'MODULE_PATHNAME', 'test_slru_page_readonly' LANGUAGE C;
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 4dc74e19620..e4bd2af0bf5 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -93,14 +93,14 @@ test_slru_page_read(PG_FUNCTION_ARGS)
{
int64 pageno = PG_GETARG_INT64(0);
bool write_ok = PG_GETARG_BOOL(1);
+ TransactionId xid = PG_GETARG_TRANSACTIONID(2);
char *data = NULL;
int slotno;
LWLock *lock = SimpleLruGetBankLock(TestSlruCtl, pageno);
/* find page in buffers, reading it if necessary */
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(TestSlruCtl, pageno,
- write_ok, InvalidTransactionId);
+ slotno = SimpleLruReadPage(TestSlruCtl, pageno, write_ok, &xid);
data = (char *) TestSlruCtl->shared->page_buffer[slotno];
LWLockRelease(lock);
@@ -118,7 +118,7 @@ test_slru_page_readonly(PG_FUNCTION_ARGS)
/* find page in buffers, reading it if necessary */
slotno = SimpleLruReadPage_ReadOnly(TestSlruCtl,
pageno,
- InvalidTransactionId);
+ NULL);
Assert(LWLockHeldByMe(lock));
data = (char *) TestSlruCtl->shared->page_buffer[slotno];
LWLockRelease(lock);
@@ -210,6 +210,14 @@ test_slru_page_precedes_logically(int64 page1, int64 page2)
return page1 < page2;
}
+static int
+test_slru_errdetail_for_io_error(const void *opaque_data)
+{
+ TransactionId xid = *(const TransactionId *) opaque_data;
+
+ return errdetail("Could not access test_slru entry %u.", xid);
+}
+
static void
test_slru_shmem_startup(void)
{
@@ -245,6 +253,7 @@ test_slru_shmem_startup(void)
}
TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically;
+ TestSlruCtl->errdetail_for_io_error = test_slru_errdetail_for_io_error;
SimpleLruInit(TestSlruCtl, "TestSLRU",
NUM_TEST_BUFFERS, 0, slru_dir_name,
test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE,
--
2.47.3
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Rework SLRU I/O errors handle
@ 2026-03-06 16:49 Heikki Linnakangas <[email protected]>
parent: Heikki Linnakangas <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Heikki Linnakangas @ 2026-03-06 16:49 UTC (permalink / raw)
To: Álvaro Herrera <[email protected]>; +Cc: Maxim Orlov <[email protected]>; Postgres hackers <[email protected]>
On 06/03/2026 18:38, Heikki Linnakangas wrote:
> On 06/03/2026 18:22, Heikki Linnakangas wrote:
>> On 06/03/2026 17:33, Álvaro Herrera wrote:
>>> The comment added in 0005 is bogus too. It mentions
>>> InvalidTransactionId,
>>> but the problem is not that the value is 0 but rather that we get no
>>> pointer. Also, in all other callbacks the pointer is asserted to not be
>>> NULL, so why don't we do the same here, and avoid an error message
>>> that's not going to help anyone? I see however that in the patch we're
>>> passing a NULL to SlruReportIOError(), which means if you get an IO
>>> error with any SLRU other than xact, you're going to get either a crash
>>> on the assertion, or (on non-debug builds) a crash on dereferencing the
>>> NULL pointer.
>>
>> Hmm, I thought we could just never pass a NULL pointer, but if a Write
>> fails, slru.c has no context where to pull that opaque_data. Perhaps
>> we should just not call the callback in that case.
>>
>> I'm starting to feel that what SlruReportIOError() currently uses as
>> the errdetail, could well be the main error message, and the callback
>> could provide the errdetail. I.e. swap the errmsg and errdetail we
>> have now. That way, we can just leave out the errdetail for Write
>> failures. The current errmsg we have for Write failures is pretty bad:
>> "ERROR: could not access status of transaction 0". What's currently
>> the errdetail, e.g. "Could not read from file \"%s\" at offset %d:
>> %m", is a lot more informative.
>
> Attached version that does that.
That version was a little rushed, with wrong message styles etc.
leftovers. Sorry about that, here's yet another, more cleaned-up version.
- Heikki
Attachments:
[text/x-patch] v4-0001-Add-a-callback-for-I-O-error-messages-in-SLRUs.patch (31.8K, 2-v4-0001-Add-a-callback-for-I-O-error-messages-in-SLRUs.patch)
download | inline diff:
From 8ce55ee5f5957c810dad9d0e076dbff3354ffd45 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Fri, 6 Mar 2026 18:45:23 +0200
Subject: [PATCH v4 1/1] Add a callback for I/O error messages in SLRUs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Historically, all SLRUs were addressed by transaction IDs, but that
hasn't been true for long time. However, the error message on I/O
error still always talked about accessing a transaction ID.
This commit adds a callback that allow subsystems to construct their
own error messages, which can then correctly refer to a transaction
ID, a multixid or whatever else is used to address the SLRU.
Author: Maxim Orlov <[email protected]>
Reviewed-by: Álvaro Herrera <[email protected]>
Discussion: https://www.postgresql.org/message-id/CACG=ezZZfurhYV+66ceubxQAyWqv9vaUi0yoO4-t48OE5xc0DQ@mail.gmail.com
---
src/backend/access/transam/clog.c | 17 ++++-
src/backend/access/transam/commit_ts.c | 13 +++-
src/backend/access/transam/multixact.c | 42 +++++++---
src/backend/access/transam/slru.c | 76 ++++++++++---------
src/backend/access/transam/subtrans.c | 14 +++-
src/backend/commands/async.c | 20 +++--
src/backend/storage/lmgr/predicate.c | 14 +++-
src/include/access/slru.h | 15 +++-
.../modules/test_slru/expected/test_slru.out | 4 +
src/test/modules/test_slru/sql/test_slru.sql | 3 +
src/test/modules/test_slru/test_slru--1.0.sql | 2 +-
src/test/modules/test_slru/test_slru.c | 15 +++-
12 files changed, 168 insertions(+), 67 deletions(-)
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index b5c38bbb162..9ed7f2f7477 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -111,6 +111,7 @@ static SlruCtlData XactCtlData;
static bool CLOGPagePrecedes(int64 page1, int64 page2);
+static int clog_errdetail_for_io_error(const void *opaque_data);
static void WriteTruncateXlogRec(int64 pageno, TransactionId oldestXact,
Oid oldestXactDb);
static void TransactionIdSetPageStatus(TransactionId xid, int nsubxids,
@@ -381,8 +382,7 @@ TransactionIdSetPageStatusInternal(TransactionId xid, int nsubxids,
* write-busy, since we don't care if the update reaches disk sooner than
* we think.
*/
- slotno = SimpleLruReadPage(XactCtl, pageno, !XLogRecPtrIsValid(lsn),
- xid);
+ slotno = SimpleLruReadPage(XactCtl, pageno, !XLogRecPtrIsValid(lsn), &xid);
/*
* Set the main transaction id, if any.
@@ -743,7 +743,7 @@ TransactionIdGetStatus(TransactionId xid, XLogRecPtr *lsn)
/* lock is acquired by SimpleLruReadPage_ReadOnly */
- slotno = SimpleLruReadPage_ReadOnly(XactCtl, pageno, xid);
+ slotno = SimpleLruReadPage_ReadOnly(XactCtl, pageno, &xid);
byteptr = XactCtl->shared->page_buffer[slotno] + byteno;
status = (*byteptr >> bshift) & CLOG_XACT_BITMASK;
@@ -807,6 +807,7 @@ CLOGShmemInit(void)
Assert(transaction_buffers != 0);
XactCtl->PagePrecedes = CLOGPagePrecedes;
+ XactCtl->errdetail_for_io_error = clog_errdetail_for_io_error;
SimpleLruInit(XactCtl, "transaction", CLOGShmemBuffers(), CLOG_LSNS_PER_PAGE,
"pg_xact", LWTRANCHE_XACT_BUFFER,
LWTRANCHE_XACT_SLRU, SYNC_HANDLER_CLOG, false);
@@ -882,7 +883,7 @@ TrimCLOG(void)
int slotno;
char *byteptr;
- slotno = SimpleLruReadPage(XactCtl, pageno, false, xid);
+ slotno = SimpleLruReadPage(XactCtl, pageno, false, &xid);
byteptr = XactCtl->shared->page_buffer[slotno] + byteno;
/* Zero so-far-unused positions in the current byte */
@@ -1033,6 +1034,14 @@ CLOGPagePrecedes(int64 page1, int64 page2)
TransactionIdPrecedes(xid1, xid2 + CLOG_XACTS_PER_PAGE - 1));
}
+static int
+clog_errdetail_for_io_error(const void *opaque_data)
+{
+ TransactionId xid = *(const TransactionId *) opaque_data;
+
+ return errdetail("Could not access commit status of transaction %u.", xid);
+}
+
/*
* Write a TRUNCATE xlog record
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 6fa2178f1dd..36219dd13cc 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -115,6 +115,7 @@ static void TransactionIdSetCommitTs(TransactionId xid, TimestampTz ts,
ReplOriginId nodeid, int slotno);
static void error_commit_ts_disabled(void);
static bool CommitTsPagePrecedes(int64 page1, int64 page2);
+static int commit_ts_errdetail_for_io_error(const void *opaque_data);
static void ActivateCommitTs(void);
static void DeactivateCommitTs(void);
static void WriteTruncateXlogRec(int64 pageno, TransactionId oldestXid);
@@ -227,7 +228,7 @@ SetXidCommitTsInPage(TransactionId xid, int nsubxids,
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(CommitTsCtl, pageno, true, xid);
+ slotno = SimpleLruReadPage(CommitTsCtl, pageno, true, &xid);
TransactionIdSetCommitTs(xid, ts, nodeid, slotno);
for (i = 0; i < nsubxids; i++)
@@ -332,7 +333,7 @@ TransactionIdGetCommitTsData(TransactionId xid, TimestampTz *ts,
}
/* lock is acquired by SimpleLruReadPage_ReadOnly */
- slotno = SimpleLruReadPage_ReadOnly(CommitTsCtl, pageno, xid);
+ slotno = SimpleLruReadPage_ReadOnly(CommitTsCtl, pageno, &xid);
memcpy(&entry,
CommitTsCtl->shared->page_buffer[slotno] +
SizeOfCommitTimestampEntry * entryno,
@@ -551,6 +552,7 @@ CommitTsShmemInit(void)
Assert(commit_timestamp_buffers != 0);
CommitTsCtl->PagePrecedes = CommitTsPagePrecedes;
+ CommitTsCtl->errdetail_for_io_error = commit_ts_errdetail_for_io_error;
SimpleLruInit(CommitTsCtl, "commit_timestamp", CommitTsShmemBuffers(), 0,
"pg_commit_ts", LWTRANCHE_COMMITTS_BUFFER,
LWTRANCHE_COMMITTS_SLRU,
@@ -959,6 +961,13 @@ CommitTsPagePrecedes(int64 page1, int64 page2)
TransactionIdPrecedes(xid1, xid2 + COMMIT_TS_XACTS_PER_PAGE - 1));
}
+static int
+commit_ts_errdetail_for_io_error(const void *opaque_data)
+{
+ TransactionId xid = *(const TransactionId *) opaque_data;
+
+ return errdetail("Could not access commit timestamp of transaction %u.", xid);
+}
/*
* Write a TRUNCATE xlog record
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 8a5c9818ed6..2ae252fd05a 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -315,6 +315,8 @@ static void mXactCachePut(MultiXactId multi, int nmembers,
/* management of SLRU infrastructure */
static bool MultiXactOffsetPagePrecedes(int64 page1, int64 page2);
static bool MultiXactMemberPagePrecedes(int64 page1, int64 page2);
+static int MultiXactOffsetIoErrorDetail(const void *opaque_data);
+static int MultiXactMemberIoErrorDetail(const void *opaque_data);
static void ExtendMultiXactOffset(MultiXactId multi);
static void ExtendMultiXactMember(MultiXactOffset offset, int nmembers);
static void SetOldestOffset(void);
@@ -836,7 +838,7 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
* enough that a MultiXactId is really involved. Perhaps someday we'll
* take the trouble to generalize the slru.c error reporting code.
*/
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, multi);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, &multi);
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
offptr += entryno;
@@ -865,7 +867,7 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
lock = SimpleLruGetBankLock(MultiXactOffsetCtl, next_pageno);
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, next_pageno, true, next);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, next_pageno, true, &next);
next_offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
next_offptr += next_entryno;
}
@@ -919,7 +921,8 @@ RecordNewMultiXact(MultiXactId multi, MultiXactOffset offset,
LWLockAcquire(lock, LW_EXCLUSIVE);
prevlock = lock;
}
- slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, multi);
+ slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true,
+ &offset);
prev_pageno = pageno;
}
@@ -1244,7 +1247,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
LWLockAcquire(lock, LW_EXCLUSIVE);
/* read this multi's offset */
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, multi);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, &multi);
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
offptr += entryno;
offset = *offptr;
@@ -1282,7 +1285,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
LWLockAcquire(newlock, LW_EXCLUSIVE);
lock = newlock;
}
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, tmpMXact);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, &tmpMXact);
}
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
@@ -1347,7 +1350,8 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
lock = newlock;
}
- slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, multi);
+ slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true,
+ &offset);
prev_pageno = pageno;
}
@@ -1779,6 +1783,9 @@ MultiXactShmemInit(void)
MultiXactOffsetCtl->PagePrecedes = MultiXactOffsetPagePrecedes;
MultiXactMemberCtl->PagePrecedes = MultiXactMemberPagePrecedes;
+ MultiXactOffsetCtl->errdetail_for_io_error = MultiXactOffsetIoErrorDetail;
+ MultiXactMemberCtl->errdetail_for_io_error = MultiXactMemberIoErrorDetail;
+
SimpleLruInit(MultiXactOffsetCtl,
"multixact_offset", multixact_offset_buffers, 0,
"pg_multixact/offsets", LWTRANCHE_MULTIXACTOFFSET_BUFFER,
@@ -1928,7 +1935,7 @@ TrimMultiXact(void)
if (entryno == 0 || nextMXact == FirstMultiXactId)
slotno = SimpleLruZeroPage(MultiXactOffsetCtl, pageno);
else
- slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, nextMXact);
+ slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, &nextMXact);
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
offptr += entryno;
@@ -1963,7 +1970,7 @@ TrimMultiXact(void)
LWLockAcquire(lock, LW_EXCLUSIVE);
memberoff = MXOffsetToMemberOffset(offset);
- slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, offset);
+ slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, &offset);
xidptr = (TransactionId *)
(MultiXactMemberCtl->shared->page_buffer[slotno] + memberoff);
@@ -2497,7 +2504,7 @@ find_multixact_start(MultiXactId multi, MultiXactOffset *result)
return false;
/* lock is acquired by SimpleLruReadPage_ReadOnly */
- slotno = SimpleLruReadPage_ReadOnly(MultiXactOffsetCtl, pageno, multi);
+ slotno = SimpleLruReadPage_ReadOnly(MultiXactOffsetCtl, pageno, &multi);
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
offptr += entryno;
offset = *offptr;
@@ -2808,6 +2815,23 @@ MultiXactMemberPagePrecedes(int64 page1, int64 page2)
return page1 < page2;
}
+static int
+MultiXactOffsetIoErrorDetail(const void *opaque_data)
+{
+ MultiXactId multixid = *(const MultiXactId *) opaque_data;
+
+ return errdetail("Could not access offset of multixact %u.", multixid);
+}
+
+static int
+MultiXactMemberIoErrorDetail(const void *opaque_data)
+{
+ MultiXactOffset mxoff = *(const MultiXactOffset *) opaque_data;
+
+ return errdetail("Could not access multixact member at offset %" PRIu64 ".",
+ mxoff);
+}
+
/*
* Decide which of two MultiXactIds is earlier.
*
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 549c7e3e64b..5d127cb915e 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -181,7 +181,8 @@ static void SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata);
static bool SlruPhysicalReadPage(SlruCtl ctl, int64 pageno, int slotno);
static bool SlruPhysicalWritePage(SlruCtl ctl, int64 pageno, int slotno,
SlruWriteAll fdata);
-static void SlruReportIOError(SlruCtl ctl, int64 pageno, TransactionId xid);
+static void SlruReportIOError(SlruCtl ctl, int64 pageno,
+ const void *opaque_data);
static int SlruSelectLRUPage(SlruCtl ctl, int64 pageno);
static bool SlruScanDirCbDeleteCutoff(SlruCtl ctl, char *filename,
@@ -515,8 +516,9 @@ SimpleLruWaitIO(SlruCtl ctl, int slotno)
* that modification of the page is safe. If write_ok is false then we
* will not return the page until it is not undergoing active I/O.
*
- * The passed-in xid is used only for error reporting, and may be
- * InvalidTransactionId if no specific xid is associated with the action.
+ * On error, the passed-in 'opaque_data' is passed to the
+ * 'errdetail_for_io_error' callback, to provide details on the operation that
+ * failed. It is only used for error reporting.
*
* Return value is the shared-buffer slot number now holding the page.
* The buffer's LRU access info is updated.
@@ -525,7 +527,7 @@ SimpleLruWaitIO(SlruCtl ctl, int slotno)
*/
int
SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
- TransactionId xid)
+ const void *opaque_data)
{
SlruShared shared = ctl->shared;
LWLock *banklock = SimpleLruGetBankLock(ctl, pageno);
@@ -601,7 +603,7 @@ SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
/* Now it's okay to ereport if we failed */
if (!ok)
- SlruReportIOError(ctl, pageno, xid);
+ SlruReportIOError(ctl, pageno, opaque_data);
SlruRecentlyUsed(shared, slotno);
@@ -617,8 +619,9 @@ SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
* The page number must correspond to an already-initialized page.
* The caller must intend only read-only access to the page.
*
- * The passed-in xid is used only for error reporting, and may be
- * InvalidTransactionId if no specific xid is associated with the action.
+ * On error, the passed-in 'opaque_data' is passed to the
+ * 'errdetail_for_io_error' callback, to provide details on the operation that
+ * failed. It is only used for error reporting.
*
* Return value is the shared-buffer slot number now holding the page.
* The buffer's LRU access info is updated.
@@ -627,7 +630,7 @@ SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
* It is unspecified whether the lock will be shared or exclusive.
*/
int
-SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno, TransactionId xid)
+SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno, const void *opaque_data)
{
SlruShared shared = ctl->shared;
LWLock *banklock = SimpleLruGetBankLock(ctl, pageno);
@@ -659,7 +662,7 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno, TransactionId xid)
LWLockRelease(banklock);
LWLockAcquire(banklock, LW_EXCLUSIVE);
- return SimpleLruReadPage(ctl, pageno, true, xid);
+ return SimpleLruReadPage(ctl, pageno, true, opaque_data);
}
/*
@@ -739,7 +742,7 @@ SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata)
/* Now it's okay to ereport if we failed */
if (!ok)
- SlruReportIOError(ctl, pageno, InvalidTransactionId);
+ SlruReportIOError(ctl, pageno, NULL);
/* If part of a checkpoint, count this as a SLRU buffer written. */
if (fdata)
@@ -793,14 +796,14 @@ SimpleLruDoesPhysicalPageExist(SlruCtl ctl, int64 pageno)
/* report error normally */
slru_errcause = SLRU_OPEN_FAILED;
slru_errno = errno;
- SlruReportIOError(ctl, pageno, 0);
+ SlruReportIOError(ctl, pageno, NULL);
}
if ((endpos = lseek(fd, 0, SEEK_END)) < 0)
{
slru_errcause = SLRU_SEEK_FAILED;
slru_errno = errno;
- SlruReportIOError(ctl, pageno, 0);
+ SlruReportIOError(ctl, pageno, NULL);
}
result = endpos >= (off_t) (offset + BLCKSZ);
@@ -1070,7 +1073,7 @@ SlruPhysicalWritePage(SlruCtl ctl, int64 pageno, int slotno, SlruWriteAll fdata)
* SlruPhysicalWritePage. Call this after cleaning up shared-memory state.
*/
static void
-SlruReportIOError(SlruCtl ctl, int64 pageno, TransactionId xid)
+SlruReportIOError(SlruCtl ctl, int64 pageno, const void *opaque_data)
{
int64 segno = pageno / SLRU_PAGES_PER_SEGMENT;
int rpageno = pageno % SLRU_PAGES_PER_SEGMENT;
@@ -1084,54 +1087,55 @@ SlruReportIOError(SlruCtl ctl, int64 pageno, TransactionId xid)
case SLRU_OPEN_FAILED:
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
- errdetail("Could not open file \"%s\": %m.", path)));
+ errmsg("could not open file \"%s\": %m", path),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
break;
case SLRU_SEEK_FAILED:
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
- errdetail("Could not seek in file \"%s\" to offset %d: %m.",
- path, offset)));
+ errmsg("could not seek in file \"%s\" to offset %d: %m",
+ path, offset),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
break;
case SLRU_READ_FAILED:
if (errno)
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
- errdetail("Could not read from file \"%s\" at offset %d: %m.",
- path, offset)));
+ errmsg("could not read from file \"%s\" at offset %d: %m",
+ path, offset),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
else
ereport(ERROR,
- (errmsg("could not access status of transaction %u", xid),
- errdetail("Could not read from file \"%s\" at offset %d: read too few bytes.", path, offset)));
+ (errmsg("could not read from file \"%s\" at offset %d: read too few bytes",
+ path, offset),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
break;
case SLRU_WRITE_FAILED:
if (errno)
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
- errdetail("Could not write to file \"%s\" at offset %d: %m.",
- path, offset)));
+ errmsg("Could not write to file \"%s\" at offset %d: %m",
+ path, offset),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
else
ereport(ERROR,
- (errmsg("could not access status of transaction %u", xid),
- errdetail("Could not write to file \"%s\" at offset %d: wrote too few bytes.",
- path, offset)));
+ (errmsg("Could not write to file \"%s\" at offset %d: wrote too few bytes.",
+ path, offset),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
break;
case SLRU_FSYNC_FAILED:
ereport(data_sync_elevel(ERROR),
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
- errdetail("Could not fsync file \"%s\": %m.",
- path)));
+ errmsg("could not fsync file \"%s\": %m",
+ path),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
break;
case SLRU_CLOSE_FAILED:
ereport(ERROR,
(errcode_for_file_access(),
- errmsg("could not access status of transaction %u", xid),
- errdetail("Could not close file \"%s\": %m.",
- path)));
+ errmsg("could not close file \"%s\": %m",
+ path),
+ opaque_data ? ctl->errdetail_for_io_error(opaque_data) : 0));
break;
default:
/* can't get here, we trust */
@@ -1411,7 +1415,7 @@ SimpleLruWriteAll(SlruCtl ctl, bool allow_redirtied)
}
}
if (!ok)
- SlruReportIOError(ctl, pageno, InvalidTransactionId);
+ SlruReportIOError(ctl, pageno, NULL);
/* Ensure that directory entries for new files are on disk. */
if (ctl->sync_handler != SYNC_HANDLER_NONE)
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index c0987f43f11..c6ce71fc703 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -75,6 +75,7 @@ static SlruCtlData SubTransCtlData;
static bool SubTransPagePrecedes(int64 page1, int64 page2);
+static int subtrans_errdetail_for_io_error(const void *opaque_data);
/*
@@ -95,7 +96,7 @@ SubTransSetParent(TransactionId xid, TransactionId parent)
lock = SimpleLruGetBankLock(SubTransCtl, pageno);
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(SubTransCtl, pageno, true, xid);
+ slotno = SimpleLruReadPage(SubTransCtl, pageno, true, &xid);
ptr = (TransactionId *) SubTransCtl->shared->page_buffer[slotno];
ptr += entryno;
@@ -135,7 +136,7 @@ SubTransGetParent(TransactionId xid)
/* lock is acquired by SimpleLruReadPage_ReadOnly */
- slotno = SimpleLruReadPage_ReadOnly(SubTransCtl, pageno, xid);
+ slotno = SimpleLruReadPage_ReadOnly(SubTransCtl, pageno, &xid);
ptr = (TransactionId *) SubTransCtl->shared->page_buffer[slotno];
ptr += entryno;
@@ -240,6 +241,7 @@ SUBTRANSShmemInit(void)
Assert(subtransaction_buffers != 0);
SubTransCtl->PagePrecedes = SubTransPagePrecedes;
+ SubTransCtl->errdetail_for_io_error = subtrans_errdetail_for_io_error;
SimpleLruInit(SubTransCtl, "subtransaction", SUBTRANSShmemBuffers(), 0,
"pg_subtrans", LWTRANCHE_SUBTRANS_BUFFER,
LWTRANCHE_SUBTRANS_SLRU, SYNC_HANDLER_NONE, false);
@@ -419,3 +421,11 @@ SubTransPagePrecedes(int64 page1, int64 page2)
return (TransactionIdPrecedes(xid1, xid2) &&
TransactionIdPrecedes(xid1, xid2 + SUBTRANS_XACTS_PER_PAGE - 1));
}
+
+static int
+subtrans_errdetail_for_io_error(const void *opaque_data)
+{
+ TransactionId xid = *(const TransactionId *) opaque_data;
+
+ return errdetail("Could not access subtransaction status of transaction %u.", xid);
+}
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 0b6d119dad0..5c9a56c3d40 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -570,6 +570,7 @@ bool Trace_notify = false;
int max_notify_queue_pages = 1048576;
/* local function prototypes */
+static int asyncQueueErrdetailForIoError(const void *opaque_data);
static inline int64 asyncQueuePageDiff(int64 p, int64 q);
static inline bool asyncQueuePagePrecedes(int64 p, int64 q);
static inline void GlobalChannelKeyInit(GlobalChannelKey *key, Oid dboid,
@@ -610,6 +611,15 @@ static uint32 notification_hash(const void *key, Size keysize);
static int notification_match(const void *key1, const void *key2, Size keysize);
static void ClearPendingActionsAndNotifies(void);
+static int
+asyncQueueErrdetailForIoError(const void *opaque_data)
+{
+ const QueuePosition *position = opaque_data;
+
+ return errdetail("Could not access async queue at page %" PRId64 ", offset %d.",
+ position->page, position->offset);
+}
+
/*
* Compute the difference between two queue page numbers.
* Previously this function accounted for a wraparound.
@@ -830,6 +840,7 @@ AsyncShmemInit(void)
* names are used in order to avoid wraparound.
*/
NotifyCtl->PagePrecedes = asyncQueuePagePrecedes;
+ NotifyCtl->errdetail_for_io_error = asyncQueueErrdetailForIoError;
SimpleLruInit(NotifyCtl, "notify", notify_buffers, 0,
"pg_notify", LWTRANCHE_NOTIFY_BUFFER, LWTRANCHE_NOTIFY_SLRU,
SYNC_HANDLER_NONE, true);
@@ -2068,8 +2079,7 @@ asyncQueueAddEntries(ListCell *nextNotify)
if (QUEUE_POS_IS_ZERO(queue_head))
slotno = SimpleLruZeroPage(NotifyCtl, pageno);
else
- slotno = SimpleLruReadPage(NotifyCtl, pageno, true,
- InvalidTransactionId);
+ slotno = SimpleLruReadPage(NotifyCtl, pageno, true, &queue_head);
/* Note we mark the page dirty before writing in it */
NotifyCtl->shared->page_dirty[slotno] = true;
@@ -2739,8 +2749,7 @@ asyncQueueProcessPageEntries(QueuePosition *current,
alignas(AsyncQueueEntry) char local_buf[QUEUE_PAGESIZE];
char *local_buf_end = local_buf;
- slotno = SimpleLruReadPage_ReadOnly(NotifyCtl, curpage,
- InvalidTransactionId);
+ slotno = SimpleLruReadPage_ReadOnly(NotifyCtl, curpage, current);
page_buffer = NotifyCtl->shared->page_buffer[slotno];
do
@@ -2998,8 +3007,7 @@ AsyncNotifyFreezeXids(TransactionId newFrozenXid)
lock = SimpleLruGetBankLock(NotifyCtl, pageno);
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(NotifyCtl, pageno, true,
- InvalidTransactionId);
+ slotno = SimpleLruReadPage(NotifyCtl, pageno, true, &pos);
page_buffer = NotifyCtl->shared->page_buffer[slotno];
curpage = pageno;
}
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index fe75ead3501..e72eb80a03f 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -442,6 +442,7 @@ static void ReleaseRWConflict(RWConflict conflict);
static void FlagSxactUnsafe(SERIALIZABLEXACT *sxact);
static bool SerialPagePrecedesLogically(int64 page1, int64 page2);
+static int serial_errdetail_for_io_error(const void *opaque_data);
static void SerialInit(void);
static void SerialAdd(TransactionId xid, SerCommitSeqNo minConflictCommitSeqNo);
static SerCommitSeqNo SerialGetMinConflictCommitSeqNo(TransactionId xid);
@@ -742,6 +743,14 @@ SerialPagePrecedesLogically(int64 page1, int64 page2)
TransactionIdPrecedes(xid1, xid2 + SERIAL_ENTRIESPERPAGE - 1));
}
+static int
+serial_errdetail_for_io_error(const void *opaque_data)
+{
+ TransactionId xid = *(const TransactionId *) opaque_data;
+
+ return errdetail("Could not access serializable CSN of transaction %u.", xid);
+}
+
#ifdef USE_ASSERT_CHECKING
static void
SerialPagePrecedesLogicallyUnitTests(void)
@@ -811,6 +820,7 @@ SerialInit(void)
* Set up SLRU management of the pg_serial data.
*/
SerialSlruCtl->PagePrecedes = SerialPagePrecedesLogically;
+ SerialSlruCtl->errdetail_for_io_error = serial_errdetail_for_io_error;
SimpleLruInit(SerialSlruCtl, "serializable",
serializable_buffers, 0, "pg_serial",
LWTRANCHE_SERIAL_BUFFER, LWTRANCHE_SERIAL_SLRU,
@@ -930,7 +940,7 @@ SerialAdd(TransactionId xid, SerCommitSeqNo minConflictCommitSeqNo)
else
{
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(SerialSlruCtl, targetPage, true, xid);
+ slotno = SimpleLruReadPage(SerialSlruCtl, targetPage, true, &xid);
}
SerialValue(slotno, xid) = minConflictCommitSeqNo;
@@ -974,7 +984,7 @@ SerialGetMinConflictCommitSeqNo(TransactionId xid)
* but will return with that lock held, which must then be released.
*/
slotno = SimpleLruReadPage_ReadOnly(SerialSlruCtl,
- SerialPage(xid), xid);
+ SerialPage(xid), &xid);
val = SerialValue(slotno, xid);
LWLockRelease(SimpleLruGetBankLock(SerialSlruCtl, SerialPage(xid)));
return val;
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 4cb8f478fce..dcb59ef8761 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -13,6 +13,7 @@
#ifndef SLRU_H
#define SLRU_H
+#include "access/transam.h"
#include "access/xlogdefs.h"
#include "storage/lwlock.h"
#include "storage/sync.h"
@@ -141,6 +142,16 @@ typedef struct SlruCtlData
*/
bool (*PagePrecedes) (int64, int64);
+ /*
+ * Callback to provide more details on an I/O error. This is called as
+ * part of ereport(), and the callback function is expected to call
+ * errdetail() to provide more details.
+ *
+ * The opaque_data argument here is the argument passed to the
+ * SimpleLruReadPage() function.
+ */
+ int (*errdetail_for_io_error) (const void *opaque_data);
+
/*
* Dir is set during SimpleLruInit and does not change thereafter. Since
* it's always the same, it doesn't need to be in shared memory.
@@ -174,9 +185,9 @@ extern void SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
extern int SimpleLruZeroPage(SlruCtl ctl, int64 pageno);
extern void SimpleLruZeroAndWritePage(SlruCtl ctl, int64 pageno);
extern int SimpleLruReadPage(SlruCtl ctl, int64 pageno, bool write_ok,
- TransactionId xid);
+ const void *opaque_data);
extern int SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno,
- TransactionId xid);
+ const void *opaque_data);
extern void SimpleLruWritePage(SlruCtl ctl, int slotno);
extern void SimpleLruWriteAll(SlruCtl ctl, bool allow_redirtied);
#ifdef USE_ASSERT_CHECKING
diff --git a/src/test/modules/test_slru/expected/test_slru.out b/src/test/modules/test_slru/expected/test_slru.out
index 185c56e5d62..7ae1b0d4a98 100644
--- a/src/test/modules/test_slru/expected/test_slru.out
+++ b/src/test/modules/test_slru/expected/test_slru.out
@@ -23,6 +23,10 @@ SELECT test_slru_page_exists(12345);
t
(1 row)
+-- Test read failure
+SELECT test_slru_page_read(54321, false, '123'::xid);
+ERROR: could not open file "pg_test_slru/0000000000006A1": No such file or directory
+DETAIL: Could not access test_slru entry 123.
-- 48 extra pages
SELECT count(test_slru_page_write(a, 'Test SLRU'))
FROM generate_series(12346, 12393, 1) as a;
diff --git a/src/test/modules/test_slru/sql/test_slru.sql b/src/test/modules/test_slru/sql/test_slru.sql
index b1b376581ab..c6454c5bf82 100644
--- a/src/test/modules/test_slru/sql/test_slru.sql
+++ b/src/test/modules/test_slru/sql/test_slru.sql
@@ -5,6 +5,9 @@ SELECT test_slru_page_write(12345, 'Test SLRU');
SELECT test_slru_page_read(12345);
SELECT test_slru_page_exists(12345);
+-- Test read failure
+SELECT test_slru_page_read(54321, false, '123'::xid);
+
-- 48 extra pages
SELECT count(test_slru_page_write(a, 'Test SLRU'))
FROM generate_series(12346, 12393, 1) as a;
diff --git a/src/test/modules/test_slru/test_slru--1.0.sql b/src/test/modules/test_slru/test_slru--1.0.sql
index abecb5e2183..2148e7204ec 100644
--- a/src/test/modules/test_slru/test_slru--1.0.sql
+++ b/src/test/modules/test_slru/test_slru--1.0.sql
@@ -7,7 +7,7 @@ CREATE OR REPLACE FUNCTION test_slru_page_writeall() RETURNS VOID
AS 'MODULE_PATHNAME', 'test_slru_page_writeall' LANGUAGE C;
CREATE OR REPLACE FUNCTION test_slru_page_sync(bigint) RETURNS VOID
AS 'MODULE_PATHNAME', 'test_slru_page_sync' LANGUAGE C;
-CREATE OR REPLACE FUNCTION test_slru_page_read(bigint, bool DEFAULT true) RETURNS text
+CREATE OR REPLACE FUNCTION test_slru_page_read(bigint, bool DEFAULT true, xid DEFAULT '0') RETURNS text
AS 'MODULE_PATHNAME', 'test_slru_page_read' LANGUAGE C;
CREATE OR REPLACE FUNCTION test_slru_page_readonly(bigint) RETURNS text
AS 'MODULE_PATHNAME', 'test_slru_page_readonly' LANGUAGE C;
diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c
index 4dc74e19620..e4bd2af0bf5 100644
--- a/src/test/modules/test_slru/test_slru.c
+++ b/src/test/modules/test_slru/test_slru.c
@@ -93,14 +93,14 @@ test_slru_page_read(PG_FUNCTION_ARGS)
{
int64 pageno = PG_GETARG_INT64(0);
bool write_ok = PG_GETARG_BOOL(1);
+ TransactionId xid = PG_GETARG_TRANSACTIONID(2);
char *data = NULL;
int slotno;
LWLock *lock = SimpleLruGetBankLock(TestSlruCtl, pageno);
/* find page in buffers, reading it if necessary */
LWLockAcquire(lock, LW_EXCLUSIVE);
- slotno = SimpleLruReadPage(TestSlruCtl, pageno,
- write_ok, InvalidTransactionId);
+ slotno = SimpleLruReadPage(TestSlruCtl, pageno, write_ok, &xid);
data = (char *) TestSlruCtl->shared->page_buffer[slotno];
LWLockRelease(lock);
@@ -118,7 +118,7 @@ test_slru_page_readonly(PG_FUNCTION_ARGS)
/* find page in buffers, reading it if necessary */
slotno = SimpleLruReadPage_ReadOnly(TestSlruCtl,
pageno,
- InvalidTransactionId);
+ NULL);
Assert(LWLockHeldByMe(lock));
data = (char *) TestSlruCtl->shared->page_buffer[slotno];
LWLockRelease(lock);
@@ -210,6 +210,14 @@ test_slru_page_precedes_logically(int64 page1, int64 page2)
return page1 < page2;
}
+static int
+test_slru_errdetail_for_io_error(const void *opaque_data)
+{
+ TransactionId xid = *(const TransactionId *) opaque_data;
+
+ return errdetail("Could not access test_slru entry %u.", xid);
+}
+
static void
test_slru_shmem_startup(void)
{
@@ -245,6 +253,7 @@ test_slru_shmem_startup(void)
}
TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically;
+ TestSlruCtl->errdetail_for_io_error = test_slru_errdetail_for_io_error;
SimpleLruInit(TestSlruCtl, "TestSLRU",
NUM_TEST_BUFFERS, 0, slru_dir_name,
test_buffer_tranche_id, test_tranche_id, SYNC_HANDLER_NONE,
--
2.47.3
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Rework SLRU I/O errors handle
@ 2026-03-06 17:08 Maxim Orlov <[email protected]>
parent: Heikki Linnakangas <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Maxim Orlov @ 2026-03-06 17:08 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; Postgres hackers <[email protected]>
I don't know what's happening with my mail, but I haven't
received the previous letters.
Anyway, v4 looks good to me.
Perhaps the extra double line following clog_errdetail_for_io_error
is unnecessary? But as always, to your taste.
--
Best regards,
Maxim Orlov.
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Rework SLRU I/O errors handle
@ 2026-03-13 14:32 Heikki Linnakangas <[email protected]>
parent: Maxim Orlov <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Heikki Linnakangas @ 2026-03-13 14:32 UTC (permalink / raw)
To: Maxim Orlov <[email protected]>; +Cc: Álvaro Herrera <[email protected]>; Postgres hackers <[email protected]>
On 11/03/2026 12:51, Heikki Linnakangas wrote:
> On 06/03/2026 19:08, Maxim Orlov wrote:
>> I don't know what's happening with my mail, but I haven't
>> received the previous letters.
>>
>> Anyway, v4 looks good to me.
>> Perhaps the extra double line following clog_errdetail_for_io_error
>> is unnecessary? But as always, to your taste.
>
> Thanks. I did one more iteration on this: I realized that the error we
> now printed for errors on pg_multixact/members always printed the
> failing offset, whereas before this patch we usually printed the failing
> *multixid* that the member is part of. Printing the multixid might
> actually be more useful; the offset can more easily be deduced from the
> segment filename and physical offset that is printed anyway, but it's
> harder to know which multixid it belongs to. This printing the
> originating multixid seems useful. If things go badly wrong and you need
> to do manual debugging of a corrupted database, the multixid can more
> easily be compared with the xmax in the heap and with pg_waldump output,
> for example.
>
> We can print both, per attached, which is even better. This is perhaps
> overkill, but then again, if you hit an error like this, you really
> appreciate any extra information you can get.
I hear no objections, so committed that. Thanks!
- Heikki
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-03-13 14:32 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-02-26 14:26 Rework SLRU I/O errors handle Maxim Orlov <[email protected]>
2026-03-06 14:43 ` Heikki Linnakangas <[email protected]>
2026-03-06 16:22 Re: Rework SLRU I/O errors handle Heikki Linnakangas <[email protected]>
2026-03-06 16:38 ` Heikki Linnakangas <[email protected]>
2026-03-06 16:49 ` Heikki Linnakangas <[email protected]>
2026-03-06 17:08 ` Maxim Orlov <[email protected]>
2026-03-13 14:32 ` Heikki Linnakangas <[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