public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v5 3/3] Change policy of XLog read-buffer allocation
17+ messages / 5 participants
[nested] [flat]
* [PATCH v11 4/4] Change policy of XLog read-buffer allocation
@ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw)
Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but
actually it'd be the responsibility to the callers of XLogReadRecord,
which now actually reads in pages. This patch does that.
---
src/backend/access/transam/twophase.c | 2 ++
src/backend/access/transam/xlog.c | 2 ++
src/backend/access/transam/xlogreader.c | 3 ---
src/backend/replication/logical/logical.c | 2 ++
src/bin/pg_rewind/parsexlog.c | 6 ++++++
src/bin/pg_waldump/pg_waldump.c | 2 ++
6 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index b6c7125aa1..e69b6abae7 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1336,6 +1336,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
@@ -1365,6 +1366,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
*buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader));
memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader));
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 364b1948e4..df1e9a5830 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6352,6 +6352,7 @@ StartupXLOG(void)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
xlogreader->system_identifier = ControlFile->system_identifier;
/*
@@ -7736,6 +7737,7 @@ StartupXLOG(void)
close(readFile);
readFile = -1;
}
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
/*
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 9acc8d03e2..cb47aa239a 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -106,7 +106,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir)
MCXT_ALLOC_NO_OOM);
if (!state->errormsg_buf)
{
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -119,7 +118,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir)
if (!allocate_recordbuf(state, 0))
{
pfree(state->errormsg_buf);
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -143,7 +141,6 @@ XLogReaderFree(XLogReaderState *state)
pfree(state->errormsg_buf);
if (state->readRecordBuf)
pfree(state->readRecordBuf);
- pfree(state->readBuf);
pfree(state);
}
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index d3a92044b5..59671f99f0 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -174,6 +174,7 @@ StartupDecodingContext(List *output_plugin_options,
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));
+ ctx->reader->readBuf = palloc(XLOG_BLCKSZ);
ctx->read_page = read_page;
ctx->reorder = ReorderBufferAllocate();
@@ -519,6 +520,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx)
ReorderBufferFree(ctx->reorder);
FreeSnapshotBuilder(ctx->snapshot_builder);
+ pfree(ctx->reader->readBuf);
XLogReaderFree(ctx->reader);
MemoryContextDelete(ctx->context);
}
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index 4236ea1d12..ee380b25e1 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -58,6 +58,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
do
{
@@ -90,6 +91,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
} while (xlogreader->ReadRecPtr != endpoint);
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -113,6 +115,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) ==
XLREAD_NEED_DATA)
@@ -131,6 +134,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
}
endptr = xlogreader->EndRecPtr;
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -172,6 +176,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
searchptr = forkptr;
for (;;)
@@ -220,6 +225,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
searchptr = record->xl_prev;
}
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index c7c05edcda..5a40047733 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1035,6 +1035,7 @@ main(int argc, char **argv)
xlogreader_state = XLogReaderAllocate(WalSegSz, waldir);
if (!xlogreader_state)
fatal_error("out of memory");
+ xlogreader_state->readBuf = palloc(XLOG_BLCKSZ);
/* first find a valid recptr to start from */
first_record = XLogFindNextRecord(xlogreader_state, private.startptr,
@@ -1115,6 +1116,7 @@ main(int argc, char **argv)
(uint32) xlogreader_state->ReadRecPtr,
errormsg);
+ pfree(xlogreader_state->readBuf);
XLogReaderFree(xlogreader_state);
return EXIT_SUCCESS;
--
2.23.0
----Next_Part(Wed_Nov_27_12_09_23_2019_240)----
^ permalink raw reply [nested|flat] 17+ messages in thread
* [PATCH v9 4/4] Change policy of XLog read-buffer allocation
@ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw)
Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but
actually it'd be the responsibility to the callers of XLogReadRecord,
which now actually reads in pages. This patch does that.
---
src/backend/access/transam/twophase.c | 2 ++
src/backend/access/transam/xlog.c | 2 ++
src/backend/access/transam/xlogreader.c | 3 ---
src/backend/replication/logical/logical.c | 2 ++
src/bin/pg_rewind/parsexlog.c | 6 ++++++
src/bin/pg_waldump/pg_waldump.c | 2 ++
6 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index ced11bbdae..9e80a2cdb1 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1391,6 +1391,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
@@ -1420,6 +1421,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
*buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader));
memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader));
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e8a4c7916b..0bc7549b07 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6355,6 +6355,7 @@ StartupXLOG(void)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
xlogreader->system_identifier = ControlFile->system_identifier;
/*
@@ -7739,6 +7740,7 @@ StartupXLOG(void)
close(readFile);
readFile = -1;
}
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
/*
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index ce7fd55496..12a097c3eb 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -104,7 +104,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir)
MCXT_ALLOC_NO_OOM);
if (!state->errormsg_buf)
{
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -117,7 +116,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir)
if (!allocate_recordbuf(state, 0))
{
pfree(state->errormsg_buf);
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -141,7 +139,6 @@ XLogReaderFree(XLogReaderState *state)
pfree(state->errormsg_buf);
if (state->readRecordBuf)
pfree(state->readRecordBuf);
- pfree(state->readBuf);
pfree(state);
}
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index db3e8f9dc0..99cb9bcc54 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -178,6 +178,7 @@ StartupDecodingContext(List *output_plugin_options,
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));
+ ctx->reader->readBuf = palloc(XLOG_BLCKSZ);
ctx->read_page = read_page;
ctx->reorder = ReorderBufferAllocate();
@@ -523,6 +524,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx)
ReorderBufferFree(ctx->reorder);
FreeSnapshotBuilder(ctx->snapshot_builder);
+ pfree(ctx->reader->readBuf);
XLogReaderFree(ctx->reader);
MemoryContextDelete(ctx->context);
}
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index c5e5d75a87..d0e408c0a8 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -60,6 +60,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
do
{
@@ -92,6 +93,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
} while (xlogreader->ReadRecPtr != endpoint);
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -115,6 +117,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) ==
XLREAD_NEED_DATA)
@@ -133,6 +136,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
}
endptr = xlogreader->EndRecPtr;
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -174,6 +178,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
searchptr = forkptr;
for (;;)
@@ -222,6 +227,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
searchptr = record->xl_prev;
}
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index ee49712830..e17e66e688 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1094,6 +1094,7 @@ main(int argc, char **argv)
xlogreader_state = XLogReaderAllocate(WalSegSz, waldir);
if (!xlogreader_state)
fatal_error("out of memory");
+ xlogreader_state->readBuf = palloc(XLOG_BLCKSZ);
/* first find a valid recptr to start from */
first_record = XLogFindNextRecord(xlogreader_state, private.startptr,
@@ -1174,6 +1175,7 @@ main(int argc, char **argv)
(uint32) xlogreader_state->ReadRecPtr,
errormsg);
+ pfree(xlogreader_state->readBuf);
XLogReaderFree(xlogreader_state);
return EXIT_SUCCESS;
--
2.23.0
----Next_Part(Thu_Oct_24_14_51_01_2019_720)----
^ permalink raw reply [nested|flat] 17+ messages in thread
* [PATCH v6 4/4] Change policy of XLog read-buffer allocation
@ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw)
Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but
actually it'd be the responsibility to the callers of XLogReadRecord,
which now actually reads in pages. This patch does that.
---
src/backend/access/transam/twophase.c | 2 ++
src/backend/access/transam/xlog.c | 2 ++
src/backend/access/transam/xlogreader.c | 18 ------------------
src/backend/replication/logical/logical.c | 2 ++
src/bin/pg_rewind/parsexlog.c | 6 ++++++
src/bin/pg_waldump/pg_waldump.c | 2 ++
6 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index a736504d62..358951c232 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1392,6 +1392,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
@@ -1421,6 +1422,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
*buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader));
memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader));
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c20d3b1418..a199600399 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6349,6 +6349,7 @@ StartupXLOG(void)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
xlogreader->system_identifier = ControlFile->system_identifier;
/*
@@ -7721,6 +7722,7 @@ StartupXLOG(void)
close(readFile);
readFile = -1;
}
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
/*
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index e00fa270ae..1f98dd0168 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -80,27 +80,11 @@ XLogReaderAllocate(int wal_segment_size)
state->max_block_id = -1;
- /*
- * Permanently allocate readBuf. We do it this way, rather than just
- * making a static array, for two reasons: (1) no need to waste the
- * storage in most instantiations of the backend; (2) a static char array
- * isn't guaranteed to have any particular alignment, whereas
- * palloc_extended() will provide MAXALIGN'd storage.
- */
- state->readBuf = (char *) palloc_extended(XLOG_BLCKSZ,
- MCXT_ALLOC_NO_OOM);
- if (!state->readBuf)
- {
- pfree(state);
- return NULL;
- }
-
state->wal_segment_size = wal_segment_size;
state->errormsg_buf = palloc_extended(MAX_ERRORMSG_LEN + 1,
MCXT_ALLOC_NO_OOM);
if (!state->errormsg_buf)
{
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -113,7 +97,6 @@ XLogReaderAllocate(int wal_segment_size)
if (!allocate_recordbuf(state, 0))
{
pfree(state->errormsg_buf);
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -137,7 +120,6 @@ XLogReaderFree(XLogReaderState *state)
pfree(state->errormsg_buf);
if (state->readRecordBuf)
pfree(state->readRecordBuf);
- pfree(state->readBuf);
pfree(state);
}
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index 11e52e4c01..ea027caa69 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -178,6 +178,7 @@ StartupDecodingContext(List *output_plugin_options,
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));
+ ctx->reader->readBuf = palloc(XLOG_BLCKSZ);
ctx->read_page = read_page;
ctx->reorder = ReorderBufferAllocate();
@@ -523,6 +524,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx)
ReorderBufferFree(ctx->reorder);
FreeSnapshotBuilder(ctx->snapshot_builder);
+ pfree(ctx->reader->readBuf);
XLogReaderFree(ctx->reader);
MemoryContextDelete(ctx->context);
}
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index ff26b30f82..c60267e87e 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -60,6 +60,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
do
{
@@ -92,6 +93,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
} while (xlogreader->ReadRecPtr != endpoint);
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -115,6 +117,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
xlogreader = XLogReaderAllocate(WalSegSz);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) ==
XLREAD_NEED_DATA)
@@ -133,6 +136,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
}
endptr = xlogreader->EndRecPtr;
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -174,6 +178,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
searchptr = forkptr;
for (;;)
@@ -222,6 +227,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
searchptr = record->xl_prev;
}
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index a61a5a91cb..5eba802720 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1108,6 +1108,7 @@ main(int argc, char **argv)
xlogreader_state = XLogReaderAllocate(WalSegSz);
if (!xlogreader_state)
fatal_error("out of memory");
+ xlogreader_state->readBuf = palloc(XLOG_BLCKSZ);
/* first find a valid recptr to start from */
first_record = XLogFindNextRecord(xlogreader_state, private.startptr,
@@ -1188,6 +1189,7 @@ main(int argc, char **argv)
(uint32) xlogreader_state->ReadRecPtr,
errormsg);
+ pfree(xlogreader_state->readBuf);
XLogReaderFree(xlogreader_state);
return EXIT_SUCCESS;
--
2.16.3
----Next_Part(Tue_Sep_10_17_40_54_2019_177)----
^ permalink raw reply [nested|flat] 17+ messages in thread
* [PATCH v12 4/4] Change policy of XLog read-buffer allocation
@ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw)
Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but
actually it'd be the responsibility to the callers of XLogReadRecord,
which now actually reads in pages. This patch does that.
---
src/backend/access/transam/twophase.c | 2 ++
src/backend/access/transam/xlog.c | 2 ++
src/backend/access/transam/xlogreader.c | 3 ---
src/backend/replication/logical/logical.c | 2 ++
src/bin/pg_rewind/parsexlog.c | 6 ++++++
src/bin/pg_waldump/pg_waldump.c | 2 ++
6 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index f519a5b6a2..e72d5c7ccb 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1336,6 +1336,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
@@ -1365,6 +1366,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
*buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader));
memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader));
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 364b1948e4..df1e9a5830 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6352,6 +6352,7 @@ StartupXLOG(void)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
xlogreader->system_identifier = ControlFile->system_identifier;
/*
@@ -7736,6 +7737,7 @@ StartupXLOG(void)
close(readFile);
readFile = -1;
}
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
/*
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 71984f00f7..81a95623a2 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -106,7 +106,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir)
MCXT_ALLOC_NO_OOM);
if (!state->errormsg_buf)
{
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -119,7 +118,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir)
if (!allocate_recordbuf(state, 0))
{
pfree(state->errormsg_buf);
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -143,7 +141,6 @@ XLogReaderFree(XLogReaderState *state)
pfree(state->errormsg_buf);
if (state->readRecordBuf)
pfree(state->readRecordBuf);
- pfree(state->readBuf);
pfree(state);
}
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index d3a92044b5..59671f99f0 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -174,6 +174,7 @@ StartupDecodingContext(List *output_plugin_options,
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));
+ ctx->reader->readBuf = palloc(XLOG_BLCKSZ);
ctx->read_page = read_page;
ctx->reorder = ReorderBufferAllocate();
@@ -519,6 +520,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx)
ReorderBufferFree(ctx->reorder);
FreeSnapshotBuilder(ctx->snapshot_builder);
+ pfree(ctx->reader->readBuf);
XLogReaderFree(ctx->reader);
MemoryContextDelete(ctx->context);
}
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index 4236ea1d12..ee380b25e1 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -58,6 +58,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
do
{
@@ -90,6 +91,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
} while (xlogreader->ReadRecPtr != endpoint);
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -113,6 +115,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) ==
XLREAD_NEED_DATA)
@@ -131,6 +134,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
}
endptr = xlogreader->EndRecPtr;
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -172,6 +176,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
searchptr = forkptr;
for (;;)
@@ -220,6 +225,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
searchptr = record->xl_prev;
}
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 16ee68efbe..4a6491007f 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1036,6 +1036,7 @@ main(int argc, char **argv)
if (!xlogreader_state)
fatal_error("out of memory");
+ xlogreader_state->readBuf = palloc(XLOG_BLCKSZ);
/* first find a valid recptr to start from */
first_record = XLogFindNextRecord(xlogreader_state, private.startptr,
@@ -1116,6 +1117,7 @@ main(int argc, char **argv)
(uint32) xlogreader_state->ReadRecPtr,
errormsg);
+ pfree(xlogreader_state->readBuf);
XLogReaderFree(xlogreader_state);
return EXIT_SUCCESS;
--
2.23.0
----Next_Part(Fri_Nov_29_17_14_21_2019_330)----
^ permalink raw reply [nested|flat] 17+ messages in thread
* [PATCH v8 4/4] Change policy of XLog read-buffer allocation
@ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw)
Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but
actually it'd be the responsibility to the callers of XLogReadRecord,
which now actually reads in pages. This patch does that.
---
src/backend/access/transam/twophase.c | 2 ++
src/backend/access/transam/xlog.c | 2 ++
src/backend/access/transam/xlogreader.c | 3 ---
src/backend/replication/logical/logical.c | 2 ++
src/bin/pg_rewind/parsexlog.c | 6 ++++++
src/bin/pg_waldump/pg_waldump.c | 2 ++
6 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index ced11bbdae..9e80a2cdb1 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1391,6 +1391,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
@@ -1420,6 +1421,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
*buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader));
memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader));
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 49e8ca486e..4cbb6de1bb 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6349,6 +6349,7 @@ StartupXLOG(void)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
xlogreader->system_identifier = ControlFile->system_identifier;
/*
@@ -7721,6 +7722,7 @@ StartupXLOG(void)
close(readFile);
readFile = -1;
}
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
/*
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 9cab82e76d..80434aed2e 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -105,7 +105,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir)
MCXT_ALLOC_NO_OOM);
if (!state->errormsg_buf)
{
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -118,7 +117,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir)
if (!allocate_recordbuf(state, 0))
{
pfree(state->errormsg_buf);
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -142,7 +140,6 @@ XLogReaderFree(XLogReaderState *state)
pfree(state->errormsg_buf);
if (state->readRecordBuf)
pfree(state->readRecordBuf);
- pfree(state->readBuf);
pfree(state);
}
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index db3e8f9dc0..99cb9bcc54 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -178,6 +178,7 @@ StartupDecodingContext(List *output_plugin_options,
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));
+ ctx->reader->readBuf = palloc(XLOG_BLCKSZ);
ctx->read_page = read_page;
ctx->reorder = ReorderBufferAllocate();
@@ -523,6 +524,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx)
ReorderBufferFree(ctx->reorder);
FreeSnapshotBuilder(ctx->snapshot_builder);
+ pfree(ctx->reader->readBuf);
XLogReaderFree(ctx->reader);
MemoryContextDelete(ctx->context);
}
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index c5e5d75a87..d0e408c0a8 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -60,6 +60,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
do
{
@@ -92,6 +93,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
} while (xlogreader->ReadRecPtr != endpoint);
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -115,6 +117,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) ==
XLREAD_NEED_DATA)
@@ -133,6 +136,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
}
endptr = xlogreader->EndRecPtr;
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -174,6 +178,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
searchptr = forkptr;
for (;;)
@@ -222,6 +227,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
searchptr = record->xl_prev;
}
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index ee49712830..e17e66e688 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1094,6 +1094,7 @@ main(int argc, char **argv)
xlogreader_state = XLogReaderAllocate(WalSegSz, waldir);
if (!xlogreader_state)
fatal_error("out of memory");
+ xlogreader_state->readBuf = palloc(XLOG_BLCKSZ);
/* first find a valid recptr to start from */
first_record = XLogFindNextRecord(xlogreader_state, private.startptr,
@@ -1174,6 +1175,7 @@ main(int argc, char **argv)
(uint32) xlogreader_state->ReadRecPtr,
errormsg);
+ pfree(xlogreader_state->readBuf);
XLogReaderFree(xlogreader_state);
return EXIT_SUCCESS;
--
2.16.3
----Next_Part(Fri_Sep_27_12_07_26_2019_308)----
^ permalink raw reply [nested|flat] 17+ messages in thread
* [PATCH v7 4/4] Change policy of XLog read-buffer allocation
@ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw)
Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but
actually it'd be the responsibility to the callers of XLogReadRecord,
which now actually reads in pages. This patch does that.
---
src/backend/access/transam/twophase.c | 2 ++
src/backend/access/transam/xlog.c | 2 ++
src/backend/access/transam/xlogreader.c | 3 ---
src/backend/replication/logical/logical.c | 2 ++
src/bin/pg_rewind/parsexlog.c | 6 ++++++
src/bin/pg_waldump/pg_waldump.c | 2 ++
6 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index ced11bbdae..9e80a2cdb1 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1391,6 +1391,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
@@ -1420,6 +1421,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
*buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader));
memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader));
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 49e8ca486e..4cbb6de1bb 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6349,6 +6349,7 @@ StartupXLOG(void)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
xlogreader->system_identifier = ControlFile->system_identifier;
/*
@@ -7721,6 +7722,7 @@ StartupXLOG(void)
close(readFile);
readFile = -1;
}
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
/*
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 9932ba3882..b8ad4ffcd7 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -105,7 +105,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir)
MCXT_ALLOC_NO_OOM);
if (!state->errormsg_buf)
{
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -118,7 +117,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir)
if (!allocate_recordbuf(state, 0))
{
pfree(state->errormsg_buf);
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -142,7 +140,6 @@ XLogReaderFree(XLogReaderState *state)
pfree(state->errormsg_buf);
if (state->readRecordBuf)
pfree(state->readRecordBuf);
- pfree(state->readBuf);
pfree(state);
}
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index db3e8f9dc0..99cb9bcc54 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -178,6 +178,7 @@ StartupDecodingContext(List *output_plugin_options,
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));
+ ctx->reader->readBuf = palloc(XLOG_BLCKSZ);
ctx->read_page = read_page;
ctx->reorder = ReorderBufferAllocate();
@@ -523,6 +524,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx)
ReorderBufferFree(ctx->reorder);
FreeSnapshotBuilder(ctx->snapshot_builder);
+ pfree(ctx->reader->readBuf);
XLogReaderFree(ctx->reader);
MemoryContextDelete(ctx->context);
}
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index c5e5d75a87..d0e408c0a8 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -60,6 +60,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
do
{
@@ -92,6 +93,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
} while (xlogreader->ReadRecPtr != endpoint);
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -115,6 +117,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) ==
XLREAD_NEED_DATA)
@@ -133,6 +136,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
}
endptr = xlogreader->EndRecPtr;
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -174,6 +178,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
searchptr = forkptr;
for (;;)
@@ -222,6 +227,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
searchptr = record->xl_prev;
}
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index ee49712830..e17e66e688 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1094,6 +1094,7 @@ main(int argc, char **argv)
xlogreader_state = XLogReaderAllocate(WalSegSz, waldir);
if (!xlogreader_state)
fatal_error("out of memory");
+ xlogreader_state->readBuf = palloc(XLOG_BLCKSZ);
/* first find a valid recptr to start from */
first_record = XLogFindNextRecord(xlogreader_state, private.startptr,
@@ -1174,6 +1175,7 @@ main(int argc, char **argv)
(uint32) xlogreader_state->ReadRecPtr,
errormsg);
+ pfree(xlogreader_state->readBuf);
XLogReaderFree(xlogreader_state);
return EXIT_SUCCESS;
--
2.16.3
----Next_Part(Wed_Sep_25_15_50_32_2019_576)----
^ permalink raw reply [nested|flat] 17+ messages in thread
* [PATCH v5 3/3] Change policy of XLog read-buffer allocation
@ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw)
Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but
actually it'd be the responsibility to the callers of XLogReadRecord,
which now actually reads in pages. This patch does that.
---
src/backend/access/transam/twophase.c | 2 ++
src/backend/access/transam/xlog.c | 2 ++
src/backend/access/transam/xlogreader.c | 18 ------------------
src/backend/replication/logical/logical.c | 2 ++
src/bin/pg_rewind/parsexlog.c | 6 ++++++
src/bin/pg_waldump/pg_waldump.c | 2 ++
6 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 929da4eef2..9ec88b35ef 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1392,6 +1392,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
while (XLogReadRecord(xlogreader, lsn, &record, &errormsg) ==
XLREAD_NEED_DATA)
@@ -1421,6 +1422,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
*buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader));
memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader));
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 96e2115aad..425f7a12bd 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6351,6 +6351,7 @@ StartupXLOG(void)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
xlogreader->system_identifier = ControlFile->system_identifier;
/*
@@ -7722,6 +7723,7 @@ StartupXLOG(void)
close(readFile);
readFile = -1;
}
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
/*
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 66bd9eb8d7..26f6834b8f 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -79,27 +79,11 @@ XLogReaderAllocate(int wal_segment_size)
state->max_block_id = -1;
- /*
- * Permanently allocate readBuf. We do it this way, rather than just
- * making a static array, for two reasons: (1) no need to waste the
- * storage in most instantiations of the backend; (2) a static char array
- * isn't guaranteed to have any particular alignment, whereas
- * palloc_extended() will provide MAXALIGN'd storage.
- */
- state->readBuf = (char *) palloc_extended(XLOG_BLCKSZ,
- MCXT_ALLOC_NO_OOM);
- if (!state->readBuf)
- {
- pfree(state);
- return NULL;
- }
-
state->wal_segment_size = wal_segment_size;
state->errormsg_buf = palloc_extended(MAX_ERRORMSG_LEN + 1,
MCXT_ALLOC_NO_OOM);
if (!state->errormsg_buf)
{
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -112,7 +96,6 @@ XLogReaderAllocate(int wal_segment_size)
if (!allocate_recordbuf(state, 0))
{
pfree(state->errormsg_buf);
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -136,7 +119,6 @@ XLogReaderFree(XLogReaderState *state)
pfree(state->errormsg_buf);
if (state->readRecordBuf)
pfree(state->readRecordBuf);
- pfree(state->readBuf);
pfree(state);
}
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index 11e52e4c01..ea027caa69 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -178,6 +178,7 @@ StartupDecodingContext(List *output_plugin_options,
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));
+ ctx->reader->readBuf = palloc(XLOG_BLCKSZ);
ctx->read_page = read_page;
ctx->reorder = ReorderBufferAllocate();
@@ -523,6 +524,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx)
ReorderBufferFree(ctx->reorder);
FreeSnapshotBuilder(ctx->snapshot_builder);
+ pfree(ctx->reader->readBuf);
XLogReaderFree(ctx->reader);
MemoryContextDelete(ctx->context);
}
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index ff26b30f82..c60267e87e 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -60,6 +60,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
do
{
@@ -92,6 +93,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
} while (xlogreader->ReadRecPtr != endpoint);
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -115,6 +117,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
xlogreader = XLogReaderAllocate(WalSegSz);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
while (XLogReadRecord(xlogreader, ptr, &record, &errormsg) ==
XLREAD_NEED_DATA)
@@ -133,6 +136,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
}
endptr = xlogreader->EndRecPtr;
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -174,6 +178,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
searchptr = forkptr;
for (;;)
@@ -222,6 +227,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
searchptr = record->xl_prev;
}
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 56e2f8b0b0..bdf3c81b03 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1108,6 +1108,7 @@ main(int argc, char **argv)
xlogreader_state = XLogReaderAllocate(WalSegSz);
if (!xlogreader_state)
fatal_error("out of memory");
+ xlogreader_state->readBuf = palloc(XLOG_BLCKSZ);
/* first find a valid recptr to start from */
first_record = XLogFindNextRecord(xlogreader_state, private.startptr,
@@ -1190,6 +1191,7 @@ main(int argc, char **argv)
(uint32) xlogreader_state->ReadRecPtr,
errormsg);
+ pfree(xlogreader_state->readBuf);
XLogReaderFree(xlogreader_state);
return EXIT_SUCCESS;
--
2.16.3
----Next_Part(Fri_Sep_06_16_33_18_2019_198)----
^ permalink raw reply [nested|flat] 17+ messages in thread
* [PATCH v7 4/4] Change policy of XLog read-buffer allocation
@ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Kyotaro Horiguchi @ 2019-09-05 12:29 UTC (permalink / raw)
Page buffer in XLogReaderState was allocated by XLogReaderAllcoate but
actually it'd be the responsibility to the callers of XLogReadRecord,
which now actually reads in pages. This patch does that.
---
src/backend/access/transam/twophase.c | 2 ++
src/backend/access/transam/xlog.c | 2 ++
src/backend/access/transam/xlogreader.c | 3 ---
src/backend/replication/logical/logical.c | 2 ++
src/bin/pg_rewind/parsexlog.c | 6 ++++++
src/bin/pg_waldump/pg_waldump.c | 2 ++
6 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index f5f6278880..6f5e6e5e56 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1336,6 +1336,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
XLogBeginRead(xlogreader, lsn);
while (XLogReadRecord(xlogreader, &record, &errormsg) ==
@@ -1366,6 +1367,7 @@ XlogReadTwoPhaseData(XLogRecPtr lsn, char **buf, int *len)
*buf = palloc(sizeof(char) * XLogRecGetDataLen(xlogreader));
memcpy(*buf, XLogRecGetData(xlogreader), sizeof(char) * XLogRecGetDataLen(xlogreader));
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index a3eded30ad..3f7f3e4d4f 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6411,6 +6411,7 @@ StartupXLOG(void)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory"),
errdetail("Failed while allocating a WAL reading processor.")));
+ xlogreader->readBuf = palloc(XLOG_BLCKSZ);
xlogreader->system_identifier = ControlFile->system_identifier;
/*
@@ -7813,6 +7814,7 @@ StartupXLOG(void)
close(readFile);
readFile = -1;
}
+ pfree(xlogreader->readBuf);
XLogReaderFree(xlogreader);
/*
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index ad85f50c77..6b9287d68a 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -106,7 +106,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir)
MCXT_ALLOC_NO_OOM);
if (!state->errormsg_buf)
{
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -119,7 +118,6 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir)
if (!allocate_recordbuf(state, 0))
{
pfree(state->errormsg_buf);
- pfree(state->readBuf);
pfree(state);
return NULL;
}
@@ -143,7 +141,6 @@ XLogReaderFree(XLogReaderState *state)
pfree(state->errormsg_buf);
if (state->readRecordBuf)
pfree(state->readRecordBuf);
- pfree(state->readBuf);
pfree(state);
}
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index 7782e3ad2f..858c537558 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -174,6 +174,7 @@ StartupDecodingContext(List *output_plugin_options,
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));
+ ctx->reader->readBuf = palloc(XLOG_BLCKSZ);
ctx->read_page = read_page;
ctx->reorder = ReorderBufferAllocate();
@@ -518,6 +519,7 @@ FreeDecodingContext(LogicalDecodingContext *ctx)
ReorderBufferFree(ctx->reorder);
FreeSnapshotBuilder(ctx->snapshot_builder);
+ pfree(ctx->reader->readBuf);
XLogReaderFree(ctx->reader);
MemoryContextDelete(ctx->context);
}
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index 55337b65f1..48502151a1 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -58,6 +58,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
XLogBeginRead(xlogreader, startpoint);
do
@@ -86,6 +87,7 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, int tliIndex,
} while (xlogreader->ReadRecPtr != endpoint);
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -109,6 +111,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
XLogBeginRead(xlogreader, ptr);
while (XLogReadRecord(xlogreader, &record, &errormsg) ==
@@ -128,6 +131,7 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex)
}
endptr = xlogreader->EndRecPtr;
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
@@ -169,6 +173,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
xlogreader = XLogReaderAllocate(WalSegSz, datadir);
if (xlogreader == NULL)
pg_fatal("out of memory");
+ xlogreader->readBuf = pg_malloc(XLOG_BLCKSZ);
searchptr = forkptr;
for (;;)
@@ -218,6 +223,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
searchptr = record->xl_prev;
}
+ pg_free(xlogreader->readBuf);
XLogReaderFree(xlogreader);
if (xlogreadfd != -1)
{
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 0cd0d132af..ac6740f21d 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1033,6 +1033,7 @@ main(int argc, char **argv)
if (!xlogreader_state)
fatal_error("out of memory");
+ xlogreader_state->readBuf = palloc(XLOG_BLCKSZ);
/* first find a valid recptr to start from */
first_record = XLogFindNextRecord(xlogreader_state, private.startptr,
@@ -1109,6 +1110,7 @@ main(int argc, char **argv)
(uint32) xlogreader_state->ReadRecPtr,
errormsg);
+ pfree(xlogreader_state->readBuf);
XLogReaderFree(xlogreader_state);
return EXIT_SUCCESS;
--
2.18.2
----Next_Part(Tue_Mar_24_18_24_13_2020_275)----
^ permalink raw reply [nested|flat] 17+ messages in thread
* Fix array access (src/bin/pg_dump/pg_dump.c)
@ 2024-11-12 17:05 Ranier Vilela <[email protected]>
2024-11-12 19:11 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Ranier Vilela @ 2024-11-12 17:05 UTC (permalink / raw)
To: pgsql-hackers
Hi.
Per Coverity.
The function *determineNotNullFlags* has a little oversight.
The struct field *notnull_islocal* is an array.
I think this is a simple typo.
Fix using array notation access.
Trivial patch attached.
best regards,
Ranier Vilela
Attachments:
[application/octet-stream] fix_array_access_pg_dump.patch (473B, ../../CAEudQAo7ah=4TDheuEjtb0dsv6bHoK7uBNqv53Tsub2h-xBSJw@mail.gmail.com/3-fix_array_access_pg_dump.patch)
download | inline diff:
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a8c141b689..404f5d8675 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -9397,7 +9397,7 @@ determineNotNullFlags(Archive *fout, PGresult *res, int r,
*/
if (dopt->binary_upgrade &&
!tbinfo->ispartition &&
- !tbinfo->notnull_islocal)
+ !tbinfo->notnull_islocal[j])
{
tbinfo->notnull_constrs[j] =
pstrdup(PQgetvalue(res, r, i_notnull_name));
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Fix array access (src/bin/pg_dump/pg_dump.c)
2024-11-12 17:05 Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
@ 2024-11-12 19:11 ` Alvaro Herrera <[email protected]>
2024-11-12 22:13 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Alvaro Herrera @ 2024-11-12 19:11 UTC (permalink / raw)
To: Ranier Vilela <[email protected]>; +Cc: pgsql-hackers
On 2024-Nov-12, Ranier Vilela wrote:
> Per Coverity.
>
> The function *determineNotNullFlags* has a little oversight.
> The struct field *notnull_islocal* is an array.
>
> I think this is a simple typo.
> Fix using array notation access.
Yeah, thanks, I had been made aware of this bug. Before fixing I'd like
to construct a test case that tickles that code, because it's currently
uncovered *shudder*
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Find a bug in a program, and fix it, and the program will work today.
Show the program how to find and fix a bug, and the program
will work forever" (Oliver Silfridge)
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Fix array access (src/bin/pg_dump/pg_dump.c)
2024-11-12 17:05 Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
2024-11-12 19:11 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Alvaro Herrera <[email protected]>
@ 2024-11-12 22:13 ` Ranier Vilela <[email protected]>
2025-10-10 16:41 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Ranier Vilela @ 2024-11-12 22:13 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: pgsql-hackers
Em ter., 12 de nov. de 2024 às 16:11, Alvaro Herrera <
[email protected]> escreveu:
> On 2024-Nov-12, Ranier Vilela wrote:
>
> > Per Coverity.
> >
> > The function *determineNotNullFlags* has a little oversight.
> > The struct field *notnull_islocal* is an array.
> >
> > I think this is a simple typo.
> > Fix using array notation access.
>
> Yeah, thanks, I had been made aware of this bug. Before fixing I'd like
> to construct a test case that tickles that code, because it's currently
> uncovered *shudder*
>
Thanks for taking care of this.
best regards,
Ranier Vilela
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Fix array access (src/bin/pg_dump/pg_dump.c)
2024-11-12 17:05 Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
2024-11-12 19:11 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Alvaro Herrera <[email protected]>
2024-11-12 22:13 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
@ 2025-10-10 16:41 ` Ranier Vilela <[email protected]>
2025-10-11 05:24 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Chao Li <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Ranier Vilela @ 2025-10-10 16:41 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: pgsql-hackers
Hi.
Em ter., 12 de nov. de 2024 às 19:13, Ranier Vilela <[email protected]>
escreveu:
> Em ter., 12 de nov. de 2024 às 16:11, Alvaro Herrera <
> [email protected]> escreveu:
>
>> On 2024-Nov-12, Ranier Vilela wrote:
>>
>> > Per Coverity.
>> >
>> > The function *determineNotNullFlags* has a little oversight.
>> > The struct field *notnull_islocal* is an array.
>> >
>> > I think this is a simple typo.
>> > Fix using array notation access.
>>
>> Yeah, thanks, I had been made aware of this bug. Before fixing I'd like
>> to construct a test case that tickles that code, because it's currently
>> uncovered *shudder*
>>
> Thanks for taking care of this.
>
Ping.
best regards,
Ranier Vilela
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Fix array access (src/bin/pg_dump/pg_dump.c)
2024-11-12 17:05 Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
2024-11-12 19:11 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Alvaro Herrera <[email protected]>
2024-11-12 22:13 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
2025-10-10 16:41 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
@ 2025-10-11 05:24 ` Chao Li <[email protected]>
2025-10-11 10:10 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Chao Li @ 2025-10-11 05:24 UTC (permalink / raw)
To: Ranier Vilela <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; pgsql-hackers
On Oct 11, 2025, at 00:41, Ranier Vilela <[email protected]> wrote:
Em ter., 12 de nov. de 2024 às 19:13, Ranier Vilela <[email protected]>
escreveu:
> Em ter., 12 de nov. de 2024 às 16:11, Alvaro Herrera <
> [email protected]> escreveu:
>
>> On 2024-Nov-12, Ranier Vilela wrote:
>>
>> > Per Coverity.
>> >
>> > The function *determineNotNullFlags* has a little oversight.
>> > The struct field *notnull_islocal* is an array.
>> >
>> > I think this is a simple typo.
>> > Fix using array notation access.
>>
>> Yeah, thanks, I had been made aware of this bug. Before fixing I'd like
>> to construct a test case that tickles that code, because it's currently
>> uncovered *shudder*
>>
> Thanks for taking care of this.
>
Ping.
I tried to debug this bug, and it looks like this bug can never be
triggered.
To do the test, I created two tables:
```
evantest=# CREATE TABLE parent_nn(a int NOT NULL, b int);
CREATE TABLE
evantest=# CREATE TABLE child_nn() INHERITS (parent_nn);
CREATE TABLE
evantest=# ALTER TABLE child_nn ALTER COLUMN b SET NOT NULL;
ALTER TABLE
```
Then let’s look at the code:
/*
* In binary upgrade of inheritance child tables, must have a
* constraint name that we can UPDATE later; same if there's a
* comment on the constraint.
*/
if ((dopt->binary_upgrade &&
!tbinfo->ispartition &&
!tbinfo->notnull_islocal) ||
!PQgetisnull(res, r, i_notnull_comment)) // A
{
const char *val = PQgetvalue(res, r, i_notnull_name); // B
tbinfo->notnull_constrs[j] =
pstrdup(val);
}
else
{
char *default_name;
const char *val = PQgetvalue(res, r, i_notnull_name);
/* XXX should match ChooseConstraintName better */
default_name = psprintf("%s_%s_not_null", tbinfo->dobj.name,
tbinfo->attnames[j]);
if (strcmp(default_name, // C
val) == 0)
tbinfo->notnull_constrs[j] = "";
else
{
tbinfo->notnull_constrs[j] = // D
pstrdup(val);
}
free(default_name);
}
Notice that I marked four lines with A/B/C/D.
For child table’s column “b”, when it reaches line A, it hits the bug,
as tbinfo->notnull_islocal[j] should be false, but tbinfo->notnull_islocal
is always true.
If the bug is fixed, as tbinfo->notnull_islocal[j] is false, it will enter
the “if” clause, in line B, PGgetvalue() will return “parent_nn_a_not_null”.
With this bug, if will go the the “else” clause, run strcmp() in line C.
Here, “default_name” is built from the table name, its value is
“child_nn_a_not_null”, while PGgetvalue() is “parent_nn_a_not_null”, thus
it won’t meet the “if” of “strcmp”, instead it goes to line D, that runs
the same assignment as in line B.
So, Ranier, please let me know if you have an example that generates the
wrong result, then I can verify the fix with your test case.
But I believe we should still fix the bug: 1) otherwise the code is
confusing 2) after fixing, when tbinfo->notnull_islocal[j] is false, the
execution path is shorter 3) to make Coverity happy.
I also added a TAP test with test cases for determining NULL:
```
chaol@ChaodeMacBook-Air pg_dump % make check PROVE_TESTS='t/
011_dump_determine_null.pl'
# +++ tap check in src/bin/pg_dump +++
t/011_dump_determine_null.pl .. ok
All tests successful.
Files=1, Tests=5, 2 wallclock secs ( 0.00 usr 0.01 sys + 0.08 cusr 0.31
csys = 0.40 CPU)
Result: PASS
```
Please see the attached patch.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v1-0001-Fixed-a-bug-in-pg_dump-about-determining-NotNull.patch (3.9K, ../../CAEoWx2nJXLDb_gZuGcAE4rCfwUX=ndtkcBx55ynNSyFPOgXpEQ@mail.gmail.com/3-v1-0001-Fixed-a-bug-in-pg_dump-about-determining-NotNull.patch)
download | inline diff:
From 92608d497d779000077d13dae6c2064549c3785f Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Sat, 11 Oct 2025 13:13:32 +0800
Subject: [PATCH v1] Fixed a bug in pg_dump about determining NotNull
In determineNotNullFlags(), a check should be done against
tbinfo->notnull_islocal[j], but "[j]" was missing. However,
this bug cannot be actually fired, see the discussion.
This commit fixes the bug and adds a TAP test of determining
NotNull for pg_dump.
Author: Chao Li <[email protected]>
Discussion: https://www.postgresql.org/message-id/CAEudQAo7ah%3D4TDheuEjtb0dsv6bHoK7uBNqv53Tsub2h-xBSJw%40mail.gmail.com
---
src/bin/pg_dump/meson.build | 1 +
src/bin/pg_dump/pg_dump.c | 2 +-
src/bin/pg_dump/t/011_dump_determine_null.pl | 59 ++++++++++++++++++++
3 files changed, 61 insertions(+), 1 deletion(-)
create mode 100644 src/bin/pg_dump/t/011_dump_determine_null.pl
diff --git a/src/bin/pg_dump/meson.build b/src/bin/pg_dump/meson.build
index a2233b0a1b4..47225b818da 100644
--- a/src/bin/pg_dump/meson.build
+++ b/src/bin/pg_dump/meson.build
@@ -103,6 +103,7 @@ tests += {
't/004_pg_dump_parallel.pl',
't/005_pg_dump_filterfile.pl',
't/010_dump_connstr.pl',
+ 't/011_dump_determine_null.pl',
],
},
}
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 641bece12c7..9dc9396a34d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -10074,7 +10074,7 @@ determineNotNullFlags(Archive *fout, PGresult *res, int r,
*/
if ((dopt->binary_upgrade &&
!tbinfo->ispartition &&
- !tbinfo->notnull_islocal) ||
+ !tbinfo->notnull_islocal[j]) ||
!PQgetisnull(res, r, i_notnull_comment))
{
tbinfo->notnull_constrs[j] =
diff --git a/src/bin/pg_dump/t/011_dump_determine_null.pl b/src/bin/pg_dump/t/011_dump_determine_null.pl
new file mode 100644
index 00000000000..11d93135dd8
--- /dev/null
+++ b/src/bin/pg_dump/t/011_dump_determine_null.pl
@@ -0,0 +1,59 @@
+# Copyright (c) 2021-2025, PostgreSQL Global Development Group
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $tempdir = PostgreSQL::Test::Utils::tempdir;
+my $node = PostgreSQL::Test::Cluster->new('main');
+my $port = $node->port;
+my $backupdir = $node->backup_dir;
+my $plainfile = "$backupdir/plain.sql";
+
+# Init & start the node (no extra args; follow conventions used by other tests)
+$node->init;
+$node->start;
+
+# Setup parent/child table for binary upgrade NOT NULL check
+$node->safe_psql('postgres', "CREATE TABLE parent_nn(a int NOT NULL, b int)");
+$node->safe_psql('postgres', "CREATE TABLE child_nn() INHERITS (parent_nn)");
+
+# Add a local NOT NULL to child column 'b'
+$node->safe_psql('postgres', "ALTER TABLE child_nn ALTER COLUMN b SET NOT NULL");
+
+# Do a schema-only pg_dump through the tap helper
+command_ok(
+ [
+ 'pg_dump',
+ '--file' => $plainfile,
+ '--schema-only',
+ '--binary-upgrade',
+ '--port' => $port,
+ 'postgres'
+ ],
+ 'pg_dump schema-only succeeds'
+);
+
+# Read dump and assert expected NOT NULL markings
+my $dump = slurp_file($plainfile);
+
+# Check parent table that NOT NULL is there
+ok($dump =~ qr/CREATE TABLE public\.parent_nn\s*\(\s*a integer NOT NULL/m,
+ "parent NOT NULL column 'a' is there");
+
+# Check parent table that NULL-able is there
+ok($dump =~ qr/CREATE TABLE public\.parent_nn\s*\(\s*a.*,\s*b integer/m,
+ "parent NULL-able column 'b' is there");
+
+# Check child table that inherited NOT NULL is preserved
+ok($dump =~ qr/CREATE TABLE public\.child_nn\s*\(\s*a integer CONSTRAINT parent_nn_a_not_null NOT NULL/m,
+ "inherited NOT NULL column 'a' preserved");
+
+# Check child table that locally added NOT NULL is preserved
+ok($dump =~ qr/CREATE TABLE public\.child_nn\s*\(\s*a.*,\s*b integer NOT NULL/m,
+ "child local NOT NULL column 'b' preserved");
+
+done_testing();
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Fix array access (src/bin/pg_dump/pg_dump.c)
2024-11-12 17:05 Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
2024-11-12 19:11 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Alvaro Herrera <[email protected]>
2024-11-12 22:13 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
2025-10-10 16:41 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
2025-10-11 05:24 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Chao Li <[email protected]>
@ 2025-10-11 10:10 ` Ranier Vilela <[email protected]>
2025-10-11 11:59 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Chao Li <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Ranier Vilela @ 2025-10-11 10:10 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; pgsql-hackers
Em sáb., 11 de out. de 2025 às 02:24, Chao Li <[email protected]>
escreveu:
>
> On Oct 11, 2025, at 00:41, Ranier Vilela <[email protected]> wrote:
>
> Em ter., 12 de nov. de 2024 às 19:13, Ranier Vilela <[email protected]>
> escreveu:
>
>> Em ter., 12 de nov. de 2024 às 16:11, Alvaro Herrera <
>> [email protected]> escreveu:
>>
>>> On 2024-Nov-12, Ranier Vilela wrote:
>>>
>>> > Per Coverity.
>>> >
>>> > The function *determineNotNullFlags* has a little oversight.
>>> > The struct field *notnull_islocal* is an array.
>>> >
>>> > I think this is a simple typo.
>>> > Fix using array notation access.
>>>
>>> Yeah, thanks, I had been made aware of this bug. Before fixing I'd like
>>> to construct a test case that tickles that code, because it's currently
>>> uncovered *shudder*
>>>
>> Thanks for taking care of this.
>>
> Ping.
>
>
> I tried to debug this bug, and it looks like this bug can never be
> triggered.
>
> To do the test, I created two tables:
> ```
> evantest=# CREATE TABLE parent_nn(a int NOT NULL, b int);
> CREATE TABLE
> evantest=# CREATE TABLE child_nn() INHERITS (parent_nn);
> CREATE TABLE
> evantest=# ALTER TABLE child_nn ALTER COLUMN b SET NOT NULL;
> ALTER TABLE
> ```
>
> Then let’s look at the code:
>
> /*
> * In binary upgrade of inheritance child tables, must have a
> * constraint name that we can UPDATE later; same if there's a
> * comment on the constraint.
> */
> if ((dopt->binary_upgrade &&
> !tbinfo->ispartition &&
> !tbinfo->notnull_islocal) ||
> !PQgetisnull(res, r, i_notnull_comment)) // A
> {
> const char *val = PQgetvalue(res, r, i_notnull_name); // B
> tbinfo->notnull_constrs[j] =
> pstrdup(val);
> }
> else
> {
> char *default_name;
> const char *val = PQgetvalue(res, r, i_notnull_name);
>
> /* XXX should match ChooseConstraintName better */
> default_name = psprintf("%s_%s_not_null", tbinfo->dobj.name,
> tbinfo->attnames[j]);
> if (strcmp(default_name, // C
> val) == 0)
> tbinfo->notnull_constrs[j] = "";
> else
> {
> tbinfo->notnull_constrs[j] = // D
> pstrdup(val);
> }
> free(default_name);
> }
>
> Notice that I marked four lines with A/B/C/D.
>
> For child table’s column “b”, when it reaches line A, it hits the bug,
> as tbinfo->notnull_islocal[j] should be false, but tbinfo->notnull_islocal
> is always true.
>
> If the bug is fixed, as tbinfo->notnull_islocal[j] is false, it will enter
> the “if” clause, in line B, PGgetvalue() will return “parent_nn_a_not_null”.
>
> With this bug, if will go the the “else” clause, run strcmp() in line C.
> Here, “default_name” is built from the table name, its value is
> “child_nn_a_not_null”, while PGgetvalue() is “parent_nn_a_not_null”, thus
> it won’t meet the “if” of “strcmp”, instead it goes to line D, that runs
> the same assignment as in line B.
>
> So, Ranier, please let me know if you have an example that generates the
> wrong result, then I can verify the fix with your test case.
>
> But I believe we should still fix the bug: 1) otherwise the code is
> confusing 2) after fixing, when tbinfo->notnull_islocal[j] is false, the
> execution path is shorter 3) to make Coverity happy.
>
> I also added a TAP test with test cases for determining NULL:
>
> ```
> chaol@ChaodeMacBook-Air pg_dump % make check PROVE_TESTS='t/
> 011_dump_determine_null.pl'
> # +++ tap check in src/bin/pg_dump +++
> t/011_dump_determine_null.pl .. ok
> All tests successful.
> Files=1, Tests=5, 2 wallclock secs ( 0.00 usr 0.01 sys + 0.08 cusr
> 0.31 csys = 0.40 CPU)
> Result: PASS
> ```
>
> Please see the attached patch.
>
Did you read the entire thread?
I think it's very rude and inelegant to suggest a patch while taking
advantage of another patch.
Best regards,
Ranier Vilela
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Fix array access (src/bin/pg_dump/pg_dump.c)
2024-11-12 17:05 Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
2024-11-12 19:11 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Alvaro Herrera <[email protected]>
2024-11-12 22:13 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
2025-10-10 16:41 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
2025-10-11 05:24 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Chao Li <[email protected]>
2025-10-11 10:10 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
@ 2025-10-11 11:59 ` Chao Li <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Chao Li @ 2025-10-11 11:59 UTC (permalink / raw)
To: Ranier Vilela <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; pgsql-hackers
> On Oct 11, 2025, at 18:10, Ranier Vilela <[email protected]> wrote:
>
>
> Did you read the entire thread?
> I think it's very rude and inelegant to suggest a patch while taking advantage of another patch.
>
> Best regards,
> Ranier Vilela
No, I guess I didn’t read the entire thread. I just saw ping and thought to help. If you don’t like, just ignore my previous response.
^ permalink raw reply [nested|flat] 17+ messages in thread
* [PATCH] Reproduce filtering issue.
@ 2026-03-23 11:50 Antonin Houska <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Antonin Houska @ 2026-03-23 11:50 UTC (permalink / raw)
---
contrib/test_decoding/expected/filtering.out | 74 ++++++++++++++
contrib/test_decoding/specs/filtering.spec | 101 +++++++++++++++++++
src/backend/executor/nodeModifyTable.c | 2 +
src/backend/replication/logical/snapbuild.c | 3 +
src/test/isolation/isolationtester.c | 9 +-
5 files changed, 188 insertions(+), 1 deletion(-)
create mode 100644 contrib/test_decoding/expected/filtering.out
create mode 100644 contrib/test_decoding/specs/filtering.spec
diff --git a/contrib/test_decoding/expected/filtering.out b/contrib/test_decoding/expected/filtering.out
new file mode 100644
index 00000000000..6ba9690509f
--- /dev/null
+++ b/contrib/test_decoding/expected/filtering.out
@@ -0,0 +1,74 @@
+Parsed test spec with 5 sessions
+
+starting permutation: s1_assign_xid s3_repack s2_assign_xid s1_rollback s4_insert s5_wakeup_snapbuild s2_rollback s5_wakeup_insert_speculative s4_insert_commit s5_wakeup_repack
+injection_points_attach
+-----------------------
+
+(1 row)
+
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step s1_assign_xid:
+ BEGIN;
+ CREATE TABLE c(i int);
+
+step s3_repack:
+ REPACK (CONCURRENTLY) a;
+ <waiting ...>
+step s2_assign_xid:
+ BEGIN;
+ CREATE TABLE d(i int);
+
+step s1_rollback:
+ ROLLBACK;
+
+step s4_insert:
+ BEGIN;
+ INSERT INTO t(i)
+ SELECT max(i) + 1 FROM t ON CONFLICT (i) DO UPDATE SET i=EXCLUDED.i;
+ <waiting ...>
+step s5_wakeup_snapbuild:
+ SELECT injection_points_wakeup('snapbuild-full');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step s2_rollback:
+ ROLLBACK;
+
+step s5_wakeup_insert_speculative:
+ SELECT injection_points_wakeup('insert-speculative-before-confirm');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step s4_insert: <... completed>
+step s4_insert_commit:
+ COMMIT;
+
+step s5_wakeup_repack:
+ SELECT injection_points_wakeup('repack-concurrently-before-lock');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step s3_repack: <... completed>
+injection_points_detach
+-----------------------
+
+(1 row)
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
diff --git a/contrib/test_decoding/specs/filtering.spec b/contrib/test_decoding/specs/filtering.spec
new file mode 100644
index 00000000000..a02c9f8facb
--- /dev/null
+++ b/contrib/test_decoding/specs/filtering.spec
@@ -0,0 +1,101 @@
+setup
+{
+ CREATE TABLE a(i int primary key, j int) WITH (autovacuum_enabled = off);
+ INSERT INTO a(i, j) VALUES (1, 1), (2, 2);
+ CREATE TABLE t(i int primary key);
+ INSERT INTO t(i) VALUES (1);
+ CREATE EXTENSION injection_points;
+}
+
+session s1
+step s1_assign_xid
+{
+ BEGIN;
+ CREATE TABLE c(i int);
+}
+step s1_rollback
+{
+ ROLLBACK;
+}
+
+session s2
+step s2_assign_xid
+{
+ BEGIN;
+ CREATE TABLE d(i int);
+}
+step s2_rollback
+{
+ ROLLBACK;
+}
+
+session s3
+setup
+{
+ SELECT injection_points_attach('snapbuild-full', 'wait');
+ SELECT injection_points_attach('repack-concurrently-before-lock', 'wait');
+}
+step s3_repack
+{
+ REPACK (CONCURRENTLY) a;
+}
+teardown
+{
+ SELECT injection_points_detach('repack-concurrently-before-lock');
+ SELECT injection_points_detach('snapbuild-full');
+}
+
+session s4
+setup
+{
+ SELECT injection_points_set_local();
+ SELECT injection_points_attach('insert-speculative-before-confirm', 'wait');
+}
+step s4_insert
+{
+ BEGIN;
+ INSERT INTO t(i)
+ SELECT max(i) + 1 FROM t ON CONFLICT (i) DO UPDATE SET i=EXCLUDED.i;
+}
+step s4_insert_commit
+{
+ COMMIT;
+}
+teardown
+{
+ SELECT injection_points_detach('insert-speculative-before-confirm');
+}
+
+session s5
+step s5_wakeup_snapbuild
+{
+ SELECT injection_points_wakeup('snapbuild-full');
+}
+step s5_wakeup_insert_speculative
+{
+ SELECT injection_points_wakeup('insert-speculative-before-confirm');
+}
+step s5_wakeup_repack
+{
+ SELECT injection_points_wakeup('repack-concurrently-before-lock');
+}
+
+permutation
+# Bring the snapshot builder to the FULL_SNAPSHOT state.
+s1_assign_xid
+s3_repack
+s2_assign_xid
+s1_rollback
+# Perform the speculative insert, but no confirmation so far. The snapshot
+# builder should decode it.
+s4_insert
+# Let the snapshout builder achieve CONSISTENT state and finish the setup.
+s5_wakeup_snapbuild
+s2_rollback
+# While REPACK is waiting on repack-concurrently-before-lock, let the insert
+# get confirmed. Relation filtering is now enabled.
+s5_wakeup_insert_speculative
+s4_insert_commit
+# REPACK should now decode the speculative insert and decode the speculative
+# insert (with the confirmation record filtered out).
+s5_wakeup_repack
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 680c29f35d5..6d5482e5746 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1232,6 +1232,8 @@ ExecInsert(ModifyTableContext *context,
slot, arbiterIndexes,
&specConflict);
+ INJECTION_POINT("insert-speculative-before-confirm", NULL);
+
/* adjust the tuple's state accordingly */
table_tuple_complete_speculative(resultRelationDesc, slot,
specToken, !specConflict);
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index fbdd4600a2b..883e5b6e18f 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -141,6 +141,7 @@
#include "storage/procarray.h"
#include "storage/standby.h"
#include "utils/builtins.h"
+#include "utils/injection_point.h"
#include "utils/memutils.h"
#include "utils/snapmgr.h"
#include "utils/snapshot.h"
@@ -1390,6 +1391,8 @@ SnapBuildFindSnapshot(SnapBuild *builder, XLogRecPtr lsn, xl_running_xacts *runn
builder->state = SNAPBUILD_FULL_SNAPSHOT;
builder->next_phase_at = running->nextXid;
+ INJECTION_POINT("snapbuild-full", NULL);
+
ereport(LOG,
errmsg("logical decoding found initial consistent point at %X/%08X",
LSN_FORMAT_ARGS(lsn)),
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index 440c875b8ac..8f17ee412c9 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -216,15 +216,22 @@ main(int argc, char **argv)
* exactly expect concurrent use of test tables. However, autovacuum will
* occasionally take AccessExclusiveLock to truncate a table, and we must
* ignore that transient wait.
+ *
+ * If the session's backend is blocked, and if its background worker is
+ * waiting on an injection point, we assume that the injection point is
+ * the reason for the backend to be blocked. That's what we check in the
+ * second query of the UNION. XXX Should we use a separate query for that?
*/
initPQExpBuffer(&wait_query);
appendPQExpBufferStr(&wait_query,
+ "WITH blocking(res) AS ("
"SELECT pg_catalog.pg_isolation_test_session_is_blocked($1, '{");
/* The spec syntax requires at least one session; assume that here. */
appendPQExpBufferStr(&wait_query, conns[1].backend_pid_str);
for (i = 2; i < nconns; i++)
appendPQExpBuffer(&wait_query, ",%s", conns[i].backend_pid_str);
- appendPQExpBufferStr(&wait_query, "}')");
+ appendPQExpBufferStr(&wait_query, "}') UNION "
+ "SELECT pg_catalog.pg_isolation_test_session_is_blocked(pid, '{}') FROM pg_stat_activity WHERE leader_pid=$1) SELECT bool_or(res) FROM blocking");
res = PQprepare(conns[0].conn, PREP_WAITING, wait_query.data, 0, NULL);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
--
2.47.3
--=-=-=--
^ permalink raw reply [nested|flat] 17+ messages in thread
* [PATCH] Reproduce filtering issue.
@ 2026-03-23 11:50 Antonin Houska <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Antonin Houska @ 2026-03-23 11:50 UTC (permalink / raw)
---
contrib/test_decoding/expected/filtering.out | 74 ++++++++++++++
contrib/test_decoding/specs/filtering.spec | 101 +++++++++++++++++++
src/backend/executor/nodeModifyTable.c | 2 +
src/backend/replication/logical/snapbuild.c | 3 +
src/test/isolation/isolationtester.c | 9 +-
5 files changed, 188 insertions(+), 1 deletion(-)
create mode 100644 contrib/test_decoding/expected/filtering.out
create mode 100644 contrib/test_decoding/specs/filtering.spec
diff --git a/contrib/test_decoding/expected/filtering.out b/contrib/test_decoding/expected/filtering.out
new file mode 100644
index 00000000000..6ba9690509f
--- /dev/null
+++ b/contrib/test_decoding/expected/filtering.out
@@ -0,0 +1,74 @@
+Parsed test spec with 5 sessions
+
+starting permutation: s1_assign_xid s3_repack s2_assign_xid s1_rollback s4_insert s5_wakeup_snapbuild s2_rollback s5_wakeup_insert_speculative s4_insert_commit s5_wakeup_repack
+injection_points_attach
+-----------------------
+
+(1 row)
+
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step s1_assign_xid:
+ BEGIN;
+ CREATE TABLE c(i int);
+
+step s3_repack:
+ REPACK (CONCURRENTLY) a;
+ <waiting ...>
+step s2_assign_xid:
+ BEGIN;
+ CREATE TABLE d(i int);
+
+step s1_rollback:
+ ROLLBACK;
+
+step s4_insert:
+ BEGIN;
+ INSERT INTO t(i)
+ SELECT max(i) + 1 FROM t ON CONFLICT (i) DO UPDATE SET i=EXCLUDED.i;
+ <waiting ...>
+step s5_wakeup_snapbuild:
+ SELECT injection_points_wakeup('snapbuild-full');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step s2_rollback:
+ ROLLBACK;
+
+step s5_wakeup_insert_speculative:
+ SELECT injection_points_wakeup('insert-speculative-before-confirm');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step s4_insert: <... completed>
+step s4_insert_commit:
+ COMMIT;
+
+step s5_wakeup_repack:
+ SELECT injection_points_wakeup('repack-concurrently-before-lock');
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step s3_repack: <... completed>
+injection_points_detach
+-----------------------
+
+(1 row)
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
diff --git a/contrib/test_decoding/specs/filtering.spec b/contrib/test_decoding/specs/filtering.spec
new file mode 100644
index 00000000000..a02c9f8facb
--- /dev/null
+++ b/contrib/test_decoding/specs/filtering.spec
@@ -0,0 +1,101 @@
+setup
+{
+ CREATE TABLE a(i int primary key, j int) WITH (autovacuum_enabled = off);
+ INSERT INTO a(i, j) VALUES (1, 1), (2, 2);
+ CREATE TABLE t(i int primary key);
+ INSERT INTO t(i) VALUES (1);
+ CREATE EXTENSION injection_points;
+}
+
+session s1
+step s1_assign_xid
+{
+ BEGIN;
+ CREATE TABLE c(i int);
+}
+step s1_rollback
+{
+ ROLLBACK;
+}
+
+session s2
+step s2_assign_xid
+{
+ BEGIN;
+ CREATE TABLE d(i int);
+}
+step s2_rollback
+{
+ ROLLBACK;
+}
+
+session s3
+setup
+{
+ SELECT injection_points_attach('snapbuild-full', 'wait');
+ SELECT injection_points_attach('repack-concurrently-before-lock', 'wait');
+}
+step s3_repack
+{
+ REPACK (CONCURRENTLY) a;
+}
+teardown
+{
+ SELECT injection_points_detach('repack-concurrently-before-lock');
+ SELECT injection_points_detach('snapbuild-full');
+}
+
+session s4
+setup
+{
+ SELECT injection_points_set_local();
+ SELECT injection_points_attach('insert-speculative-before-confirm', 'wait');
+}
+step s4_insert
+{
+ BEGIN;
+ INSERT INTO t(i)
+ SELECT max(i) + 1 FROM t ON CONFLICT (i) DO UPDATE SET i=EXCLUDED.i;
+}
+step s4_insert_commit
+{
+ COMMIT;
+}
+teardown
+{
+ SELECT injection_points_detach('insert-speculative-before-confirm');
+}
+
+session s5
+step s5_wakeup_snapbuild
+{
+ SELECT injection_points_wakeup('snapbuild-full');
+}
+step s5_wakeup_insert_speculative
+{
+ SELECT injection_points_wakeup('insert-speculative-before-confirm');
+}
+step s5_wakeup_repack
+{
+ SELECT injection_points_wakeup('repack-concurrently-before-lock');
+}
+
+permutation
+# Bring the snapshot builder to the FULL_SNAPSHOT state.
+s1_assign_xid
+s3_repack
+s2_assign_xid
+s1_rollback
+# Perform the speculative insert, but no confirmation so far. The snapshot
+# builder should decode it.
+s4_insert
+# Let the snapshout builder achieve CONSISTENT state and finish the setup.
+s5_wakeup_snapbuild
+s2_rollback
+# While REPACK is waiting on repack-concurrently-before-lock, let the insert
+# get confirmed. Relation filtering is now enabled.
+s5_wakeup_insert_speculative
+s4_insert_commit
+# REPACK should now decode the speculative insert and decode the speculative
+# insert (with the confirmation record filtered out).
+s5_wakeup_repack
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 680c29f35d5..6d5482e5746 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1232,6 +1232,8 @@ ExecInsert(ModifyTableContext *context,
slot, arbiterIndexes,
&specConflict);
+ INJECTION_POINT("insert-speculative-before-confirm", NULL);
+
/* adjust the tuple's state accordingly */
table_tuple_complete_speculative(resultRelationDesc, slot,
specToken, !specConflict);
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index fbdd4600a2b..883e5b6e18f 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -141,6 +141,7 @@
#include "storage/procarray.h"
#include "storage/standby.h"
#include "utils/builtins.h"
+#include "utils/injection_point.h"
#include "utils/memutils.h"
#include "utils/snapmgr.h"
#include "utils/snapshot.h"
@@ -1390,6 +1391,8 @@ SnapBuildFindSnapshot(SnapBuild *builder, XLogRecPtr lsn, xl_running_xacts *runn
builder->state = SNAPBUILD_FULL_SNAPSHOT;
builder->next_phase_at = running->nextXid;
+ INJECTION_POINT("snapbuild-full", NULL);
+
ereport(LOG,
errmsg("logical decoding found initial consistent point at %X/%08X",
LSN_FORMAT_ARGS(lsn)),
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index 440c875b8ac..8f17ee412c9 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -216,15 +216,22 @@ main(int argc, char **argv)
* exactly expect concurrent use of test tables. However, autovacuum will
* occasionally take AccessExclusiveLock to truncate a table, and we must
* ignore that transient wait.
+ *
+ * If the session's backend is blocked, and if its background worker is
+ * waiting on an injection point, we assume that the injection point is
+ * the reason for the backend to be blocked. That's what we check in the
+ * second query of the UNION. XXX Should we use a separate query for that?
*/
initPQExpBuffer(&wait_query);
appendPQExpBufferStr(&wait_query,
+ "WITH blocking(res) AS ("
"SELECT pg_catalog.pg_isolation_test_session_is_blocked($1, '{");
/* The spec syntax requires at least one session; assume that here. */
appendPQExpBufferStr(&wait_query, conns[1].backend_pid_str);
for (i = 2; i < nconns; i++)
appendPQExpBuffer(&wait_query, ",%s", conns[i].backend_pid_str);
- appendPQExpBufferStr(&wait_query, "}')");
+ appendPQExpBufferStr(&wait_query, "}') UNION "
+ "SELECT pg_catalog.pg_isolation_test_session_is_blocked(pid, '{}') FROM pg_stat_activity WHERE leader_pid=$1) SELECT bool_or(res) FROM blocking");
res = PQprepare(conns[0].conn, PREP_WAITING, wait_query.data, 0, NULL);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
--
2.47.3
--=-=-=--
^ permalink raw reply [nested|flat] 17+ messages in thread
end of thread, other threads:[~2026-03-23 11:50 UTC | newest]
Thread overview: 17+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-09-05 12:29 [PATCH v11 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]>
2019-09-05 12:29 [PATCH v9 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]>
2019-09-05 12:29 [PATCH v6 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]>
2019-09-05 12:29 [PATCH v12 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]>
2019-09-05 12:29 [PATCH v8 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]>
2019-09-05 12:29 [PATCH v7 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]>
2019-09-05 12:29 [PATCH v5 3/3] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]>
2019-09-05 12:29 [PATCH v7 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]>
2024-11-12 17:05 Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
2024-11-12 19:11 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Alvaro Herrera <[email protected]>
2024-11-12 22:13 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
2025-10-10 16:41 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
2025-10-11 05:24 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Chao Li <[email protected]>
2025-10-11 10:10 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Ranier Vilela <[email protected]>
2025-10-11 11:59 ` Re: Fix array access (src/bin/pg_dump/pg_dump.c) Chao Li <[email protected]>
2026-03-23 11:50 [PATCH] Reproduce filtering issue. Antonin Houska <[email protected]>
2026-03-23 11:50 [PATCH] Reproduce filtering issue. Antonin Houska <[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