Skip to content

Commit ed57cac

Browse files
jeff-davisjeff-davis
committed
pg_walinspect: fix case where flush LSN is in the middle of a record.
Instability in the test for pg_walinspect revealed that pg_get_wal_records_info_till_end_of_wal(x) would try to decode all the records with a start LSN earlier than the flush LSN, even though that might include a partial record at the end of the range. In that case, read_local_xlog_page_no_wait() would return NULL when it tried to read past the flush LSN, which would be interpreted as an error by the caller. That caused a test failure only on a BF animal that had been restarted recently, but could be expected to happen in the wild quite easily depending on the alignment of various parameters. Fix by using private data in read_local_xlog_page_no_wait() to signal end-of-wal to the caller, so that it can be properly distinguished from a real error. Discussion: https://postgr.es/m/Ymd/e5eeZMNAkrXo%40paquier.xyz Discussion: https://postgr.es/m/111657.1650910309@sss.pgh.pa.us Authors: Thomas Munro, Bharath Rupireddy.
1 parent ccd10a9 commit ed57cac

File tree

3 files changed

+50
-26
lines changed

3 files changed

+50
-26
lines changed

‎contrib/pg_walinspect/pg_walinspect.c

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ static XLogReaderState *
8989
InitXLogReaderState(XLogRecPtr lsn, XLogRecPtr *first_record)
9090
{
9191
XLogReaderState *xlogreader;
92+
ReadLocalXLogPageNoWaitPrivate *private_data;
9293

9394
/*
9495
* Reading WAL below the first page of the first segments isn't allowed.
@@ -100,11 +101,14 @@ InitXLogReaderState(XLogRecPtr lsn, XLogRecPtr *first_record)
100101
(errmsg("could not read WAL at LSN %X/%X",
101102
LSN_FORMAT_ARGS(lsn))));
102103

104+
private_data = (ReadLocalXLogPageNoWaitPrivate *)
105+
palloc0(sizeof(ReadLocalXLogPageNoWaitPrivate));
106+
103107
xlogreader = XLogReaderAllocate(wal_segment_size, NULL,
104108
XL_ROUTINE(.page_read = &read_local_xlog_page_no_wait,
105109
.segment_open = &wal_segment_open,
106110
.segment_close = &wal_segment_close),
107-
NULL);
111+
private_data);
108112

109113
if (xlogreader == NULL)
110114
ereport(ERROR,
@@ -132,7 +136,8 @@ InitXLogReaderState(XLogRecPtr lsn, XLogRecPtr *first_record)
132136
*
133137
* We guard against ordinary errors trying to read WAL that hasn't been
134138
* written yet by limiting end_lsn to the flushed WAL, but that can also
135-
* encounter errors if the flush pointer falls in the middle of a record.
139+
* encounter errors if the flush pointer falls in the middle of a record. In
140+
* that case we'll return NULL.
136141
*/
137142
static XLogRecord *
138143
ReadNextXLogRecord(XLogReaderState *xlogreader, XLogRecPtr first_record)
@@ -144,6 +149,15 @@ ReadNextXLogRecord(XLogReaderState *xlogreader, XLogRecPtr first_record)
144149

145150
if (record == NULL)
146151
{
152+
ReadLocalXLogPageNoWaitPrivate *private_data;
153+
154+
/* return NULL, if end of WAL is reached */
155+
private_data = (ReadLocalXLogPageNoWaitPrivate *)
156+
xlogreader->private_data;
157+
158+
if (private_data->end_of_wal)
159+
return NULL;
160+
147161
if (errormsg)
148162
ereport(ERROR,
149163
(errcode_for_file_access(),
@@ -246,14 +260,19 @@ pg_get_wal_record_info(PG_FUNCTION_ARGS)
246260

247261
xlogreader = InitXLogReaderState(lsn, &first_record);
248262

249-
(void) ReadNextXLogRecord(xlogreader, first_record);
263+
if (!ReadNextXLogRecord(xlogreader, first_record))
264+
ereport(ERROR,
265+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
266+
errmsg("could not read WAL at %X/%X",
267+
LSN_FORMAT_ARGS(first_record))));
250268

251269
MemSet(values, 0, sizeof(values));
252270
MemSet(nulls, 0, sizeof(nulls));
253271

254272
GetWALRecordInfo(xlogreader, first_record, values, nulls,
255273
PG_GET_WAL_RECORD_INFO_COLS);
256274

275+
pfree(xlogreader->private_data);
257276
XLogReaderFree(xlogreader);
258277

259278
tuple = heap_form_tuple(tupdesc, values, nulls);
@@ -327,26 +346,19 @@ GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
327346
MemSet(values, 0, sizeof(values));
328347
MemSet(nulls, 0, sizeof(nulls));
329348

330-
for (;;)
349+
while (ReadNextXLogRecord(xlogreader, first_record) &&
350+
xlogreader->EndRecPtr <= end_lsn)
331351
{
332-
(void) ReadNextXLogRecord(xlogreader, first_record);
333-
334-
if (xlogreader->EndRecPtr <= end_lsn)
335-
{
336-
GetWALRecordInfo(xlogreader, xlogreader->currRecPtr, values, nulls,
337-
PG_GET_WAL_RECORDS_INFO_COLS);
352+
GetWALRecordInfo(xlogreader, xlogreader->currRecPtr, values, nulls,
353+
PG_GET_WAL_RECORDS_INFO_COLS);
338354

339-
tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc,
340-
values, nulls);
341-
}
342-
343-
/* if we read up to end_lsn, we're done */
344-
if (xlogreader->EndRecPtr >= end_lsn)
345-
break;
355+
tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc,
356+
values, nulls);
346357

347358
CHECK_FOR_INTERRUPTS();
348359
}
349360

361+
pfree(xlogreader->private_data);
350362
XLogReaderFree(xlogreader);
351363

352364
#undef PG_GET_WAL_RECORDS_INFO_COLS
@@ -555,20 +567,15 @@ GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
555567

556568
MemSet(&stats, 0, sizeof(stats));
557569

558-
for (;;)
570+
while (ReadNextXLogRecord(xlogreader, first_record) &&
571+
xlogreader->EndRecPtr <= end_lsn)
559572
{
560-
(void) ReadNextXLogRecord(xlogreader, first_record);
561-
562-
if (xlogreader->EndRecPtr <= end_lsn)
563-
XLogRecStoreStats(&stats, xlogreader);
564-
565-
/* if we read up to end_lsn, we're done */
566-
if (xlogreader->EndRecPtr >= end_lsn)
567-
break;
573+
XLogRecStoreStats(&stats, xlogreader);
568574

569575
CHECK_FOR_INTERRUPTS();
570576
}
571577

578+
pfree(xlogreader->private_data);
572579
XLogReaderFree(xlogreader);
573580

574581
MemSet(values, 0, sizeof(values));

‎src/backend/access/transam/xlogutils.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -957,7 +957,18 @@ read_local_xlog_page_guts(XLogReaderState *state, XLogRecPtr targetPagePtr,
957957

958958
/* If asked, let's not wait for future WAL. */
959959
if (!wait_for_wal)
960+
{
961+
ReadLocalXLogPageNoWaitPrivate *private_data;
962+
963+
/*
964+
* Inform the caller of read_local_xlog_page_no_wait that the
965+
* end of WAL has been reached.
966+
*/
967+
private_data = (ReadLocalXLogPageNoWaitPrivate *)
968+
state->private_data;
969+
private_data->end_of_wal = true;
960970
break;
971+
}
961972

962973
CHECK_FOR_INTERRUPTS();
963974
pg_usleep(1000L);

‎src/include/access/xlogutils.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,12 @@ typedef enum
7575
* need to be replayed) */
7676
} XLogRedoAction;
7777

78+
/* Private data of the read_local_xlog_page_no_wait callback. */
79+
typedef struct ReadLocalXLogPageNoWaitPrivate
80+
{
81+
bool end_of_wal; /* true, when end of WAL is reached */
82+
} ReadLocalXLogPageNoWaitPrivate;
83+
7884
extern XLogRedoAction XLogReadBufferForRedo(XLogReaderState *record,
7985
uint8 buffer_id, Buffer *buf);
8086
extern Buffer XLogInitBufferForRedo(XLogReaderState *record, uint8 block_id);

0 commit comments

Comments
 (0)