pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #16931: source code problem about commit_ts
12+ messages / 4 participants
[nested] [flat]
* BUG #16931: source code problem about commit_ts
@ 2021-03-18 04:36 PG Bug reporting form <noreply@postgresql.org>
2021-03-18 10:19 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 12+ messages in thread
From: PG Bug reporting form @ 2021-03-18 04:36 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: zoulx1982@163.com
The following bug has been logged on the website:
Bug reference: 16931
Logged by: lx zou
Email address: zoulx1982@163.com
PostgreSQL version: 13.2
Operating system: Linux
Description:
Hi,
recently i am reading commit ts code, and i have a problem about it.
i found commit_ts_redo call TransactionTreeSetCommitTsData with last param
`write_xlog` true,
but RecordTransactionCommitPrepared and RecordTransactionCommit call
TransactionTreeSetCommitTsData
with `write_xlog` false, which cause there are never xlog with commit_ts -
COMMIT_TS_SETTS type.
thanks for your time.
best wishes.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #16931: source code problem about commit_ts
2021-03-18 04:36 BUG #16931: source code problem about commit_ts PG Bug reporting form <noreply@postgresql.org>
@ 2021-03-18 10:19 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-03-24 09:05 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 1 reply; 12+ messages in thread
From: Fujii Masao @ 2021-03-18 10:19 UTC (permalink / raw)
To: zoulx1982@163.com; pgsql-bugs@lists.postgresql.org
On 2021/03/18 13:36, PG Bug reporting form wrote:
> The following bug has been logged on the website:
>
> Bug reference: 16931
> Logged by: lx zou
> Email address: zoulx1982@163.com
> PostgreSQL version: 13.2
> Operating system: Linux
> Description:
>
> Hi,
> recently i am reading commit ts code, and i have a problem about it.
> i found commit_ts_redo call TransactionTreeSetCommitTsData with last param
> `write_xlog` true,
> but RecordTransactionCommitPrepared and RecordTransactionCommit call
> TransactionTreeSetCommitTsData
> with `write_xlog` false, which cause there are never xlog with commit_ts -
> COMMIT_TS_SETTS type.
> thanks for your time.
I guess that TransactionTreeSetCommitTsData(write_xlog=true) is basically
used by some extensions using commit_ts.
IIUC commit_ts_redo() is called during recovery. So it's strange that
commit_ts_redo() calls TransactionTreeSetCommitTsData() with write_xlog=true
because no new WAL can be generated during recovery. Probably this is a bug,
and it should be called with write_xlog=false, instead. Patch attached.
Also one problem is that there is no test for WAL replay of COMMIT_TS_SETTS
for now. Maybe this is why we could not find that bug.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 48e8d66286..268bdba339 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -1074,7 +1074,7 @@ commit_ts_redo(XLogReaderState *record)
subxids = NULL;
TransactionTreeSetCommitTsData(setts->mainxid, nsubxids, subxids,
- setts->timestamp, setts->nodeid, true);
+ setts->timestamp, setts->nodeid, false);
if (subxids)
pfree(subxids);
}
Attachments:
[text/plain] bugfix.patch (519B, ../../d5b98b4b-a3e3-8095-cfb9-aafb5dbe4f7b@oss.nttdata.com/2-bugfix.patch)
download | inline diff:
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 48e8d66286..268bdba339 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -1074,7 +1074,7 @@ commit_ts_redo(XLogReaderState *record)
subxids = NULL;
TransactionTreeSetCommitTsData(setts->mainxid, nsubxids, subxids,
- setts->timestamp, setts->nodeid, true);
+ setts->timestamp, setts->nodeid, false);
if (subxids)
pfree(subxids);
}
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #16931: source code problem about commit_ts
2021-03-18 04:36 BUG #16931: source code problem about commit_ts PG Bug reporting form <noreply@postgresql.org>
2021-03-18 10:19 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-03-24 09:05 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-03-24 10:25 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 12+ messages in thread
From: Alvaro Herrera @ 2021-03-24 09:05 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: zoulx1982@163.com; pgsql-bugs@lists.postgresql.org
On 2021-Mar-18, Fujii Masao wrote:
> On 2021/03/18 13:36, PG Bug reporting form wrote:
> > Hi,
> > recently i am reading commit ts code, and i have a problem about it.
> > i found commit_ts_redo call TransactionTreeSetCommitTsData with last param
> > `write_xlog` true,
> > but RecordTransactionCommitPrepared and RecordTransactionCommit call
> > TransactionTreeSetCommitTsData
> > with `write_xlog` false, which cause there are never xlog with commit_ts -
> > COMMIT_TS_SETTS type.
Hmm, sharp eyes.
> I guess that TransactionTreeSetCommitTsData(write_xlog=true) is basically
> used by some extensions using commit_ts.
I am not aware of any extensions that do that.
> IIUC commit_ts_redo() is called during recovery. So it's strange that
> commit_ts_redo() calls TransactionTreeSetCommitTsData() with write_xlog=true
> because no new WAL can be generated during recovery. Probably this is a bug,
> and it should be called with write_xlog=false, instead. Patch attached.
Yeah, it seems like a bug. With your patch, the write_xlog=true path
becomes unused, and thus the whole COMMIT_TS_SETTS record, so we could
remove those things in branch master. The timestamp is acquired from
the COMMIT record.
> Also one problem is that there is no test for WAL replay of COMMIT_TS_SETTS
> for now. Maybe this is why we could not find that bug.
Yeah, coverage for xlog in commit_ts.c is lacking.
--
Álvaro Herrera 39°49'30"S 73°17'W
"En las profundidades de nuestro inconsciente hay una obsesiva necesidad
de un universo lógico y coherente. Pero el universo real se halla siempre
un paso más allá de la lógica" (Irulan)
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #16931: source code problem about commit_ts
2021-03-18 04:36 BUG #16931: source code problem about commit_ts PG Bug reporting form <noreply@postgresql.org>
2021-03-18 10:19 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-03-24 09:05 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
@ 2021-03-24 10:25 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-03-24 16:39 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-04-09 22:42 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
0 siblings, 2 replies; 12+ messages in thread
From: Fujii Masao @ 2021-03-24 10:25 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: zoulx1982@163.com; pgsql-bugs@lists.postgresql.org
On 2021/03/24 18:05, Alvaro Herrera wrote:
> Yeah, it seems like a bug. With your patch, the write_xlog=true path
> becomes unused, and thus the whole COMMIT_TS_SETTS record, so we could
> remove those things in branch master. The timestamp is acquired from
> the COMMIT record.
I agree to remove COMMIT_TS_SETTS record from the master branch
if there are no users or extensions of it. Patch attached.
Anyway, at first, what about applying the bugfix patch I posted upthread
to all supported branches?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/src/backend/access/rmgrdesc/committsdesc.c b/src/backend/access/rmgrdesc/committsdesc.c
index 7ebd3d35ef..26bad44b96 100644
--- a/src/backend/access/rmgrdesc/committsdesc.c
+++ b/src/backend/access/rmgrdesc/committsdesc.c
@@ -38,31 +38,6 @@ commit_ts_desc(StringInfo buf, XLogReaderState *record)
appendStringInfo(buf, "pageno %d, oldestXid %u",
trunc->pageno, trunc->oldestXid);
}
- else if (info == COMMIT_TS_SETTS)
- {
- xl_commit_ts_set *xlrec = (xl_commit_ts_set *) rec;
- int nsubxids;
-
- appendStringInfo(buf, "set %s/%d for: %u",
- timestamptz_to_str(xlrec->timestamp),
- xlrec->nodeid,
- xlrec->mainxid);
- nsubxids = ((XLogRecGetDataLen(record) - SizeOfCommitTsSet) /
- sizeof(TransactionId));
- if (nsubxids > 0)
- {
- int i;
- TransactionId *subxids;
-
- subxids = palloc(sizeof(TransactionId) * nsubxids);
- memcpy(subxids,
- XLogRecGetData(record) + SizeOfCommitTsSet,
- sizeof(TransactionId) * nsubxids);
- for (i = 0; i < nsubxids; i++)
- appendStringInfo(buf, ", %u", subxids[i]);
- pfree(subxids);
- }
- }
}
const char *
@@ -74,8 +49,6 @@ commit_ts_identify(uint8 info)
return "ZEROPAGE";
case COMMIT_TS_TRUNCATE:
return "TRUNCATE";
- case COMMIT_TS_SETTS:
- return "SETTS";
default:
return NULL;
}
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 268bdba339..0985fa155c 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -114,9 +114,6 @@ static void ActivateCommitTs(void);
static void DeactivateCommitTs(void);
static void WriteZeroPageXlogRec(int pageno);
static void WriteTruncateXlogRec(int pageno, TransactionId oldestXid);
-static void WriteSetTimestampXlogRec(TransactionId mainxid, int nsubxids,
- TransactionId *subxids, TimestampTz timestamp,
- RepOriginId nodeid);
/*
* TransactionTreeSetCommitTsData
@@ -133,18 +130,11 @@ static void WriteSetTimestampXlogRec(TransactionId mainxid, int nsubxids,
* permanent) so we need to keep the information about them here. If the
* subtrans implementation changes in the future, we might want to revisit the
* decision of storing timestamp info for each subxid.
- *
- * The write_xlog parameter tells us whether to include an XLog record of this
- * or not. Normally, this is called from transaction commit routines (both
- * normal and prepared) and the information will be stored in the transaction
- * commit XLog record, and so they should pass "false" for this. The XLog redo
- * code should use "false" here as well. Other callers probably want to pass
- * true, so that the given values persist in case of crashes.
*/
void
TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
TransactionId *subxids, TimestampTz timestamp,
- RepOriginId nodeid, bool write_xlog)
+ RepOriginId nodeid)
{
int i;
TransactionId headxid;
@@ -161,13 +151,6 @@ TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
if (!commitTsShared->commitTsActive)
return;
- /*
- * Comply with the WAL-before-data rule: if caller specified it wants this
- * value to be recorded in WAL, do so before touching the data.
- */
- if (write_xlog)
- WriteSetTimestampXlogRec(xid, nsubxids, subxids, timestamp, nodeid);
-
/*
* Figure out the latest Xid in this batch: either the last subxid if
* there's any, otherwise the parent xid.
@@ -993,28 +976,6 @@ WriteTruncateXlogRec(int pageno, TransactionId oldestXid)
(void) XLogInsert(RM_COMMIT_TS_ID, COMMIT_TS_TRUNCATE);
}
-/*
- * Write a SETTS xlog record
- */
-static void
-WriteSetTimestampXlogRec(TransactionId mainxid, int nsubxids,
- TransactionId *subxids, TimestampTz timestamp,
- RepOriginId nodeid)
-{
- xl_commit_ts_set record;
-
- record.timestamp = timestamp;
- record.nodeid = nodeid;
- record.mainxid = mainxid;
-
- XLogBeginInsert();
- XLogRegisterData((char *) &record,
- offsetof(xl_commit_ts_set, mainxid) +
- sizeof(TransactionId));
- XLogRegisterData((char *) subxids, nsubxids * sizeof(TransactionId));
- XLogInsert(RM_COMMIT_TS_ID, COMMIT_TS_SETTS);
-}
-
/*
* CommitTS resource manager's routines
*/
@@ -1055,29 +1016,6 @@ commit_ts_redo(XLogReaderState *record)
SimpleLruTruncate(CommitTsCtl, trunc->pageno);
}
- else if (info == COMMIT_TS_SETTS)
- {
- xl_commit_ts_set *setts = (xl_commit_ts_set *) XLogRecGetData(record);
- int nsubxids;
- TransactionId *subxids;
-
- nsubxids = ((XLogRecGetDataLen(record) - SizeOfCommitTsSet) /
- sizeof(TransactionId));
- if (nsubxids > 0)
- {
- subxids = palloc(sizeof(TransactionId) * nsubxids);
- memcpy(subxids,
- XLogRecGetData(record) + SizeOfCommitTsSet,
- sizeof(TransactionId) * nsubxids);
- }
- else
- subxids = NULL;
-
- TransactionTreeSetCommitTsData(setts->mainxid, nsubxids, subxids,
- setts->timestamp, setts->nodeid, false);
- if (subxids)
- pfree(subxids);
- }
else
elog(PANIC, "commit_ts_redo: unknown op code %u", info);
}
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 89335b64a2..46f3d08249 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -2244,7 +2244,7 @@ RecordTransactionCommitPrepared(TransactionId xid,
TransactionTreeSetCommitTsData(xid, nchildren, children,
replorigin_session_origin_timestamp,
- replorigin_session_origin, false);
+ replorigin_session_origin);
/*
* We don't currently try to sleep before flush here ... nor is there any
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index c83aa16f2c..441445927e 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1366,7 +1366,7 @@ RecordTransactionCommit(void)
TransactionTreeSetCommitTsData(xid, nchildren, children,
replorigin_session_origin_timestamp,
- replorigin_session_origin, false);
+ replorigin_session_origin);
}
/*
@@ -5804,7 +5804,7 @@ xact_redo_commit(xl_xact_parsed_commit *parsed,
/* Set the transaction commit timestamp and metadata */
TransactionTreeSetCommitTsData(xid, parsed->nsubxacts, parsed->subxacts,
- commit_time, origin_id, false);
+ commit_time, origin_id);
if (standbyState == STANDBY_DISABLED)
{
diff --git a/src/include/access/commit_ts.h b/src/include/access/commit_ts.h
index 750369104a..608a1643cd 100644
--- a/src/include/access/commit_ts.h
+++ b/src/include/access/commit_ts.h
@@ -25,7 +25,7 @@ extern bool check_track_commit_timestamp(bool *newval, void **extra,
extern void TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
TransactionId *subxids, TimestampTz timestamp,
- RepOriginId nodeid, bool write_xlog);
+ RepOriginId nodeid);
extern bool TransactionIdGetCommitTsData(TransactionId xid,
TimestampTz *ts, RepOriginId *nodeid);
extern TransactionId GetLatestCommitTsData(TimestampTz *ts,
@@ -50,7 +50,6 @@ extern int committssyncfiletag(const FileTag *ftag, char *path);
/* XLOG stuff */
#define COMMIT_TS_ZEROPAGE 0x00
#define COMMIT_TS_TRUNCATE 0x10
-#define COMMIT_TS_SETTS 0x20
typedef struct xl_commit_ts_set
{
Attachments:
[text/plain] v1_remove_commit_ts_setts.patch (7.2K, ../../38d5b3c4-3bbf-64be-69f5-5f50cd1f6c2d@oss.nttdata.com/2-v1_remove_commit_ts_setts.patch)
download | inline diff:
diff --git a/src/backend/access/rmgrdesc/committsdesc.c b/src/backend/access/rmgrdesc/committsdesc.c
index 7ebd3d35ef..26bad44b96 100644
--- a/src/backend/access/rmgrdesc/committsdesc.c
+++ b/src/backend/access/rmgrdesc/committsdesc.c
@@ -38,31 +38,6 @@ commit_ts_desc(StringInfo buf, XLogReaderState *record)
appendStringInfo(buf, "pageno %d, oldestXid %u",
trunc->pageno, trunc->oldestXid);
}
- else if (info == COMMIT_TS_SETTS)
- {
- xl_commit_ts_set *xlrec = (xl_commit_ts_set *) rec;
- int nsubxids;
-
- appendStringInfo(buf, "set %s/%d for: %u",
- timestamptz_to_str(xlrec->timestamp),
- xlrec->nodeid,
- xlrec->mainxid);
- nsubxids = ((XLogRecGetDataLen(record) - SizeOfCommitTsSet) /
- sizeof(TransactionId));
- if (nsubxids > 0)
- {
- int i;
- TransactionId *subxids;
-
- subxids = palloc(sizeof(TransactionId) * nsubxids);
- memcpy(subxids,
- XLogRecGetData(record) + SizeOfCommitTsSet,
- sizeof(TransactionId) * nsubxids);
- for (i = 0; i < nsubxids; i++)
- appendStringInfo(buf, ", %u", subxids[i]);
- pfree(subxids);
- }
- }
}
const char *
@@ -74,8 +49,6 @@ commit_ts_identify(uint8 info)
return "ZEROPAGE";
case COMMIT_TS_TRUNCATE:
return "TRUNCATE";
- case COMMIT_TS_SETTS:
- return "SETTS";
default:
return NULL;
}
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 268bdba339..0985fa155c 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -114,9 +114,6 @@ static void ActivateCommitTs(void);
static void DeactivateCommitTs(void);
static void WriteZeroPageXlogRec(int pageno);
static void WriteTruncateXlogRec(int pageno, TransactionId oldestXid);
-static void WriteSetTimestampXlogRec(TransactionId mainxid, int nsubxids,
- TransactionId *subxids, TimestampTz timestamp,
- RepOriginId nodeid);
/*
* TransactionTreeSetCommitTsData
@@ -133,18 +130,11 @@ static void WriteSetTimestampXlogRec(TransactionId mainxid, int nsubxids,
* permanent) so we need to keep the information about them here. If the
* subtrans implementation changes in the future, we might want to revisit the
* decision of storing timestamp info for each subxid.
- *
- * The write_xlog parameter tells us whether to include an XLog record of this
- * or not. Normally, this is called from transaction commit routines (both
- * normal and prepared) and the information will be stored in the transaction
- * commit XLog record, and so they should pass "false" for this. The XLog redo
- * code should use "false" here as well. Other callers probably want to pass
- * true, so that the given values persist in case of crashes.
*/
void
TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
TransactionId *subxids, TimestampTz timestamp,
- RepOriginId nodeid, bool write_xlog)
+ RepOriginId nodeid)
{
int i;
TransactionId headxid;
@@ -161,13 +151,6 @@ TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
if (!commitTsShared->commitTsActive)
return;
- /*
- * Comply with the WAL-before-data rule: if caller specified it wants this
- * value to be recorded in WAL, do so before touching the data.
- */
- if (write_xlog)
- WriteSetTimestampXlogRec(xid, nsubxids, subxids, timestamp, nodeid);
-
/*
* Figure out the latest Xid in this batch: either the last subxid if
* there's any, otherwise the parent xid.
@@ -993,28 +976,6 @@ WriteTruncateXlogRec(int pageno, TransactionId oldestXid)
(void) XLogInsert(RM_COMMIT_TS_ID, COMMIT_TS_TRUNCATE);
}
-/*
- * Write a SETTS xlog record
- */
-static void
-WriteSetTimestampXlogRec(TransactionId mainxid, int nsubxids,
- TransactionId *subxids, TimestampTz timestamp,
- RepOriginId nodeid)
-{
- xl_commit_ts_set record;
-
- record.timestamp = timestamp;
- record.nodeid = nodeid;
- record.mainxid = mainxid;
-
- XLogBeginInsert();
- XLogRegisterData((char *) &record,
- offsetof(xl_commit_ts_set, mainxid) +
- sizeof(TransactionId));
- XLogRegisterData((char *) subxids, nsubxids * sizeof(TransactionId));
- XLogInsert(RM_COMMIT_TS_ID, COMMIT_TS_SETTS);
-}
-
/*
* CommitTS resource manager's routines
*/
@@ -1055,29 +1016,6 @@ commit_ts_redo(XLogReaderState *record)
SimpleLruTruncate(CommitTsCtl, trunc->pageno);
}
- else if (info == COMMIT_TS_SETTS)
- {
- xl_commit_ts_set *setts = (xl_commit_ts_set *) XLogRecGetData(record);
- int nsubxids;
- TransactionId *subxids;
-
- nsubxids = ((XLogRecGetDataLen(record) - SizeOfCommitTsSet) /
- sizeof(TransactionId));
- if (nsubxids > 0)
- {
- subxids = palloc(sizeof(TransactionId) * nsubxids);
- memcpy(subxids,
- XLogRecGetData(record) + SizeOfCommitTsSet,
- sizeof(TransactionId) * nsubxids);
- }
- else
- subxids = NULL;
-
- TransactionTreeSetCommitTsData(setts->mainxid, nsubxids, subxids,
- setts->timestamp, setts->nodeid, false);
- if (subxids)
- pfree(subxids);
- }
else
elog(PANIC, "commit_ts_redo: unknown op code %u", info);
}
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 89335b64a2..46f3d08249 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -2244,7 +2244,7 @@ RecordTransactionCommitPrepared(TransactionId xid,
TransactionTreeSetCommitTsData(xid, nchildren, children,
replorigin_session_origin_timestamp,
- replorigin_session_origin, false);
+ replorigin_session_origin);
/*
* We don't currently try to sleep before flush here ... nor is there any
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index c83aa16f2c..441445927e 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1366,7 +1366,7 @@ RecordTransactionCommit(void)
TransactionTreeSetCommitTsData(xid, nchildren, children,
replorigin_session_origin_timestamp,
- replorigin_session_origin, false);
+ replorigin_session_origin);
}
/*
@@ -5804,7 +5804,7 @@ xact_redo_commit(xl_xact_parsed_commit *parsed,
/* Set the transaction commit timestamp and metadata */
TransactionTreeSetCommitTsData(xid, parsed->nsubxacts, parsed->subxacts,
- commit_time, origin_id, false);
+ commit_time, origin_id);
if (standbyState == STANDBY_DISABLED)
{
diff --git a/src/include/access/commit_ts.h b/src/include/access/commit_ts.h
index 750369104a..608a1643cd 100644
--- a/src/include/access/commit_ts.h
+++ b/src/include/access/commit_ts.h
@@ -25,7 +25,7 @@ extern bool check_track_commit_timestamp(bool *newval, void **extra,
extern void TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
TransactionId *subxids, TimestampTz timestamp,
- RepOriginId nodeid, bool write_xlog);
+ RepOriginId nodeid);
extern bool TransactionIdGetCommitTsData(TransactionId xid,
TimestampTz *ts, RepOriginId *nodeid);
extern TransactionId GetLatestCommitTsData(TimestampTz *ts,
@@ -50,7 +50,6 @@ extern int committssyncfiletag(const FileTag *ftag, char *path);
/* XLOG stuff */
#define COMMIT_TS_ZEROPAGE 0x00
#define COMMIT_TS_TRUNCATE 0x10
-#define COMMIT_TS_SETTS 0x20
typedef struct xl_commit_ts_set
{
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #16931: source code problem about commit_ts
2021-03-18 04:36 BUG #16931: source code problem about commit_ts PG Bug reporting form <noreply@postgresql.org>
2021-03-18 10:19 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-03-24 09:05 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-03-24 10:25 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-03-24 16:39 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-03-25 02:59 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
1 sibling, 1 reply; 12+ messages in thread
From: Alvaro Herrera @ 2021-03-24 16:39 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: zoulx1982@163.com; pgsql-bugs@lists.postgresql.org
On 2021-Mar-24, Fujii Masao wrote:
> On 2021/03/24 18:05, Alvaro Herrera wrote:
> > Yeah, it seems like a bug. With your patch, the write_xlog=true path
> > becomes unused, and thus the whole COMMIT_TS_SETTS record, so we could
> > remove those things in branch master. The timestamp is acquired from
> > the COMMIT record.
>
> I agree to remove COMMIT_TS_SETTS record from the master branch
> if there are no users or extensions of it. Patch attached.
Looks good in a quick skim. Let's get this pushed quickly; if anything
exists out there that is calling that code, it'd be good to know sooner
rather than later. However, my feeling is that no such caller exists,
because they would have told us about this problem already.
Are you bumping WAL page magic? It'd be logical to do so, but on the
other hand if no code exists that is capable of emitting that record,
then it'd be pointless.
> Anyway, at first, what about applying the bugfix patch I posted upthread
> to all supported branches?
Yeah, let's do that.
--
Álvaro Herrera 39°49'30"S 73°17'W
"XML!" Exclaimed C++. "What are you doing here? You're not a programming
language."
"Tell that to the people who use me," said XML.
https://burningbird.net/the-parable-of-the-languages/
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #16931: source code problem about commit_ts
2021-03-18 04:36 BUG #16931: source code problem about commit_ts PG Bug reporting form <noreply@postgresql.org>
2021-03-18 10:19 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-03-24 09:05 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-03-24 10:25 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-03-24 16:39 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
@ 2021-03-25 02:59 ` Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 0 replies; 12+ messages in thread
From: Fujii Masao @ 2021-03-25 02:59 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: zoulx1982@163.com; pgsql-bugs@lists.postgresql.org
On 2021/03/25 1:39, Alvaro Herrera wrote:
> On 2021-Mar-24, Fujii Masao wrote:
>
>> On 2021/03/24 18:05, Alvaro Herrera wrote:
>>> Yeah, it seems like a bug. With your patch, the write_xlog=true path
>>> becomes unused, and thus the whole COMMIT_TS_SETTS record, so we could
>>> remove those things in branch master. The timestamp is acquired from
>>> the COMMIT record.
>>
>> I agree to remove COMMIT_TS_SETTS record from the master branch
>> if there are no users or extensions of it. Patch attached.
>
> Looks good in a quick skim. Let's get this pushed quickly; if anything
> exists out there that is calling that code, it'd be good to know sooner
> rather than later. However, my feeling is that no such caller exists,
> because they would have told us about this problem already.
Agreed. I will commit the patch in the master.
> Are you bumping WAL page magic? It'd be logical to do so, but on the
> other hand if no code exists that is capable of emitting that record,
> then it'd be pointless.
Yes, I'm thinking to bump WAL page magic because it seems a bit safer.
>> Anyway, at first, what about applying the bugfix patch I posted upthread
>> to all supported branches?
>
> Yeah, let's do that.
Pushed. Thanks!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #16931: source code problem about commit_ts
2021-03-18 04:36 BUG #16931: source code problem about commit_ts PG Bug reporting form <noreply@postgresql.org>
2021-03-18 10:19 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-03-24 09:05 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-03-24 10:25 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2021-04-09 22:42 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-04-11 15:09 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
1 sibling, 1 reply; 12+ messages in thread
From: Alvaro Herrera @ 2021-04-09 22:42 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: zoulx1982@163.com; pgsql-bugs@lists.postgresql.org
On 2021-Mar-24, Fujii Masao wrote:
> diff --git a/src/backend/access/rmgrdesc/committsdesc.c b/src/backend/access/rmgrdesc/committsdesc.c
> index 7ebd3d35ef..26bad44b96 100644
> --- a/src/backend/access/rmgrdesc/committsdesc.c
> +++ b/src/backend/access/rmgrdesc/committsdesc.c
> @@ -38,31 +38,6 @@ commit_ts_desc(StringInfo buf, XLogReaderState *record)
> appendStringInfo(buf, "pageno %d, oldestXid %u",
> trunc->pageno, trunc->oldestXid);
> }
> - else if (info == COMMIT_TS_SETTS)
You have not pushed this one, right? I think we should do it now.
Thanks
--
Álvaro Herrera 39°49'30"S 73°17'W
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #16931: source code problem about commit_ts
2021-03-18 04:36 BUG #16931: source code problem about commit_ts PG Bug reporting form <noreply@postgresql.org>
2021-03-18 10:19 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-03-24 09:05 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-03-24 10:25 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-04-09 22:42 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
@ 2021-04-11 15:09 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2025-07-02 01:42 ` Re: BUG #16931: source code problem about commit_ts Andy Fan <zhihuifan1213@163.com>
0 siblings, 1 reply; 12+ messages in thread
From: Fujii Masao @ 2021-04-11 15:09 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@alvh.no-ip.org>; +Cc: zoulx1982@163.com; pgsql-bugs@lists.postgresql.org
On 2021/04/10 7:42, Alvaro Herrera wrote:
> On 2021-Mar-24, Fujii Masao wrote:
>
>> diff --git a/src/backend/access/rmgrdesc/committsdesc.c b/src/backend/access/rmgrdesc/committsdesc.c
>> index 7ebd3d35ef..26bad44b96 100644
>> --- a/src/backend/access/rmgrdesc/committsdesc.c
>> +++ b/src/backend/access/rmgrdesc/committsdesc.c
>> @@ -38,31 +38,6 @@ commit_ts_desc(StringInfo buf, XLogReaderState *record)
>> appendStringInfo(buf, "pageno %d, oldestXid %u",
>> trunc->pageno, trunc->oldestXid);
>> }
>> - else if (info == COMMIT_TS_SETTS)
>
> You have not pushed this one, right? I think we should do it now.
Thanks for the ping! Pushed!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #16931: source code problem about commit_ts
2021-03-18 04:36 BUG #16931: source code problem about commit_ts PG Bug reporting form <noreply@postgresql.org>
2021-03-18 10:19 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-03-24 09:05 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-03-24 10:25 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-04-09 22:42 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-04-11 15:09 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2025-07-02 01:42 ` Andy Fan <zhihuifan1213@163.com>
2025-07-02 08:44 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 12+ messages in thread
From: Andy Fan @ 2025-07-02 01:42 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; zoulx1982@163.com; pgsql-bugs@lists.postgresql.org
Fujii Masao <masao.fujii@oss.nttdata.com> writes:
Hi,
> On 2021/04/10 7:42, Alvaro Herrera wrote:
>> On 2021-Mar-24, Fujii Masao wrote:
>>
>>> diff --git a/src/backend/access/rmgrdesc/committsdesc.c b/src/backend/access/rmgrdesc/committsdesc.c
>>> index 7ebd3d35ef..26bad44b96 100644
>>> --- a/src/backend/access/rmgrdesc/committsdesc.c
>>> +++ b/src/backend/access/rmgrdesc/committsdesc.c
>>> @@ -38,31 +38,6 @@ commit_ts_desc(StringInfo buf, XLogReaderState *record)
>>> appendStringInfo(buf, "pageno %d, oldestXid %u",
>>> trunc->pageno, trunc->oldestXid);
>>> }
>>> - else if (info == COMMIT_TS_SETTS)
>> You have not pushed this one, right? I think we should do it now.
>
> Thanks for the ping! Pushed!
Did this commit(08aa89b326261b669648df97d4f2a6edba22d26a) forget to
remove struct xl_commit_ts_set? After it, there is no reference to
xl_commit_ts_set. So Is it better clean them as well?
typedef struct xl_commit_ts_set
{
TimestampTz timestamp;
RepOriginId nodeid;
TransactionId mainxid;
/* subxact Xids follow */
} xl_commit_ts_set;
#define SizeOfCommitTsSet (offsetof(xl_commit_ts_set, mainxid) + \
(When going through commit_ts.h, these code makes me think there is a
dedicated xlog for commit_ts, but the fact isn't, the commit_ts is
recorded in xl_xact_commit, removing dead code could be useful for such
case).
--
Best Regards
Andy Fan
Attachments:
[text/x-diff] v1-0001-Remove-xl_commit_ts_set.patch (0B, ../../87ecuzmkqf.fsf@163.com/2-v1-0001-Remove-xl_commit_ts_set.patch)
download
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #16931: source code problem about commit_ts
2021-03-18 04:36 BUG #16931: source code problem about commit_ts PG Bug reporting form <noreply@postgresql.org>
2021-03-18 10:19 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-03-24 09:05 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-03-24 10:25 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-04-09 22:42 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-04-11 15:09 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2025-07-02 01:42 ` Re: BUG #16931: source code problem about commit_ts Andy Fan <zhihuifan1213@163.com>
@ 2025-07-02 08:44 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2025-07-03 13:33 ` Re: BUG #16931: source code problem about commit_ts Andy Fan <zhihuifan1213@163.com>
0 siblings, 1 reply; 12+ messages in thread
From: Fujii Masao @ 2025-07-02 08:44 UTC (permalink / raw)
To: Andy Fan <zhihuifan1213@163.com>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; zoulx1982@163.com; pgsql-bugs@lists.postgresql.org
On 2025/07/02 10:42, Andy Fan wrote:
> Fujii Masao <masao.fujii@oss.nttdata.com> writes:
>
> Hi,
>
>> On 2021/04/10 7:42, Alvaro Herrera wrote:
>>> On 2021-Mar-24, Fujii Masao wrote:
>>>
>>>> diff --git a/src/backend/access/rmgrdesc/committsdesc.c b/src/backend/access/rmgrdesc/committsdesc.c
>>>> index 7ebd3d35ef..26bad44b96 100644
>>>> --- a/src/backend/access/rmgrdesc/committsdesc.c
>>>> +++ b/src/backend/access/rmgrdesc/committsdesc.c
>>>> @@ -38,31 +38,6 @@ commit_ts_desc(StringInfo buf, XLogReaderState *record)
>>>> appendStringInfo(buf, "pageno %d, oldestXid %u",
>>>> trunc->pageno, trunc->oldestXid);
>>>> }
>>>> - else if (info == COMMIT_TS_SETTS)
>>> You have not pushed this one, right? I think we should do it now.
>>
>> Thanks for the ping! Pushed!
>
> Did this commit(08aa89b326261b669648df97d4f2a6edba22d26a) forget to
> remove struct xl_commit_ts_set?
You're right. I seem to have overlooked that.
> After it, there is no reference to
> xl_commit_ts_set. So Is it better clean them as well?
Agreed, the patch looks good to me. Unless there are objections,
I'll go ahead and commit it. While this could be considered an oversight
in the original commit, it's not a bug fix, so I plan to apply it
only to the master branch.
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #16931: source code problem about commit_ts
2021-03-18 04:36 BUG #16931: source code problem about commit_ts PG Bug reporting form <noreply@postgresql.org>
2021-03-18 10:19 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-03-24 09:05 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-03-24 10:25 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-04-09 22:42 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-04-11 15:09 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2025-07-02 01:42 ` Re: BUG #16931: source code problem about commit_ts Andy Fan <zhihuifan1213@163.com>
2025-07-02 08:44 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
@ 2025-07-03 13:33 ` Andy Fan <zhihuifan1213@163.com>
2025-07-03 14:42 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 1 reply; 12+ messages in thread
From: Andy Fan @ 2025-07-03 13:33 UTC (permalink / raw)
To: Fujii Masao <masao.fujii@oss.nttdata.com>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; zoulx1982@163.com; pgsql-bugs@lists.postgresql.org
Fujii Masao <masao.fujii@oss.nttdata.com> writes:
>>>
>>> Thanks for the ping! Pushed!
>> Did this commit(08aa89b326261b669648df97d4f2a6edba22d26a) forget to
>> remove struct xl_commit_ts_set?
>
> You're right. I seem to have overlooked that.
>
>> After it, there is no reference to
>> xl_commit_ts_set. So Is it better clean them as well?
>
> Agreed, the patch looks good to me. Unless there are objections,
> I'll go ahead and commit it.
Thank you Fujii!
> While this could be considered an oversight in the original commit,
> it's not a bug fix, so I plan to apply it only to the master branch.
That makes sense to me.
--
Best Regards
Andy Fan
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: BUG #16931: source code problem about commit_ts
2021-03-18 04:36 BUG #16931: source code problem about commit_ts PG Bug reporting form <noreply@postgresql.org>
2021-03-18 10:19 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-03-24 09:05 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-03-24 10:25 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2021-04-09 22:42 ` Re: BUG #16931: source code problem about commit_ts Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-04-11 15:09 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2025-07-02 01:42 ` Re: BUG #16931: source code problem about commit_ts Andy Fan <zhihuifan1213@163.com>
2025-07-02 08:44 ` Re: BUG #16931: source code problem about commit_ts Fujii Masao <masao.fujii@oss.nttdata.com>
2025-07-03 13:33 ` Re: BUG #16931: source code problem about commit_ts Andy Fan <zhihuifan1213@163.com>
@ 2025-07-03 14:42 ` Fujii Masao <masao.fujii@oss.nttdata.com>
0 siblings, 0 replies; 12+ messages in thread
From: Fujii Masao @ 2025-07-03 14:42 UTC (permalink / raw)
To: Andy Fan <zhihuifan1213@163.com>; +Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>; zoulx1982@163.com; pgsql-bugs@lists.postgresql.org
On 2025/07/03 22:33, Andy Fan wrote:
> Fujii Masao <masao.fujii@oss.nttdata.com> writes:
>
>>>>
>>>> Thanks for the ping! Pushed!
>>> Did this commit(08aa89b326261b669648df97d4f2a6edba22d26a) forget to
>>> remove struct xl_commit_ts_set?
>>
>> You're right. I seem to have overlooked that.
>>
>>> After it, there is no reference to
>>> xl_commit_ts_set. So Is it better clean them as well?
>>
>> Agreed, the patch looks good to me. Unless there are objections,
>> I'll go ahead and commit it.
>
> Thank you Fujii!
>
>> While this could be considered an oversight in the original commit,
>> it's not a bug fix, so I plan to apply it only to the master branch.
>
> That makes sense to me.
I've pushed the patch. Thanks!
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2025-07-03 14:42 UTC | newest]
Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-18 04:36 BUG #16931: source code problem about commit_ts PG Bug reporting form <noreply@postgresql.org>
2021-03-18 10:19 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-03-24 09:05 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-03-24 10:25 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-03-24 16:39 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-03-25 02:59 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2021-04-09 22:42 ` Alvaro Herrera <alvherre@alvh.no-ip.org>
2021-04-11 15:09 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2025-07-02 01:42 ` Andy Fan <zhihuifan1213@163.com>
2025-07-02 08:44 ` Fujii Masao <masao.fujii@oss.nttdata.com>
2025-07-03 13:33 ` Andy Fan <zhihuifan1213@163.com>
2025-07-03 14:42 ` Fujii Masao <masao.fujii@oss.nttdata.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox