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
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-t48oe5xc...@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
