public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v6 4/4] Change policy of XLog read-buffer allocation
18+ messages / 7 participants
[nested] [flat]
* [PATCH v6 4/4] Change policy of XLog read-buffer allocation
@ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 18+ 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] 18+ 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; 18+ 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] 18+ messages in thread
* [PATCH v11 4/4] Change policy of XLog read-buffer allocation
@ 2019-09-05 12:29 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 18+ 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] 18+ 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; 18+ 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] 18+ 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; 18+ 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] 18+ 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; 18+ 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] 18+ 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; 18+ 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] 18+ 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; 18+ 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] 18+ messages in thread
* [PATCH v6 2/2] allow syncfs in frontend utilities
@ 2023-07-28 22:56 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Nathan Bossart @ 2023-07-28 22:56 UTC (permalink / raw)
---
doc/src/sgml/config.sgml | 12 +---
doc/src/sgml/filelist.sgml | 1 +
doc/src/sgml/postgres.sgml | 1 +
doc/src/sgml/ref/initdb.sgml | 22 +++++++
doc/src/sgml/ref/pg_basebackup.sgml | 25 ++++++++
doc/src/sgml/ref/pg_checksums.sgml | 22 +++++++
doc/src/sgml/ref/pg_dump.sgml | 21 +++++++
doc/src/sgml/ref/pg_rewind.sgml | 22 +++++++
doc/src/sgml/ref/pgupgrade.sgml | 23 +++++++
doc/src/sgml/syncfs.sgml | 36 +++++++++++
src/backend/storage/file/fd.c | 4 +-
src/backend/utils/misc/guc_tables.c | 7 ++-
src/bin/initdb/initdb.c | 11 +++-
src/bin/pg_basebackup/pg_basebackup.c | 12 +++-
src/bin/pg_checksums/pg_checksums.c | 9 ++-
src/bin/pg_dump/pg_backup.h | 4 +-
src/bin/pg_dump/pg_backup_archiver.c | 14 +++--
src/bin/pg_dump/pg_backup_archiver.h | 1 +
src/bin/pg_dump/pg_backup_directory.c | 2 +-
src/bin/pg_dump/pg_dump.c | 10 ++-
src/bin/pg_rewind/file_ops.c | 2 +-
src/bin/pg_rewind/pg_rewind.c | 9 +++
src/bin/pg_rewind/pg_rewind.h | 2 +
src/bin/pg_upgrade/option.c | 13 ++++
src/bin/pg_upgrade/pg_upgrade.c | 6 +-
src/bin/pg_upgrade/pg_upgrade.h | 1 +
src/common/file_utils.c | 91 ++++++++++++++++++++++++++-
src/fe_utils/option_utils.c | 24 +++++++
src/include/common/file_utils.h | 11 +++-
src/include/fe_utils/option_utils.h | 4 ++
src/include/storage/fd.h | 6 --
src/tools/pgindent/typedefs.list | 1 +
32 files changed, 389 insertions(+), 40 deletions(-)
create mode 100644 doc/src/sgml/syncfs.sgml
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 694d667bf9..2c7d9f1262 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -10580,15 +10580,9 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
On Linux, <literal>syncfs</literal> may be used instead, to ask the
operating system to synchronize the whole file systems that contain the
data directory, the WAL files and each tablespace (but not any other
- file systems that may be reachable through symbolic links). This may
- be a lot faster than the <literal>fsync</literal> setting, because it
- doesn't need to open each file one by one. On the other hand, it may
- be slower if a file system is shared by other applications that
- modify a lot of files, since those files will also be written to disk.
- Furthermore, on versions of Linux before 5.8, I/O errors encountered
- while writing data to disk may not be reported to
- <productname>PostgreSQL</productname>, and relevant error messages may
- appear only in kernel logs.
+ file systems that may be reachable through symbolic links). See
+ <xref linkend="syncfs"/> for more information about using
+ <function>syncfs()</function>.
</para>
<para>
This parameter can only be set in the
diff --git a/doc/src/sgml/filelist.sgml b/doc/src/sgml/filelist.sgml
index 63b0fc2a46..df5d7c3025 100644
--- a/doc/src/sgml/filelist.sgml
+++ b/doc/src/sgml/filelist.sgml
@@ -184,6 +184,7 @@
<!ENTITY acronyms SYSTEM "acronyms.sgml">
<!ENTITY glossary SYSTEM "glossary.sgml">
<!ENTITY color SYSTEM "color.sgml">
+<!ENTITY syncfs SYSTEM "syncfs.sgml">
<!ENTITY features-supported SYSTEM "features-supported.sgml">
<!ENTITY features-unsupported SYSTEM "features-unsupported.sgml">
diff --git a/doc/src/sgml/postgres.sgml b/doc/src/sgml/postgres.sgml
index 2e271862fc..f629524be0 100644
--- a/doc/src/sgml/postgres.sgml
+++ b/doc/src/sgml/postgres.sgml
@@ -294,6 +294,7 @@ break is not needed in a wider output rendering.
&acronyms;
&glossary;
&color;
+ &syncfs;
&obsolete;
</part>
diff --git a/doc/src/sgml/ref/initdb.sgml b/doc/src/sgml/ref/initdb.sgml
index 22f1011781..8a09c5c438 100644
--- a/doc/src/sgml/ref/initdb.sgml
+++ b/doc/src/sgml/ref/initdb.sgml
@@ -365,6 +365,28 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry id="app-initdb-option-sync-method">
+ <term><option>--sync-method</option></term>
+ <listitem>
+ <para>
+ When set to <literal>fsync</literal>, which is the default,
+ <command>initdb</command> will recursively open and synchronize all
+ files in the data directory. The search for files will follow symbolic
+ links for the WAL directory and each configured tablespace.
+ </para>
+ <para>
+ On Linux, <literal>syncfs</literal> may be used instead to ask the
+ operating system to synchronize the whole file systems that contain the
+ data directory, the WAL files, and each tablespace. See
+ <xref linkend="syncfs"/> for more information about using
+ <function>syncfs()</function>.
+ </para>
+ <para>
+ This option has no effect when <option>--no-sync</option> is used.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="app-initdb-option-sync-only">
<term><option>-S</option></term>
<term><option>--sync-only</option></term>
diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index 79d3e657c3..d2b8ddd200 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -594,6 +594,31 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--sync-method</option></term>
+ <listitem>
+ <para>
+ When set to <literal>fsync</literal>, which is the default,
+ <command>pg_basebackup</command> will recursively open and synchronize
+ all files in the backup directory. When the plain format is used, the
+ search for files will follow symbolic links for the WAL directory and
+ each configured tablespace.
+ </para>
+ <para>
+ On Linux, <literal>syncfs</literal> may be used instead to ask the
+ operating system to synchronize the whole file system that contains the
+ backup directory. When the plain format is used,
+ <command>pg_basebackup</command> will also synchronize the file systems
+ that contain the WAL files and each tablespace. See
+ <xref linkend="syncfs"/> for more information about using
+ <function>syncfs()</function>.
+ </para>
+ <para>
+ This option has no effect when <option>--no-sync</option> is used.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-v</option></term>
<term><option>--verbose</option></term>
diff --git a/doc/src/sgml/ref/pg_checksums.sgml b/doc/src/sgml/ref/pg_checksums.sgml
index a3d0b0f0a3..7b44ba71cf 100644
--- a/doc/src/sgml/ref/pg_checksums.sgml
+++ b/doc/src/sgml/ref/pg_checksums.sgml
@@ -139,6 +139,28 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--sync-method</option></term>
+ <listitem>
+ <para>
+ When set to <literal>fsync</literal>, which is the default,
+ <command>pg_checksums</command> will recursively open and synchronize
+ all files in the data directory. The search for files will follow
+ symbolic links for the WAL directory and each configured tablespace.
+ </para>
+ <para>
+ On Linux, <literal>syncfs</literal> may be used instead to ask the
+ operating system to synchronize the whole file systems that contain the
+ data directory, the WAL files, and each tablespace. See
+ <xref linkend="syncfs"/> for more information about using
+ <function>syncfs()</function>.
+ </para>
+ <para>
+ This option has no effect when <option>--no-sync</option> is used.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-v</option></term>
<term><option>--verbose</option></term>
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index a3cf0608f5..c1e2220b3c 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -1179,6 +1179,27 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--sync-method</option></term>
+ <listitem>
+ <para>
+ When set to <literal>fsync</literal>, which is the default,
+ <command>pg_dump --format=directory</command> will recursively open and
+ synchronize all files in the archive directory.
+ </para>
+ <para>
+ On Linux, <literal>syncfs</literal> may be used instead to ask the
+ operating system to synchronize the whole file system that contains the
+ archive directory. See <xref linkend="syncfs"/> for more information
+ about using <function>syncfs()</function>.
+ </para>
+ <para>
+ This option has no effect when <option>--no-sync</option> is used or
+ <option>--format</option> is not set to <literal>directory</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--table-and-children=<replaceable class="parameter">pattern</replaceable></option></term>
<listitem>
diff --git a/doc/src/sgml/ref/pg_rewind.sgml b/doc/src/sgml/ref/pg_rewind.sgml
index 15cddd086b..80dff16168 100644
--- a/doc/src/sgml/ref/pg_rewind.sgml
+++ b/doc/src/sgml/ref/pg_rewind.sgml
@@ -284,6 +284,28 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--sync-method</option></term>
+ <listitem>
+ <para>
+ When set to <literal>fsync</literal>, which is the default,
+ <command>pg_rewind</command> will recursively open and synchronize all
+ files in the data directory. The search for files will follow symbolic
+ links for the WAL directory and each configured tablespace.
+ </para>
+ <para>
+ On Linux, <literal>syncfs</literal> may be used instead to ask the
+ operating system to synchronize the whole file systems that contain the
+ data directory, the WAL files, and each tablespace. See
+ <xref linkend="syncfs"/> for more information about using
+ <function>syncfs()</function>.
+ </para>
+ <para>
+ This option has no effect when <option>--no-sync</option> is used.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-V</option></term>
<term><option>--version</option></term>
diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml
index 7816b4c685..6c033485ea 100644
--- a/doc/src/sgml/ref/pgupgrade.sgml
+++ b/doc/src/sgml/ref/pgupgrade.sgml
@@ -190,6 +190,29 @@ PostgreSQL documentation
variable <envar>PGSOCKETDIR</envar></para></listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--sync-method</option></term>
+ <listitem>
+ <para>
+ When set to <literal>fsync</literal>, which is the default,
+ <command>pg_upgrade</command> will recursively open and synchronize all
+ files in the upgraded cluster's data directory. The search for files
+ will follow symbolic links for the WAL directory and each configured
+ tablespace.
+ </para>
+ <para>
+ On Linux, <literal>syncfs</literal> may be used instead to ask the
+ operating system to synchronize the whole file systems that contain the
+ upgrade cluster's data directory, its WAL files, and each tablespace.
+ See <xref linkend="syncfs"/> for more information about using
+ <function>syncfs()</function>.
+ </para>
+ <para>
+ This option has no effect when <option>--no-sync</option> is used.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-U</option> <replaceable>username</replaceable></term>
<term><option>--username=</option><replaceable>username</replaceable></term>
diff --git a/doc/src/sgml/syncfs.sgml b/doc/src/sgml/syncfs.sgml
new file mode 100644
index 0000000000..00457d2457
--- /dev/null
+++ b/doc/src/sgml/syncfs.sgml
@@ -0,0 +1,36 @@
+<!-- doc/src/sgml/syncfs.sgml -->
+
+<appendix id="syncfs">
+ <title><function>syncfs()</function> Caveats</title>
+
+ <indexterm zone="syncfs">
+ <primary>syncfs</primary>
+ </indexterm>
+
+ <para>
+ On Linux <function>syncfs()</function> may be specified for some
+ configuration parameters (e.g.,
+ <xref linkend="guc-recovery-init-sync-method"/>), server applications (e.g.,
+ <application>pg_upgrade</application>), and client applications (e.g.,
+ <application>pg_basebackup</application>) that involve synchronizing many
+ files to disk. <function>syncfs()</function> is advantageous in many cases,
+ but there are some trade-offs to keep in mind.
+ </para>
+
+ <para>
+ Since <function>syncfs()</function> instructs the operating system to
+ synchronize a whole file system, it typically requires many fewer system
+ calls than using <function>fsync()</function> to synchronize each file one by
+ one. Therefore, using <function>syncfs()</function> may be a lot faster than
+ using <function>fsync()</function>. However, it may be slower if a file
+ system is shared by other applications that modify a lot of files, since
+ those files will also be written to disk.
+ </para>
+
+ <para>
+ Furthermore, on versions of Linux before 5.8, I/O errors encountered while
+ writing data to disk may not be reported to the calling program, and relevant
+ error messages may appear only in kernel logs.
+ </para>
+
+</appendix>
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index b490a76ba7..3fed475c38 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -162,7 +162,7 @@ int max_safe_fds = FD_MINFREE; /* default if not changed */
bool data_sync_retry = false;
/* How SyncDataDirectory() should do its job. */
-int recovery_init_sync_method = RECOVERY_INIT_SYNC_METHOD_FSYNC;
+int recovery_init_sync_method = DATA_DIR_SYNC_METHOD_FSYNC;
/* Which kinds of files should be opened with PG_O_DIRECT. */
int io_direct_flags;
@@ -3513,7 +3513,7 @@ SyncDataDirectory(void)
}
#ifdef HAVE_SYNCFS
- if (recovery_init_sync_method == RECOVERY_INIT_SYNC_METHOD_SYNCFS)
+ if (recovery_init_sync_method == DATA_DIR_SYNC_METHOD_SYNCFS)
{
DIR *dir;
struct dirent *de;
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index e565a3092f..5abebe9a9c 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -41,6 +41,7 @@
#include "commands/trigger.h"
#include "commands/user.h"
#include "commands/vacuum.h"
+#include "common/file_utils.h"
#include "common/scram-common.h"
#include "jit/jit.h"
#include "libpq/auth.h"
@@ -430,9 +431,9 @@ StaticAssertDecl(lengthof(ssl_protocol_versions_info) == (PG_TLS1_3_VERSION + 2)
"array length mismatch");
static const struct config_enum_entry recovery_init_sync_method_options[] = {
- {"fsync", RECOVERY_INIT_SYNC_METHOD_FSYNC, false},
+ {"fsync", DATA_DIR_SYNC_METHOD_FSYNC, false},
#ifdef HAVE_SYNCFS
- {"syncfs", RECOVERY_INIT_SYNC_METHOD_SYNCFS, false},
+ {"syncfs", DATA_DIR_SYNC_METHOD_SYNCFS, false},
#endif
{NULL, 0, false}
};
@@ -4964,7 +4965,7 @@ struct config_enum ConfigureNamesEnum[] =
gettext_noop("Sets the method for synchronizing the data directory before crash recovery."),
},
&recovery_init_sync_method,
- RECOVERY_INIT_SYNC_METHOD_FSYNC, recovery_init_sync_method_options,
+ DATA_DIR_SYNC_METHOD_FSYNC, recovery_init_sync_method_options,
NULL, NULL, NULL
},
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 905b979947..543927b8b4 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -165,6 +165,7 @@ static bool show_setting = false;
static bool data_checksums = false;
static char *xlog_dir = NULL;
static int wal_segment_size_mb = (DEFAULT_XLOG_SEG_SIZE) / (1024 * 1024);
+static DataDirSyncMethod sync_method = DATA_DIR_SYNC_METHOD_FSYNC;
/* internal vars */
@@ -2466,6 +2467,7 @@ usage(const char *progname)
printf(_(" -N, --no-sync do not wait for changes to be written safely to disk\n"));
printf(_(" --no-instructions do not print instructions for next steps\n"));
printf(_(" -s, --show show internal settings\n"));
+ printf(_(" --sync-method=METHOD set method for syncing files to disk\n"));
printf(_(" -S, --sync-only only sync database files to disk, then exit\n"));
printf(_("\nOther options:\n"));
printf(_(" -V, --version output version information, then exit\n"));
@@ -3106,6 +3108,7 @@ main(int argc, char *argv[])
{"locale-provider", required_argument, NULL, 15},
{"icu-locale", required_argument, NULL, 16},
{"icu-rules", required_argument, NULL, 17},
+ {"sync-method", required_argument, NULL, 18},
{NULL, 0, NULL, 0}
};
@@ -3286,6 +3289,10 @@ main(int argc, char *argv[])
case 17:
icu_rules = pg_strdup(optarg);
break;
+ case 18:
+ if (!parse_sync_method(optarg, &sync_method))
+ exit(1);
+ break;
default:
/* getopt_long already emitted a complaint */
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
@@ -3333,7 +3340,7 @@ main(int argc, char *argv[])
fputs(_("syncing data to disk ... "), stdout);
fflush(stdout);
- fsync_pgdata(pg_data, PG_VERSION_NUM);
+ fsync_pgdata(pg_data, PG_VERSION_NUM, sync_method);
check_ok();
return 0;
}
@@ -3396,7 +3403,7 @@ main(int argc, char *argv[])
{
fputs(_("syncing data to disk ... "), stdout);
fflush(stdout);
- fsync_pgdata(pg_data, PG_VERSION_NUM);
+ fsync_pgdata(pg_data, PG_VERSION_NUM, sync_method);
check_ok();
}
else
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 1dc8efe0cb..977c793a67 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -148,6 +148,7 @@ static bool verify_checksums = true;
static bool manifest = true;
static bool manifest_force_encode = false;
static char *manifest_checksums = NULL;
+static DataDirSyncMethod sync_method = DATA_DIR_SYNC_METHOD_FSYNC;
static bool success = false;
static bool made_new_pgdata = false;
@@ -424,6 +425,8 @@ usage(void)
printf(_(" --no-slot prevent creation of temporary replication slot\n"));
printf(_(" --no-verify-checksums\n"
" do not verify checksums\n"));
+ printf(_(" --sync-method=METHOD\n"
+ " set method for syncing files to disk\n"));
printf(_(" -?, --help show this help, then exit\n"));
printf(_("\nConnection options:\n"));
printf(_(" -d, --dbname=CONNSTR connection string\n"));
@@ -2199,11 +2202,11 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
if (format == 't')
{
if (strcmp(basedir, "-") != 0)
- (void) fsync_dir_recurse(basedir);
+ (void) fsync_dir_recurse(basedir, sync_method);
}
else
{
- (void) fsync_pgdata(basedir, serverVersion);
+ (void) fsync_pgdata(basedir, serverVersion, sync_method);
}
}
@@ -2281,6 +2284,7 @@ main(int argc, char **argv)
{"no-manifest", no_argument, NULL, 5},
{"manifest-force-encode", no_argument, NULL, 6},
{"manifest-checksums", required_argument, NULL, 7},
+ {"sync-method", required_argument, NULL, 8},
{NULL, 0, NULL, 0}
};
int c;
@@ -2452,6 +2456,10 @@ main(int argc, char **argv)
case 7:
manifest_checksums = pg_strdup(optarg);
break;
+ case 8:
+ if (!parse_sync_method(optarg, &sync_method))
+ exit(1);
+ break;
default:
/* getopt_long already emitted a complaint */
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 9011a19b4e..9fb2c5b3c7 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -44,6 +44,7 @@ static char *only_filenode = NULL;
static bool do_sync = true;
static bool verbose = false;
static bool showprogress = false;
+static DataDirSyncMethod sync_method = DATA_DIR_SYNC_METHOD_FSYNC;
typedef enum
{
@@ -77,6 +78,7 @@ usage(void)
printf(_(" -f, --filenode=FILENODE check only relation with specified filenode\n"));
printf(_(" -N, --no-sync do not wait for changes to be written safely to disk\n"));
printf(_(" -P, --progress show progress information\n"));
+ printf(_(" --sync-method=METHOD set method for syncing files to disk\n"));
printf(_(" -v, --verbose output verbose messages\n"));
printf(_(" -V, --version output version information, then exit\n"));
printf(_(" -?, --help show this help, then exit\n"));
@@ -435,6 +437,7 @@ main(int argc, char *argv[])
{"no-sync", no_argument, NULL, 'N'},
{"progress", no_argument, NULL, 'P'},
{"verbose", no_argument, NULL, 'v'},
+ {"sync-method", required_argument, NULL, 1},
{NULL, 0, NULL, 0}
};
@@ -493,6 +496,10 @@ main(int argc, char *argv[])
case 'v':
verbose = true;
break;
+ case 1:
+ if (!parse_sync_method(optarg, &sync_method))
+ exit(1);
+ break;
default:
/* getopt_long already emitted a complaint */
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
@@ -623,7 +630,7 @@ main(int argc, char *argv[])
if (do_sync)
{
pg_log_info("syncing data directory");
- fsync_pgdata(DataDir, PG_VERSION_NUM);
+ fsync_pgdata(DataDir, PG_VERSION_NUM, sync_method);
}
pg_log_info("updating control file");
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index aba780ef4b..3a57cdd97d 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -24,6 +24,7 @@
#define PG_BACKUP_H
#include "common/compression.h"
+#include "common/file_utils.h"
#include "fe_utils/simple_list.h"
#include "libpq-fe.h"
@@ -307,7 +308,8 @@ extern Archive *OpenArchive(const char *FileSpec, const ArchiveFormat fmt);
extern Archive *CreateArchive(const char *FileSpec, const ArchiveFormat fmt,
const pg_compress_specification compression_spec,
bool dosync, ArchiveMode mode,
- SetupWorkerPtrType setupDumpWorker);
+ SetupWorkerPtrType setupDumpWorker,
+ DataDirSyncMethod sync_method);
/* The --list option */
extern void PrintTOCSummary(Archive *AHX);
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 39ebcfec32..4d83381d84 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -66,7 +66,8 @@ typedef struct _parallelReadyList
static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt,
const pg_compress_specification compression_spec,
bool dosync, ArchiveMode mode,
- SetupWorkerPtrType setupWorkerPtr);
+ SetupWorkerPtrType setupWorkerPtr,
+ DataDirSyncMethod sync_method);
static void _getObjectDescription(PQExpBuffer buf, const TocEntry *te);
static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData);
static char *sanitize_line(const char *str, bool want_hyphen);
@@ -238,11 +239,12 @@ Archive *
CreateArchive(const char *FileSpec, const ArchiveFormat fmt,
const pg_compress_specification compression_spec,
bool dosync, ArchiveMode mode,
- SetupWorkerPtrType setupDumpWorker)
+ SetupWorkerPtrType setupDumpWorker,
+ DataDirSyncMethod sync_method)
{
ArchiveHandle *AH = _allocAH(FileSpec, fmt, compression_spec,
- dosync, mode, setupDumpWorker);
+ dosync, mode, setupDumpWorker, sync_method);
return (Archive *) AH;
}
@@ -257,7 +259,8 @@ OpenArchive(const char *FileSpec, const ArchiveFormat fmt)
compression_spec.algorithm = PG_COMPRESSION_NONE;
AH = _allocAH(FileSpec, fmt, compression_spec, true,
- archModeRead, setupRestoreWorker);
+ archModeRead, setupRestoreWorker,
+ DATA_DIR_SYNC_METHOD_FSYNC);
return (Archive *) AH;
}
@@ -2233,7 +2236,7 @@ static ArchiveHandle *
_allocAH(const char *FileSpec, const ArchiveFormat fmt,
const pg_compress_specification compression_spec,
bool dosync, ArchiveMode mode,
- SetupWorkerPtrType setupWorkerPtr)
+ SetupWorkerPtrType setupWorkerPtr, DataDirSyncMethod sync_method)
{
ArchiveHandle *AH;
CompressFileHandle *CFH;
@@ -2287,6 +2290,7 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
AH->mode = mode;
AH->compression_spec = compression_spec;
AH->dosync = dosync;
+ AH->sync_method = sync_method;
memset(&(AH->sqlparse), 0, sizeof(AH->sqlparse));
diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h
index 18b38c17ab..b07673933d 100644
--- a/src/bin/pg_dump/pg_backup_archiver.h
+++ b/src/bin/pg_dump/pg_backup_archiver.h
@@ -312,6 +312,7 @@ struct _archiveHandle
pg_compress_specification compression_spec; /* Requested specification for
* compression */
bool dosync; /* data requested to be synced on sight */
+ DataDirSyncMethod sync_method;
ArchiveMode mode; /* File mode - r or w */
void *formatData; /* Header data specific to file format */
diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c
index 7f2ac7c7fd..6faa3a511f 100644
--- a/src/bin/pg_dump/pg_backup_directory.c
+++ b/src/bin/pg_dump/pg_backup_directory.c
@@ -613,7 +613,7 @@ _CloseArchive(ArchiveHandle *AH)
* individually. Just recurse once through all the files generated.
*/
if (AH->dosync)
- fsync_dir_recurse(ctx->directory);
+ fsync_dir_recurse(ctx->directory, AH->sync_method);
}
AH->FH = NULL;
}
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 65f64c282d..dc4a28e81e 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -357,6 +357,7 @@ main(int argc, char **argv)
char *compression_algorithm_str = "none";
char *error_detail = NULL;
bool user_compression_defined = false;
+ DataDirSyncMethod sync_method = DATA_DIR_SYNC_METHOD_FSYNC;
static DumpOptions dopt;
@@ -431,6 +432,7 @@ main(int argc, char **argv)
{"table-and-children", required_argument, NULL, 12},
{"exclude-table-and-children", required_argument, NULL, 13},
{"exclude-table-data-and-children", required_argument, NULL, 14},
+ {"sync-method", required_argument, NULL, 15},
{NULL, 0, NULL, 0}
};
@@ -657,6 +659,11 @@ main(int argc, char **argv)
optarg);
break;
+ case 15:
+ if (!parse_sync_method(optarg, &sync_method))
+ exit_nicely(1);
+ break;
+
default:
/* getopt_long already emitted a complaint */
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
@@ -777,7 +784,7 @@ main(int argc, char **argv)
/* Open the output file */
fout = CreateArchive(filename, archiveFormat, compression_spec,
- dosync, archiveMode, setupDumpWorker);
+ dosync, archiveMode, setupDumpWorker, sync_method);
/* Make dump options accessible right away */
SetArchiveOptions(fout, &dopt, NULL);
@@ -1068,6 +1075,7 @@ help(const char *progname)
" compress as specified\n"));
printf(_(" --lock-wait-timeout=TIMEOUT fail after waiting TIMEOUT for a table lock\n"));
printf(_(" --no-sync do not wait for changes to be written safely to disk\n"));
+ printf(_(" --sync-method=METHOD set method for syncing files to disk\n"));
printf(_(" -?, --help show this help, then exit\n"));
printf(_("\nOptions controlling the output content:\n"));
diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c
index 25996b4da4..451fb1856e 100644
--- a/src/bin/pg_rewind/file_ops.c
+++ b/src/bin/pg_rewind/file_ops.c
@@ -296,7 +296,7 @@ sync_target_dir(void)
if (!do_sync || dry_run)
return;
- fsync_pgdata(datadir_target, PG_VERSION_NUM);
+ fsync_pgdata(datadir_target, PG_VERSION_NUM, sync_method);
}
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 7f69f02441..bfd44a284e 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -22,6 +22,7 @@
#include "common/file_perm.h"
#include "common/restricted_token.h"
#include "common/string.h"
+#include "fe_utils/option_utils.h"
#include "fe_utils/recovery_gen.h"
#include "fe_utils/string_utils.h"
#include "file_ops.h"
@@ -74,6 +75,7 @@ bool showprogress = false;
bool dry_run = false;
bool do_sync = true;
bool restore_wal = false;
+DataDirSyncMethod sync_method = DATA_DIR_SYNC_METHOD_FSYNC;
/* Target history */
TimeLineHistoryEntry *targetHistory;
@@ -107,6 +109,7 @@ usage(const char *progname)
" file when running target cluster\n"));
printf(_(" --debug write a lot of debug messages\n"));
printf(_(" --no-ensure-shutdown do not automatically fix unclean shutdown\n"));
+ printf(_(" --sync-method=METHOD set method for syncing files to disk\n"));
printf(_(" -V, --version output version information, then exit\n"));
printf(_(" -?, --help show this help, then exit\n"));
printf(_("\nReport bugs to <%s>.\n"), PACKAGE_BUGREPORT);
@@ -131,6 +134,7 @@ main(int argc, char **argv)
{"no-sync", no_argument, NULL, 'N'},
{"progress", no_argument, NULL, 'P'},
{"debug", no_argument, NULL, 3},
+ {"sync-method", required_argument, NULL, 6},
{NULL, 0, NULL, 0}
};
int option_index;
@@ -218,6 +222,11 @@ main(int argc, char **argv)
config_file = pg_strdup(optarg);
break;
+ case 6:
+ if (!parse_sync_method(optarg, &sync_method))
+ exit(1);
+ break;
+
default:
/* getopt_long already emitted a complaint */
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h
index ef8bdc1fbb..05729adfef 100644
--- a/src/bin/pg_rewind/pg_rewind.h
+++ b/src/bin/pg_rewind/pg_rewind.h
@@ -13,6 +13,7 @@
#include "access/timeline.h"
#include "common/logging.h"
+#include "common/file_utils.h"
#include "datapagemap.h"
#include "libpq-fe.h"
#include "storage/block.h"
@@ -24,6 +25,7 @@ extern bool showprogress;
extern bool dry_run;
extern bool do_sync;
extern int WalSegSz;
+extern DataDirSyncMethod sync_method;
/* Target history */
extern TimeLineHistoryEntry *targetHistory;
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 640361009e..b9d900d0db 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -14,6 +14,7 @@
#endif
#include "common/string.h"
+#include "fe_utils/option_utils.h"
#include "getopt_long.h"
#include "pg_upgrade.h"
#include "utils/pidfile.h"
@@ -57,12 +58,14 @@ parseCommandLine(int argc, char *argv[])
{"verbose", no_argument, NULL, 'v'},
{"clone", no_argument, NULL, 1},
{"copy", no_argument, NULL, 2},
+ {"sync-method", required_argument, NULL, 3},
{NULL, 0, NULL, 0}
};
int option; /* Command line option */
int optindex = 0; /* used by getopt_long */
int os_user_effective_id;
+ DataDirSyncMethod unused;
user_opts.do_sync = true;
user_opts.transfer_mode = TRANSFER_MODE_COPY;
@@ -199,6 +202,12 @@ parseCommandLine(int argc, char *argv[])
user_opts.transfer_mode = TRANSFER_MODE_COPY;
break;
+ case 3:
+ if (!parse_sync_method(optarg, &unused))
+ exit(1);
+ user_opts.sync_method = pg_strdup(optarg);
+ break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
os_info.progname);
@@ -209,6 +218,9 @@ parseCommandLine(int argc, char *argv[])
if (optind < argc)
pg_fatal("too many command-line arguments (first is \"%s\")", argv[optind]);
+ if (!user_opts.sync_method)
+ user_opts.sync_method = pg_strdup("fsync");
+
if (log_opts.verbose)
pg_log(PG_REPORT, "Running in verbose mode");
@@ -289,6 +301,7 @@ usage(void)
printf(_(" -V, --version display version information, then exit\n"));
printf(_(" --clone clone instead of copying files to new cluster\n"));
printf(_(" --copy copy files to new cluster (default)\n"));
+ printf(_(" --sync-method=METHOD set method for syncing files to disk\n"));
printf(_(" -?, --help show this help, then exit\n"));
printf(_("\n"
"Before running pg_upgrade you must:\n"
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 4562dafcff..96bfb67167 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -192,8 +192,10 @@ main(int argc, char **argv)
{
prep_status("Sync data directory to disk");
exec_prog(UTILITY_LOG_FILE, NULL, true, true,
- "\"%s/initdb\" --sync-only \"%s\"", new_cluster.bindir,
- new_cluster.pgdata);
+ "\"%s/initdb\" --sync-only \"%s\" --sync-method %s",
+ new_cluster.bindir,
+ new_cluster.pgdata,
+ user_opts.sync_method);
check_ok();
}
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 7afa96716e..842f3b6cd3 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -304,6 +304,7 @@ typedef struct
transferMode transfer_mode; /* copy files or link them? */
int jobs; /* number of processes/threads to use */
char *socketdir; /* directory to use for Unix sockets */
+ char *sync_method;
} UserOpts;
typedef struct
diff --git a/src/common/file_utils.c b/src/common/file_utils.c
index 74833c4acb..c977c990ad 100644
--- a/src/common/file_utils.c
+++ b/src/common/file_utils.c
@@ -51,6 +51,31 @@ static void walkdir(const char *path,
int (*action) (const char *fname, bool isdir),
bool process_symlinks);
+#ifdef HAVE_SYNCFS
+static void
+do_syncfs(const char *path)
+{
+ int fd;
+
+ fd = open(path, O_RDONLY, 0);
+
+ if (fd < 0)
+ {
+ pg_log_error("could not open file \"%s\": %m", path);
+ return;
+ }
+
+ if (syncfs(fd) < 0)
+ {
+ pg_log_error("could not synchronize file system for file \"%s\": %m", path);
+ (void) close(fd);
+ exit(EXIT_FAILURE);
+ }
+
+ (void) close(fd);
+}
+#endif
+
/*
* Issue fsync recursively on PGDATA and all its contents.
*
@@ -63,7 +88,8 @@ static void walkdir(const char *path,
*/
void
fsync_pgdata(const char *pg_data,
- int serverVersion)
+ int serverVersion,
+ DataDirSyncMethod sync_method)
{
bool xlog_is_symlink;
char pg_wal[MAXPGPATH];
@@ -89,6 +115,55 @@ fsync_pgdata(const char *pg_data,
xlog_is_symlink = true;
}
+#ifdef HAVE_SYNCFS
+ if (sync_method == DATA_DIR_SYNC_METHOD_SYNCFS)
+ {
+ DIR *dir;
+ struct dirent *de;
+
+ /*
+ * On Linux, we don't have to open every single file one by one. We
+ * can use syncfs() to sync whole filesystems. We only expect
+ * filesystem boundaries to exist where we tolerate symlinks, namely
+ * pg_wal and the tablespaces, so we call syncfs() for each of those
+ * directories.
+ */
+
+ /* Sync the top level pgdata directory. */
+ do_syncfs(pg_data);
+
+ /* If any tablespaces are configured, sync each of those. */
+ dir = opendir(pg_tblspc);
+ if (dir == NULL)
+ pg_log_error("could not open directory \"%s\": %m", pg_tblspc);
+ else
+ {
+ while (errno = 0, (de = readdir(dir)) != NULL)
+ {
+ char subpath[MAXPGPATH * 2];
+
+ if (strcmp(de->d_name, ".") == 0 ||
+ strcmp(de->d_name, "..") == 0)
+ continue;
+
+ snprintf(subpath, sizeof(subpath), "%s/%s", pg_tblspc, de->d_name);
+ do_syncfs(subpath);
+ }
+
+ if (errno)
+ pg_log_error("could not read directory \"%s\": %m", pg_tblspc);
+
+ (void) closedir(dir);
+ }
+
+ /* If pg_wal is a symlink, process that too. */
+ if (xlog_is_symlink)
+ do_syncfs(pg_wal);
+
+ return;
+ }
+#endif /* HAVE_SYNCFS */
+
/*
* If possible, hint to the kernel that we're soon going to fsync the data
* directory and its contents.
@@ -121,8 +196,20 @@ fsync_pgdata(const char *pg_data,
* This is a convenient wrapper on top of walkdir().
*/
void
-fsync_dir_recurse(const char *dir)
+fsync_dir_recurse(const char *dir, DataDirSyncMethod sync_method)
{
+#ifdef HAVE_SYNCFS
+ if (sync_method == DATA_DIR_SYNC_METHOD_SYNCFS)
+ {
+ /*
+ * On Linux, we don't have to open every single file one by one. We
+ * can use syncfs() to sync the whole filesystem.
+ */
+ do_syncfs(dir);
+ return;
+ }
+#endif /* HAVE_SYNCFS */
+
/*
* If possible, hint to the kernel that we're soon going to fsync the data
* directory and its contents.
diff --git a/src/fe_utils/option_utils.c b/src/fe_utils/option_utils.c
index 763c991015..36ac689964 100644
--- a/src/fe_utils/option_utils.c
+++ b/src/fe_utils/option_utils.c
@@ -82,3 +82,27 @@ option_parse_int(const char *optarg, const char *optname,
*result = val;
return true;
}
+
+bool
+parse_sync_method(const char *optarg, DataDirSyncMethod *sync_method)
+{
+ if (strcmp(optarg, "fsync") == 0)
+ *sync_method = DATA_DIR_SYNC_METHOD_FSYNC;
+ else if (strcmp(optarg, "syncfs") == 0)
+ {
+#ifdef HAVE_SYNCFS
+ *sync_method = DATA_DIR_SYNC_METHOD_SYNCFS;
+#else
+ pg_log_error("this build does not support sync method \"%s\"",
+ "syncfs");
+ return false;
+#endif
+ }
+ else
+ {
+ pg_log_error("unrecognized sync method: %s", optarg);
+ return false;
+ }
+
+ return true;
+}
diff --git a/src/include/common/file_utils.h b/src/include/common/file_utils.h
index dd1532bcb0..09d1e5d561 100644
--- a/src/include/common/file_utils.h
+++ b/src/include/common/file_utils.h
@@ -26,10 +26,17 @@ typedef enum PGFileType
struct iovec; /* avoid including port/pg_iovec.h here */
+typedef enum DataDirSyncMethod
+{
+ DATA_DIR_SYNC_METHOD_FSYNC,
+ DATA_DIR_SYNC_METHOD_SYNCFS
+} DataDirSyncMethod;
+
#ifdef FRONTEND
extern int fsync_fname(const char *fname, bool isdir);
-extern void fsync_pgdata(const char *pg_data, int serverVersion);
-extern void fsync_dir_recurse(const char *dir);
+extern void fsync_pgdata(const char *pg_data, int serverVersion,
+ DataDirSyncMethod sync_method);
+extern void fsync_dir_recurse(const char *dir, DataDirSyncMethod sync_method);
extern int durable_rename(const char *oldfile, const char *newfile);
extern int fsync_parent_path(const char *fname);
#endif
diff --git a/src/include/fe_utils/option_utils.h b/src/include/fe_utils/option_utils.h
index b7b0654cee..6f3a965916 100644
--- a/src/include/fe_utils/option_utils.h
+++ b/src/include/fe_utils/option_utils.h
@@ -14,6 +14,8 @@
#include "postgres_fe.h"
+#include "common/file_utils.h"
+
typedef void (*help_handler) (const char *progname);
extern void handle_help_version_opts(int argc, char *argv[],
@@ -22,5 +24,7 @@ extern void handle_help_version_opts(int argc, char *argv[],
extern bool option_parse_int(const char *optarg, const char *optname,
int min_range, int max_range,
int *result);
+extern bool parse_sync_method(const char *optarg,
+ DataDirSyncMethod *sync_method);
#endif /* OPTION_UTILS_H */
diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index aea30c0622..d9d5d9da5f 100644
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -46,12 +46,6 @@
#include <dirent.h>
#include <fcntl.h>
-typedef enum RecoveryInitSyncMethod
-{
- RECOVERY_INIT_SYNC_METHOD_FSYNC,
- RECOVERY_INIT_SYNC_METHOD_SYNCFS
-} RecoveryInitSyncMethod;
-
typedef int File;
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 49a33c0387..fe571c6265 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -545,6 +545,7 @@ DR_printtup
DR_sqlfunction
DR_transientrel
DWORD
+DataDirSyncMethod
DataDumperPtr
DataPageDeleteStack
DatabaseInfo
--
2.25.1
--bp/iNruPH9dso1Pn--
^ permalink raw reply [nested|flat] 18+ messages in thread
* Sorting regression of text function result since commit 586b98fdf1aae
@ 2023-12-11 20:09 Jehan-Guillaume de Rorthais <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Jehan-Guillaume de Rorthais @ 2023-12-11 20:09 UTC (permalink / raw)
To: pgsql-hackers
Hi,
A customer found what looks like a sort regression while testing his code from
v11 on a higher version. We hunt this regression down to commit 586b98fdf1aae,
introduced in v12.
Consider the following test case:
createdb -l fr_FR.utf8 -T template0 reg
psql reg <<<"
BEGIN;
CREATE TABLE IF NOT EXISTS reg
(
id bigint NOT NULL,
reg bytea NOT NULL
);
INSERT INTO reg VALUES
(1, convert_to( 'aaa', 'UTF8')),
(2, convert_to( 'aa}', 'UTF8'));
SELECT id FROM reg ORDER BY convert_from(reg, 'UTF8');"
In parent commit 68f6f2b7395fe, it results:
id
────
2
1
And in 586b98fdf1aae:
id
────
1
2
Looking at the plan, the sort node are different:
* 68f6f2b7395fe: Sort Key: (convert_from(reg, 'UTF8'::name))
* 586b98fdf1aae: Sort Key: (convert_from(reg, 'UTF8'::name)) COLLATE "C"
It looks like since 586b98fdf1aae, the result type collation of "convert_from"
is forced to "C", like the patch does for type "name", instead of the "default"
collation for type "text".
Looking at hints in the header comment of function "exprCollation", I poked
around and found that the result collation wrongly follow the input collation
in this case. With 586b98fdf1aae:
-- 2nd parameter type resolved as "name" so collation forced to "C"
SELECT id FROM reg ORDER BY convert_from(reg, 'UTF8');
-- 1
-- 2
-- Collation of 2nd parameter is forced to something else
SELECT id FROM reg ORDER BY convert_from(reg, 'UTF8' COLLATE \"default\");
-- 2
-- 1
-- Sort
-- Sort Key: (convert_from(reg, 'UTF8'::name COLLATE "default"))
-- -> Seq Scan on reg
It seems because the second parameter type is "name", the result collation
become "C" instead of being the collation associated with "text" type:
"default".
I couldn't find anything explaining this behavior in the changelog. It looks
like a regression to me, but if this is actually expected, maybe this deserve
some documentation patch?
Regards,
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Sorting regression of text function result since commit 586b98fdf1aae
@ 2023-12-11 20:43 Tom Lane <[email protected]>
parent: Jehan-Guillaume de Rorthais <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Tom Lane @ 2023-12-11 20:43 UTC (permalink / raw)
To: Jehan-Guillaume de Rorthais <[email protected]>; +Cc: pgsql-hackers
Jehan-Guillaume de Rorthais <[email protected]> writes:
> It looks like since 586b98fdf1aae, the result type collation of "convert_from"
> is forced to "C", like the patch does for type "name", instead of the "default"
> collation for type "text".
Well, convert_from() inherits its result collation from the input,
per the normal rules for collation assignment [1].
> Looking at hints in the header comment of function "exprCollation", I poked
> around and found that the result collation wrongly follow the input collation
> in this case.
It's not "wrong", it's what the SQL standard requires.
> I couldn't find anything explaining this behavior in the changelog. It looks
> like a regression to me, but if this is actually expected, maybe this deserve
> some documentation patch?
The v12 release notes do say
Type name now behaves much like a domain over type text that has
default collation “C”.
You'd have similar results from an expression involving such a domain,
I believe.
I'm less than excited about patching the v12 release notes four
years later. Maybe, if this point had come up in a more timely
fashion, we'd have mentioned it --- but it's hardly possible to
cover every potential implication of such a change in the
release notes.
regards, tom lane
[1] https://www.postgresql.org/docs/current/collation.html#COLLATION-CONCEPTS
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Sorting regression of text function result since commit 586b98fdf1aae
@ 2023-12-12 10:52 Jehan-Guillaume de Rorthais <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Jehan-Guillaume de Rorthais @ 2023-12-12 10:52 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: pgsql-hackers
On Mon, 11 Dec 2023 15:43:12 -0500
Tom Lane <[email protected]> wrote:
> Jehan-Guillaume de Rorthais <[email protected]> writes:
> > It looks like since 586b98fdf1aae, the result type collation of
> > "convert_from" is forced to "C", like the patch does for type "name",
> > instead of the "default" collation for type "text".
>
> Well, convert_from() inherits its result collation from the input,
> per the normal rules for collation assignment [1].
>
> > Looking at hints in the header comment of function "exprCollation", I poked
> > around and found that the result collation wrongly follow the input
> > collation in this case.
>
> It's not "wrong", it's what the SQL standard requires.
Mh, OK. This is at least a surprising behavior. Having a non-data related
argument impacting the result collation seems counter-intuitive. But I
understand this is by standard, no need to discuss it.
> > I couldn't find anything explaining this behavior in the changelog. It looks
> > like a regression to me, but if this is actually expected, maybe this
> > deserve some documentation patch?
>
> The v12 release notes do say
>
> Type name now behaves much like a domain over type text that has
> default collation “C”.
Sure, and I saw it, but reading at this entry, I couldn't guess this could have
such implication on text result from a function call. That's why I hunt for
the precise commit and was surprise to find this was the actual change.
> You'd have similar results from an expression involving such a domain,
> I believe.
>
> I'm less than excited about patching the v12 release notes four
> years later. Maybe, if this point had come up in a more timely
> fashion, we'd have mentioned it --- but it's hardly possible to
> cover every potential implication of such a change in the
> release notes.
This could have been documented in the collation concept page, as a trap to be
aware of. A link from the release note to such a small paragraph would have
been enough to warn devs this might have implications when mixed with other
collatable types. But I understand we can not document all the traps paving the
way to the standard anyway.
Thank you for your explanation!
Regards,
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Fix array access (src/bin/pg_dump/pg_dump.c)
@ 2025-10-10 16:41 Ranier Vilela <[email protected]>
0 siblings, 1 reply; 18+ 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] 18+ messages in thread
* Re: Fix array access (src/bin/pg_dump/pg_dump.c)
@ 2025-10-11 05:24 Chao Li <[email protected]>
parent: Ranier Vilela <[email protected]>
0 siblings, 1 reply; 18+ 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] 18+ messages in thread
* Re: Fix array access (src/bin/pg_dump/pg_dump.c)
@ 2025-10-11 10:10 Ranier Vilela <[email protected]>
parent: Chao Li <[email protected]>
0 siblings, 1 reply; 18+ 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] 18+ messages in thread
* Re: Fix array access (src/bin/pg_dump/pg_dump.c)
@ 2025-10-11 11:59 Chao Li <[email protected]>
parent: Ranier Vilela <[email protected]>
0 siblings, 0 replies; 18+ 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] 18+ messages in thread
* [PATCH] Reproduce filtering issue.
@ 2026-03-23 11:50 Antonin Houska <[email protected]>
0 siblings, 0 replies; 18+ 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] 18+ messages in thread
* [PATCH] Reproduce filtering issue.
@ 2026-03-23 11:50 Antonin Houska <[email protected]>
0 siblings, 0 replies; 18+ 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] 18+ messages in thread
end of thread, other threads:[~2026-03-23 11:50 UTC | newest]
Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
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 v8 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 v12 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]>
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 v7 4/4] Change policy of XLog read-buffer allocation Kyotaro Horiguchi <[email protected]>
2023-07-28 22:56 [PATCH v6 2/2] allow syncfs in frontend utilities Nathan Bossart <[email protected]>
2023-12-11 20:09 Sorting regression of text function result since commit 586b98fdf1aae Jehan-Guillaume de Rorthais <[email protected]>
2023-12-11 20:43 ` Re: Sorting regression of text function result since commit 586b98fdf1aae Tom Lane <[email protected]>
2023-12-12 10:52 ` Re: Sorting regression of text function result since commit 586b98fdf1aae Jehan-Guillaume de Rorthais <[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