Timeline ID hexadecimal format
Hi,
I've been puzzled by this message:
~~~
LOG: fetching timeline history file for timeline 17 from primary server
FATAL: could not receive timeline history file from the primary server:
ERROR: could not open file "pg_xlog/00000011.history": No such file or
directory
~~~
It took me a while to understand that the timeline id 11 in hexadecimal
is the same as the timeline id 17 in decimal.
It appears that the first message is formatted with %u instead of %X,
and there some others places with the some format, while WAL filename
and history file used hexadecimal.
There is another place where timeline id is used : pg_waldump, and in
these tools, timeline id ( -t or --timeline ) should be given in
decimal, while filename gives it in hexadecimal : imho, it's not
user-friendly, and can lead to user's bad input for timeline id.
The attached patch proposes to change the format of timelineid from %u
to %X.
Regarding .po files, I don't know how to manage them. Is there any
routine to spread the modifications? Or should I identify and change
each message?
best regards,
--
Sébastien
Attachments:
v1_0001_timelineid_hexadecimal_format.patchtext/x-patch; charset=UTF-8; name=v1_0001_timelineid_hexadecimal_format.patchDownload
diff --git a/src/backend/access/rmgrdesc/xlogdesc.c b/src/backend/access/rmgrdesc/xlogdesc.c
index f390c177e4..77b0fbc962 100644
--- a/src/backend/access/rmgrdesc/xlogdesc.c
+++ b/src/backend/access/rmgrdesc/xlogdesc.c
@@ -45,7 +45,7 @@ xlog_desc(StringInfo buf, XLogReaderState *record)
CheckPoint *checkpoint = (CheckPoint *) rec;
appendStringInfo(buf, "redo %X/%X; "
- "tli %u; prev tli %u; fpw %s; xid %u:%u; oid %u; multi %u; offset %u; "
+ "tli %X; prev tli %X; fpw %s; xid %u:%u; oid %u; multi %u; offset %u; "
"oldest xid %u in DB %u; oldest multi %u in DB %u; "
"oldest/newest commit timestamp xid: %u/%u; "
"oldest running xid %u; %s",
@@ -135,7 +135,7 @@ xlog_desc(StringInfo buf, XLogReaderState *record)
xl_end_of_recovery xlrec;
memcpy(&xlrec, rec, sizeof(xl_end_of_recovery));
- appendStringInfo(buf, "tli %u; prev tli %u; time %s",
+ appendStringInfo(buf, "tli %X; prev tli %X; time %s",
xlrec.ThisTimeLineID, xlrec.PrevTimeLineID,
timestamptz_to_str(xlrec.end_time));
}
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index aa6c929477..97ff6e80b6 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1329,7 +1329,7 @@ XLogReaderValidatePageHeader(XLogReaderState *state, XLogRecPtr recptr,
XLogFileName(fname, state->seg.ws_tli, segno, state->segcxt.ws_segsize);
report_invalid_record(state,
- "out-of-sequence timeline ID %u (after %u) in WAL segment %s, LSN %X/%X, offset %u",
+ "out-of-sequence timeline ID %X (after %X) in WAL segment %s, LSN %X/%X, offset %u",
hdr->xlp_tli,
state->latestPageTLI,
fname,
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 2a5352f879..c814ef2767 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -3080,7 +3080,7 @@ ReadRecord(XLogPrefetcher *xlogprefetcher, int emode,
XLogFileName(fname, xlogreader->seg.ws_tli, segno,
wal_segment_size);
ereport(emode_for_corrupt_record(emode, xlogreader->EndRecPtr),
- (errmsg("unexpected timeline ID %u in WAL segment %s, LSN %X/%X, offset %u",
+ (errmsg("unexpected timeline ID %X in WAL segment %s, LSN %X/%X, offset %u",
xlogreader->latestPageTLI,
fname,
LSN_FORMAT_ARGS(xlogreader->latestPagePtr),
@@ -3719,7 +3719,7 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
tli = tliOfPointInHistory(tliRecPtr, expectedTLEs);
if (curFileTLI > 0 && tli < curFileTLI)
- elog(ERROR, "according to history file, WAL location %X/%X belongs to timeline %u, but previous recovered WAL file came from timeline %u",
+ elog(ERROR, "according to history file, WAL location %X/%X belongs to timeline %X, but previous recovered WAL file came from timeline %X",
LSN_FORMAT_ARGS(tliRecPtr),
tli, curFileTLI);
}
@@ -4019,7 +4019,7 @@ rescanLatestTimeLine(TimeLineID replayTLI, XLogRecPtr replayLSN)
if (!found)
{
ereport(LOG,
- (errmsg("new timeline %u is not a child of database system timeline %u",
+ (errmsg("new timeline %X is not a child of database system timeline %X",
newtarget,
replayTLI)));
return false;
@@ -4033,7 +4033,7 @@ rescanLatestTimeLine(TimeLineID replayTLI, XLogRecPtr replayLSN)
if (currentTle->end < replayLSN)
{
ereport(LOG,
- (errmsg("new timeline %u forked off current database system timeline %u before current recovery point %X/%X",
+ (errmsg("new timeline %X forked off current database system timeline %X before current recovery point %X/%X",
newtarget,
replayTLI,
LSN_FORMAT_ARGS(replayLSN))));
@@ -4052,7 +4052,7 @@ rescanLatestTimeLine(TimeLineID replayTLI, XLogRecPtr replayLSN)
restoreTimeLineHistoryFiles(oldtarget + 1, newtarget);
ereport(LOG,
- (errmsg("new target timeline is %u",
+ (errmsg("new target timeline is %X",
recoveryTargetTLI)));
return true;
diff --git a/src/backend/backup/backup_manifest.c b/src/backend/backup/backup_manifest.c
index fabd2ca299..1ea3facb67 100644
--- a/src/backend/backup/backup_manifest.c
+++ b/src/backend/backup/backup_manifest.c
@@ -250,7 +250,7 @@ AddWALInfoToBackupManifest(backup_manifest_info *manifest, XLogRecPtr startptr,
*/
if (first_wal_range && endtli != entry->tli)
ereport(ERROR,
- errmsg("expected end timeline %u but found timeline %u",
+ errmsg("expected end timeline %X but found timeline %X",
starttli, entry->tli));
/*
@@ -274,12 +274,12 @@ AddWALInfoToBackupManifest(backup_manifest_info *manifest, XLogRecPtr startptr,
*/
if (XLogRecPtrIsInvalid(entry->begin))
ereport(ERROR,
- errmsg("expected start timeline %u but found timeline %u",
+ errmsg("expected start timeline %X but found timeline %X",
starttli, entry->tli));
}
AppendToManifest(manifest,
- "%s{ \"Timeline\": %u, \"Start-LSN\": \"%X/%X\", \"End-LSN\": \"%X/%X\" }",
+ "%s{ \"Timeline\": %X, \"Start-LSN\": \"%X/%X\", \"End-LSN\": \"%X/%X\" }",
first_wal_range ? "" : ",\n",
entry->tli,
LSN_FORMAT_ARGS(tl_beginptr),
@@ -301,7 +301,7 @@ AddWALInfoToBackupManifest(backup_manifest_info *manifest, XLogRecPtr startptr,
*/
if (!found_start_timeline)
ereport(ERROR,
- errmsg("start timeline %u not found in history of timeline %u",
+ errmsg("start timeline %X not found in history of timeline %X",
starttli, endtli));
/* Terminate the list of WAL ranges. */
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 560ec974fa..7c310c8ae2 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -564,7 +564,7 @@ libpqrcv_readtimelinehistoryfile(WalReceiverConn *conn,
/*
* Request the primary to send over the history file for given timeline.
*/
- snprintf(cmd, sizeof(cmd), "TIMELINE_HISTORY %u", tli);
+ snprintf(cmd, sizeof(cmd), "TIMELINE_HISTORY %X", tli);
res = libpqrcv_PQexec(conn->streamConn, cmd);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index b0cfddd548..a9702e5679 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -361,7 +361,7 @@ WalReceiverMain(void)
if (primaryTLI < startpointTLI)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("highest timeline %u of the primary is behind recovery timeline %u",
+ errmsg("highest timeline %X of the primary is behind recovery timeline %X",
primaryTLI, startpointTLI)));
/*
@@ -414,11 +414,11 @@ WalReceiverMain(void)
{
if (first_stream)
ereport(LOG,
- (errmsg("started streaming WAL from primary at %X/%X on timeline %u",
+ (errmsg("started streaming WAL from primary at %X/%X on timeline %X",
LSN_FORMAT_ARGS(startpoint), startpointTLI)));
else
ereport(LOG,
- (errmsg("restarted WAL streaming at %X/%X on timeline %u",
+ (errmsg("restarted WAL streaming at %X/%X on timeline %X",
LSN_FORMAT_ARGS(startpoint), startpointTLI)));
first_stream = false;
@@ -499,7 +499,7 @@ WalReceiverMain(void)
{
ereport(LOG,
(errmsg("replication terminated by primary server"),
- errdetail("End of WAL reached on timeline %u at %X/%X.",
+ errdetail("End of WAL reached on timeline %X at %X/%X.",
startpointTLI,
LSN_FORMAT_ARGS(LogstreamResult.Write))));
endofwal = true;
@@ -621,7 +621,7 @@ WalReceiverMain(void)
}
else
ereport(LOG,
- (errmsg("primary server contains no more WAL on requested timeline %u",
+ (errmsg("primary server contains no more WAL on requested timeline %X",
startpointTLI)));
/*
@@ -756,7 +756,7 @@ WalRcvFetchTimeLineHistoryFiles(TimeLineID first, TimeLineID last)
char expectedfname[MAXFNAMELEN];
ereport(LOG,
- (errmsg("fetching timeline history file for timeline %u from primary server",
+ (errmsg("fetching timeline history file for timeline %X from primary server",
tli)));
walrcv_readtimelinehistoryfile(wrconn, tli, &fname, &content, &len);
@@ -770,7 +770,7 @@ WalRcvFetchTimeLineHistoryFiles(TimeLineID first, TimeLineID last)
if (strcmp(fname, expectedfname) != 0)
ereport(ERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
- errmsg_internal("primary reported unexpected file name for timeline history file of timeline %u",
+ errmsg_internal("primary reported unexpected file name for timeline history file of timeline %X",
tli)));
/*
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 858d8d9f2f..d32dabe966 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -856,7 +856,7 @@ getTimelineHistory(ControlFileData *controlFile, int *nentries)
TimeLineHistoryEntry *entry;
entry = &history[i];
- pg_log_debug("%u: %X/%X - %X/%X", entry->tli,
+ pg_log_debug("%X: %X/%X - %X/%X", entry->tli,
LSN_FORMAT_ARGS(entry->begin),
LSN_FORMAT_ARGS(entry->end));
}
diff --git a/src/bin/pg_rewind/timeline.c b/src/bin/pg_rewind/timeline.c
index 2d445dac32..249f058255 100644
--- a/src/bin/pg_rewind/timeline.c
+++ b/src/bin/pg_rewind/timeline.c
@@ -67,7 +67,7 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries)
if (*ptr == '\0' || *ptr == '#')
continue;
- nfields = sscanf(fline, "%u\t%X/%X", &tli, &switchpoint_hi, &switchpoint_lo);
+ nfields = sscanf(fline, "%X\t%X/%X", &tli, &switchpoint_hi, &switchpoint_lo);
if (nfields < 1)
{
diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c
index 7634dfc285..2a54d7ab52 100644
--- a/src/bin/pg_verifybackup/pg_verifybackup.c
+++ b/src/bin/pg_verifybackup/pg_verifybackup.c
@@ -807,14 +807,14 @@ parse_required_wal(verifier_context *context, char *pg_waldump_path,
{
char *pg_waldump_cmd;
- pg_waldump_cmd = psprintf("\"%s\" --quiet --path=\"%s\" --timeline=%u --start=%X/%X --end=%X/%X\n",
+ pg_waldump_cmd = psprintf("\"%s\" --quiet --path=\"%s\" --timeline=%X --start=%X/%X --end=%X/%X\n",
pg_waldump_path, wal_directory, this_wal_range->tli,
LSN_FORMAT_ARGS(this_wal_range->start_lsn),
LSN_FORMAT_ARGS(this_wal_range->end_lsn));
fflush(NULL);
if (system(pg_waldump_cmd) != 0)
report_backup_error(context,
- "WAL parsing failed for timeline %u",
+ "WAL parsing failed for timeline %X",
this_wal_range->tli);
this_wal_range = this_wal_range->next;
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 44b5c8726e..23dc7354ba 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1007,7 +1007,7 @@ main(int argc, char **argv)
private.startptr = (uint64) xlogid << 32 | xrecoff;
break;
case 't':
- if (sscanf(optarg, "%u", &private.timeline) != 1)
+ if (sscanf(optarg, "%X", &private.timeline) != 1)
{
pg_log_error("invalid timeline specification: \"%s\"", optarg);
goto bad_argument;
On 27.01.23 14:52, Sébastien Lardière wrote:
The attached patch proposes to change the format of timelineid from %u
to %X.
I think your complaint has merit. But note that if we did a change like
this, then log files or reports from different versions would have
different meaning without a visual difference, which is kind of what you
complained about in the first place. At least we should do something
like 0x%X.
Regarding .po files, I don't know how to manage them. Is there any
routine to spread the modifications? Or should I identify and change
each message?
Don't worry about this. This is handled elsewhere.
On 27/01/2023 15:55, Peter Eisentraut wrote:
On 27.01.23 14:52, Sébastien Lardière wrote:
The attached patch proposes to change the format of timelineid from
%u to %X.I think your complaint has merit. But note that if we did a change
like this, then log files or reports from different versions would
have different meaning without a visual difference, which is kind of
what you complained about in the first place. At least we should do
something like 0x%X.
Indeed, but the messages that puzzled was in one log file, just
together, not in some differents versions.
But yes, it should be documented somewhere, actually, I can't find any
good place for that,
While digging, It seems that recovery_target_timeline should be given in
decimal, not in hexadecimal, which seems odd to me ; and pg_controldata
use decimal too, not hexadecimal…
So, if this idea is correct, the given patch is not enough.
Anyway, do you think it is a good idea or not ?
Regarding .po files, I don't know how to manage them. Is there any
routine to spread the modifications? Or should I identify and change
each message?Don't worry about this. This is handled elsewhere.
nice,
regards,
--
Sébastien
On 27/01/2023 15:55, Peter Eisentraut wrote:
On 27.01.23 14:52, Sébastien Lardière wrote:
The attached patch proposes to change the format of timelineid from
%u to %X.I think your complaint has merit. But note that if we did a change
like this, then log files or reports from different versions would
have different meaning without a visual difference, which is kind of
what you complained about in the first place. At least we should do
something like 0x%X.
Hi,
Here's the patch with the suggested format ; plus, I add some note in
the documentation about recovery_target_timeline, because I don't get
how strtoul(), with the special 0 base parameter can work without 0x
prefix ; I suppose that nobody use it.
I also change pg_controldata and the usage of this output by pg_upgrade.
I let internal usages unchanded : content of backup manifest and content
of history file.
Should I open a commitfest entry, or is it too soon ?
regards,
--
Sébastien
Attachments:
v2_0001_timelineid_hexadecimal_format.patchtext/x-patch; charset=UTF-8; name=v2_0001_timelineid_hexadecimal_format.patchDownload
[1mdiff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml[m
[1mindex be05a33205..7e26b51031 100644[m
[1m--- a/doc/src/sgml/backup.sgml[m
[1m+++ b/doc/src/sgml/backup.sgml[m
[36m@@ -1332,7 +1332,8 @@[m [mrestore_command = 'cp /mnt/server/archivedir/%f %p'[m
you like, add comments to a history file to record your own notes about[m
how and why this particular timeline was created. Such comments will be[m
especially valuable when you have a thicket of different timelines as[m
[31m- a result of experimentation.[m
[32m+[m[32m a result of experimentation. In both WAL segment file names and history files,[m
[32m+[m[32m the timeline ID number is expressed in hexadecimal.[m
</para>[m
[m
<para>[m
[1mdiff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml[m
[1mindex 1cf53c74ea..508774cfee 100644[m
[1m--- a/doc/src/sgml/config.sgml[m
[1m+++ b/doc/src/sgml/config.sgml[m
[36m@@ -4110,7 +4110,9 @@[m [mrestore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows[m
current when the base backup was taken. The[m
value <literal>latest</literal> recovers[m
to the latest timeline found in the archive, which is useful in[m
[31m- a standby server. <literal>latest</literal> is the default.[m
[32m+[m[32m a standby server. A numerical value expressed in hexadecimal must be[m
[32m+[m[32m prefixed with <literal>0x</literal>, for example <literal>0x11</literal>.[m
[32m+[m[32m <literal>latest</literal> is the default.[m
</para>[m
[m
<para>[m
[1mdiff --git a/src/backend/access/rmgrdesc/xlogdesc.c b/src/backend/access/rmgrdesc/xlogdesc.c[m
[1mindex f390c177e4..bdbe993877 100644[m
[1m--- a/src/backend/access/rmgrdesc/xlogdesc.c[m
[1m+++ b/src/backend/access/rmgrdesc/xlogdesc.c[m
[36m@@ -45,7 +45,7 @@[m [mxlog_desc(StringInfo buf, XLogReaderState *record)[m
CheckPoint *checkpoint = (CheckPoint *) rec;[m
[m
appendStringInfo(buf, "redo %X/%X; "[m
[31m- "tli %u; prev tli %u; fpw %s; xid %u:%u; oid %u; multi %u; offset %u; "[m
[32m+[m [32m "tli 0x%X; prev tli 0x%X; fpw %s; xid %u:%u; oid %u; multi %u; offset %u; "[m
"oldest xid %u in DB %u; oldest multi %u in DB %u; "[m
"oldest/newest commit timestamp xid: %u/%u; "[m
"oldest running xid %u; %s",[m
[36m@@ -135,7 +135,7 @@[m [mxlog_desc(StringInfo buf, XLogReaderState *record)[m
xl_end_of_recovery xlrec;[m
[m
memcpy(&xlrec, rec, sizeof(xl_end_of_recovery));[m
[31m- appendStringInfo(buf, "tli %u; prev tli %u; time %s",[m
[32m+[m [32mappendStringInfo(buf, "tli 0x%X; prev tli 0x%X; time %s",[m
xlrec.ThisTimeLineID, xlrec.PrevTimeLineID,[m
timestamptz_to_str(xlrec.end_time));[m
}[m
[1mdiff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c[m
[1mindex fb4c860bde..c22cf4b2a1 100644[m
[1m--- a/src/backend/access/transam/xlog.c[m
[1m+++ b/src/backend/access/transam/xlog.c[m
[36m@@ -7819,7 +7819,7 @@[m [mxlog_redo(XLogReaderState *record)[m
(void) GetCurrentReplayRecPtr(&replayTLI);[m
if (checkPoint.ThisTimeLineID != replayTLI)[m
ereport(PANIC,[m
[31m- (errmsg("unexpected timeline ID %u (should be %u) in shutdown checkpoint record",[m
[32m+[m [32m(errmsg("unexpected timeline ID 0x%X (should be 0x%X) in shutdown checkpoint record",[m
checkPoint.ThisTimeLineID, replayTLI)));[m
[m
RecoveryRestartPoint(&checkPoint, record);[m
[36m@@ -7906,7 +7906,7 @@[m [mxlog_redo(XLogReaderState *record)[m
(void) GetCurrentReplayRecPtr(&replayTLI);[m
if (xlrec.ThisTimeLineID != replayTLI)[m
ereport(PANIC,[m
[31m- (errmsg("unexpected timeline ID %u (should be %u) in end-of-recovery record",[m
[32m+[m [32m(errmsg("unexpected timeline ID 0x%X (should be 0x%X) in end-of-recovery record",[m
xlrec.ThisTimeLineID, replayTLI)));[m
}[m
else if (info == XLOG_NOOP)[m
[1mdiff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c[m
[1mindex aa6c929477..1643d0d98c 100644[m
[1m--- a/src/backend/access/transam/xlogreader.c[m
[1m+++ b/src/backend/access/transam/xlogreader.c[m
[36m@@ -1329,7 +1329,7 @@[m [mXLogReaderValidatePageHeader(XLogReaderState *state, XLogRecPtr recptr,[m
XLogFileName(fname, state->seg.ws_tli, segno, state->segcxt.ws_segsize);[m
[m
report_invalid_record(state,[m
[31m- "out-of-sequence timeline ID %u (after %u) in WAL segment %s, LSN %X/%X, offset %u",[m
[32m+[m [32m "out-of-sequence timeline ID 0x%X (after 0x%X) in WAL segment %s, LSN %X/%X, offset %u",[m
hdr->xlp_tli,[m
state->latestPageTLI,[m
fname,[m
[1mdiff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c[m
[1mindex 2a5352f879..72087b4cf9 100644[m
[1m--- a/src/backend/access/transam/xlogrecovery.c[m
[1m+++ b/src/backend/access/transam/xlogrecovery.c[m
[36m@@ -1108,7 +1108,7 @@[m [mvalidateRecoveryParameters(void)[m
if (rtli != 1 && !existsTimeLineHistory(rtli))[m
ereport(FATAL,[m
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),[m
[31m- errmsg("recovery target timeline %u does not exist",[m
[32m+[m [32m errmsg("recovery target timeline 0x%X does not exist",[m
rtli)));[m
recoveryTargetTLI = rtli;[m
}[m
[36m@@ -3080,7 +3080,7 @@[m [mReadRecord(XLogPrefetcher *xlogprefetcher, int emode,[m
XLogFileName(fname, xlogreader->seg.ws_tli, segno,[m
wal_segment_size);[m
ereport(emode_for_corrupt_record(emode, xlogreader->EndRecPtr),[m
[31m- (errmsg("unexpected timeline ID %u in WAL segment %s, LSN %X/%X, offset %u",[m
[32m+[m [32m(errmsg("unexpected timeline ID 0x%X in WAL segment %s, LSN %X/%X, offset %u",[m
xlogreader->latestPageTLI,[m
fname,[m
LSN_FORMAT_ARGS(xlogreader->latestPagePtr),[m
[36m@@ -3719,7 +3719,7 @@[m [mWaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,[m
tli = tliOfPointInHistory(tliRecPtr, expectedTLEs);[m
[m
if (curFileTLI > 0 && tli < curFileTLI)[m
[31m- elog(ERROR, "according to history file, WAL location %X/%X belongs to timeline %u, but previous recovered WAL file came from timeline %u",[m
[32m+[m [32melog(ERROR, "according to history file, WAL location %X/%X belongs to timeline 0x%X, but previous recovered WAL file came from timeline 0x%X",[m
LSN_FORMAT_ARGS(tliRecPtr),[m
tli, curFileTLI);[m
}[m
[36m@@ -4019,7 +4019,7 @@[m [mrescanLatestTimeLine(TimeLineID replayTLI, XLogRecPtr replayLSN)[m
if (!found)[m
{[m
ereport(LOG,[m
[31m- (errmsg("new timeline %u is not a child of database system timeline %u",[m
[32m+[m [32m(errmsg("new timeline 0x%X is not a child of database system timeline 0x%X",[m
newtarget,[m
replayTLI)));[m
return false;[m
[36m@@ -4033,7 +4033,7 @@[m [mrescanLatestTimeLine(TimeLineID replayTLI, XLogRecPtr replayLSN)[m
if (currentTle->end < replayLSN)[m
{[m
ereport(LOG,[m
[31m- (errmsg("new timeline %u forked off current database system timeline %u before current recovery point %X/%X",[m
[32m+[m [32m(errmsg("new timeline 0x%X forked off current database system timeline 0x%X before current recovery point %X/%X",[m
newtarget,[m
replayTLI,[m
LSN_FORMAT_ARGS(replayLSN))));[m
[36m@@ -4052,7 +4052,7 @@[m [mrescanLatestTimeLine(TimeLineID replayTLI, XLogRecPtr replayLSN)[m
restoreTimeLineHistoryFiles(oldtarget + 1, newtarget);[m
[m
ereport(LOG,[m
[31m- (errmsg("new target timeline is %u",[m
[32m+[m [32m(errmsg("new target timeline is 0x%X",[m
recoveryTargetTLI)));[m
[m
return true;[m
[1mdiff --git a/src/backend/backup/backup_manifest.c b/src/backend/backup/backup_manifest.c[m
[1mindex fabd2ca299..a405dad984 100644[m
[1m--- a/src/backend/backup/backup_manifest.c[m
[1m+++ b/src/backend/backup/backup_manifest.c[m
[36m@@ -250,7 +250,7 @@[m [mAddWALInfoToBackupManifest(backup_manifest_info *manifest, XLogRecPtr startptr,[m
*/[m
if (first_wal_range && endtli != entry->tli)[m
ereport(ERROR,[m
[31m- errmsg("expected end timeline %u but found timeline %u",[m
[32m+[m [32merrmsg("expected end timeline 0x%X but found timeline 0x%X",[m
starttli, entry->tli));[m
[m
/*[m
[36m@@ -274,7 +274,7 @@[m [mAddWALInfoToBackupManifest(backup_manifest_info *manifest, XLogRecPtr startptr,[m
*/[m
if (XLogRecPtrIsInvalid(entry->begin))[m
ereport(ERROR,[m
[31m- errmsg("expected start timeline %u but found timeline %u",[m
[32m+[m [32merrmsg("expected start timeline 0x%X but found timeline 0x%X",[m
starttli, entry->tli));[m
}[m
[m
[36m@@ -301,7 +301,7 @@[m [mAddWALInfoToBackupManifest(backup_manifest_info *manifest, XLogRecPtr startptr,[m
*/[m
if (!found_start_timeline)[m
ereport(ERROR,[m
[31m- errmsg("start timeline %u not found in history of timeline %u",[m
[32m+[m [32merrmsg("start timeline 0x%X not found in history of timeline 0x%X",[m
starttli, endtli));[m
[m
/* Terminate the list of WAL ranges. */[m
[1mdiff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c[m
[1mindex b0cfddd548..f6ef5f75ed 100644[m
[1m--- a/src/backend/replication/walreceiver.c[m
[1m+++ b/src/backend/replication/walreceiver.c[m
[36m@@ -361,7 +361,7 @@[m [mWalReceiverMain(void)[m
if (primaryTLI < startpointTLI)[m
ereport(ERROR,[m
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),[m
[31m- errmsg("highest timeline %u of the primary is behind recovery timeline %u",[m
[32m+[m [32m errmsg("highest timeline 0x%X of the primary is behind recovery timeline 0x%X",[m
primaryTLI, startpointTLI)));[m
[m
/*[m
[36m@@ -414,11 +414,11 @@[m [mWalReceiverMain(void)[m
{[m
if (first_stream)[m
ereport(LOG,[m
[31m- (errmsg("started streaming WAL from primary at %X/%X on timeline %u",[m
[32m+[m [32m(errmsg("started streaming WAL from primary at %X/%X on timeline 0x%X",[m
LSN_FORMAT_ARGS(startpoint), startpointTLI)));[m
else[m
ereport(LOG,[m
[31m- (errmsg("restarted WAL streaming at %X/%X on timeline %u",[m
[32m+[m [32m(errmsg("restarted WAL streaming at %X/%X on timeline 0x%X",[m
LSN_FORMAT_ARGS(startpoint), startpointTLI)));[m
first_stream = false;[m
[m
[36m@@ -499,7 +499,7 @@[m [mWalReceiverMain(void)[m
{[m
ereport(LOG,[m
(errmsg("replication terminated by primary server"),[m
[31m- errdetail("End of WAL reached on timeline %u at %X/%X.",[m
[32m+[m [32m errdetail("End of WAL reached on timeline 0x%X at %X/%X.",[m
startpointTLI,[m
LSN_FORMAT_ARGS(LogstreamResult.Write))));[m
endofwal = true;[m
[36m@@ -621,7 +621,7 @@[m [mWalReceiverMain(void)[m
}[m
else[m
ereport(LOG,[m
[31m- (errmsg("primary server contains no more WAL on requested timeline %u",[m
[32m+[m [32m(errmsg("primary server contains no more WAL on requested timeline 0x%X",[m
startpointTLI)));[m
[m
/*[m
[36m@@ -756,7 +756,7 @@[m [mWalRcvFetchTimeLineHistoryFiles(TimeLineID first, TimeLineID last)[m
char expectedfname[MAXFNAMELEN];[m
[m
ereport(LOG,[m
[31m- (errmsg("fetching timeline history file for timeline %u from primary server",[m
[32m+[m [32m(errmsg("fetching timeline history file for timeline 0x%X from primary server",[m
tli)));[m
[m
walrcv_readtimelinehistoryfile(wrconn, tli, &fname, &content, &len);[m
[36m@@ -770,7 +770,7 @@[m [mWalRcvFetchTimeLineHistoryFiles(TimeLineID first, TimeLineID last)[m
if (strcmp(fname, expectedfname) != 0)[m
ereport(ERROR,[m
(errcode(ERRCODE_PROTOCOL_VIOLATION),[m
[31m- errmsg_internal("primary reported unexpected file name for timeline history file of timeline %u",[m
[32m+[m [32m errmsg_internal("primary reported unexpected file name for timeline history file of timeline 0x%X",[m
tli)));[m
[m
/*[m
[1mdiff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c[m
[1mindex 1d3ef2c694..c9491de0df 100644[m
[1m--- a/src/bin/pg_basebackup/pg_basebackup.c[m
[1m+++ b/src/bin/pg_basebackup/pg_basebackup.c[m
[36m@@ -1921,7 +1921,7 @@[m [mBaseBackup(char *compression_algorithm, char *compression_detail,[m
PQclear(res);[m
[m
if (verbose && includewal != NO_WAL)[m
[31m- pg_log_info("write-ahead log start point: %s on timeline %u",[m
[32m+[m [32mpg_log_info("write-ahead log start point: %s on timeline 0x%X",[m
xlogstart, starttli);[m
[m
/*[m
[1mdiff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c[m
[1mindex c390ec51ce..436435c737 100644[m
[1m--- a/src/bin/pg_controldata/pg_controldata.c[m
[1m+++ b/src/bin/pg_controldata/pg_controldata.c[m
[36m@@ -241,9 +241,9 @@[m [mmain(int argc, char *argv[])[m
LSN_FORMAT_ARGS(ControlFile->checkPointCopy.redo));[m
printf(_("Latest checkpoint's REDO WAL file: %s\n"),[m
xlogfilename);[m
[31m- printf(_("Latest checkpoint's TimeLineID: %u\n"),[m
[32m+[m [32mprintf(_("Latest checkpoint's TimeLineID: 0x%X\n"),[m
ControlFile->checkPointCopy.ThisTimeLineID);[m
[31m- printf(_("Latest checkpoint's PrevTimeLineID: %u\n"),[m
[32m+[m [32mprintf(_("Latest checkpoint's PrevTimeLineID: 0x%X\n"),[m
ControlFile->checkPointCopy.PrevTimeLineID);[m
printf(_("Latest checkpoint's full_page_writes: %s\n"),[m
ControlFile->checkPointCopy.fullPageWrites ? _("on") : _("off"));[m
[1mdiff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c[m
[1mindex 858d8d9f2f..7f9b7b663d 100644[m
[1m--- a/src/bin/pg_rewind/pg_rewind.c[m
[1m+++ b/src/bin/pg_rewind/pg_rewind.c[m
[36m@@ -350,7 +350,7 @@[m [mmain(int argc, char **argv)[m
XLogRecPtr chkptendrec;[m
[m
findCommonAncestorTimeline(&divergerec, &lastcommontliIndex);[m
[31m- pg_log_info("servers diverged at WAL location %X/%X on timeline %u",[m
[32m+[m [32mpg_log_info("servers diverged at WAL location %X/%X on timeline 0x%X",[m
LSN_FORMAT_ARGS(divergerec),[m
targetHistory[lastcommontliIndex].tli);[m
[m
[36m@@ -856,7 +856,7 @@[m [mgetTimelineHistory(ControlFileData *controlFile, int *nentries)[m
TimeLineHistoryEntry *entry;[m
[m
entry = &history[i];[m
[31m- pg_log_debug("%u: %X/%X - %X/%X", entry->tli,[m
[32m+[m [32mpg_log_debug("0x%X: %X/%X - %X/%X", entry->tli,[m
LSN_FORMAT_ARGS(entry->begin),[m
LSN_FORMAT_ARGS(entry->end));[m
}[m
[1mdiff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c[m
[1mindex 9071a6fd45..b04347d801 100644[m
[1m--- a/src/bin/pg_upgrade/controldata.c[m
[1m+++ b/src/bin/pg_upgrade/controldata.c[m
[36m@@ -241,7 +241,7 @@[m [mget_control_data(ClusterInfo *cluster, bool live_check)[m
pg_fatal("%d: controldata retrieval problem", __LINE__);[m
[m
p++; /* remove ':' char */[m
[31m- tli = str2uint(p);[m
[32m+[m [32mtli = strtoul(p, NULL, 0);[m
got_tli = true;[m
}[m
else if ((p = strstr(bufin, "First log file ID after reset:")) != NULL)[m
[1mdiff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c[m
[1mindex 7634dfc285..2a54d7ab52 100644[m
[1m--- a/src/bin/pg_verifybackup/pg_verifybackup.c[m
[1m+++ b/src/bin/pg_verifybackup/pg_verifybackup.c[m
[36m@@ -807,14 +807,14 @@[m [mparse_required_wal(verifier_context *context, char *pg_waldump_path,[m
{[m
char *pg_waldump_cmd;[m
[m
[31m- pg_waldump_cmd = psprintf("\"%s\" --quiet --path=\"%s\" --timeline=%u --start=%X/%X --end=%X/%X\n",[m
[32m+[m [32mpg_waldump_cmd = psprintf("\"%s\" --quiet --path=\"%s\" --timeline=%X --start=%X/%X --end=%X/%X\n",[m
pg_waldump_path, wal_directory, this_wal_range->tli,[m
LSN_FORMAT_ARGS(this_wal_range->start_lsn),[m
LSN_FORMAT_ARGS(this_wal_range->end_lsn));[m
fflush(NULL);[m
if (system(pg_waldump_cmd) != 0)[m
report_backup_error(context,[m
[31m- "WAL parsing failed for timeline %u",[m
[32m+[m [32m"WAL parsing failed for timeline %X",[m
this_wal_range->tli);[m
[m
this_wal_range = this_wal_range->next;[m
[1mdiff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c[m
[1mindex 44b5c8726e..23dc7354ba 100644[m
[1m--- a/src/bin/pg_waldump/pg_waldump.c[m
[1m+++ b/src/bin/pg_waldump/pg_waldump.c[m
[36m@@ -1007,7 +1007,7 @@[m [mmain(int argc, char **argv)[m
private.startptr = (uint64) xlogid << 32 | xrecoff;[m
break;[m
case 't':[m
[31m- if (sscanf(optarg, "%u", &private.timeline) != 1)[m
[32m+[m [32mif (sscanf(optarg, "%X", &private.timeline) != 1)[m
{[m
pg_log_error("invalid timeline specification: \"%s\"", optarg);[m
goto bad_argument;[m
On 30.01.23 17:05, Sébastien Lardière wrote:
Here's the patch with the suggested format ; plus, I add some note in
the documentation about recovery_target_timeline, because I don't get
how strtoul(), with the special 0 base parameter can work without 0x
prefix ; I suppose that nobody use it.I also change pg_controldata and the usage of this output by pg_upgrade.
I let internal usages unchanded : content of backup manifest and content
of history file.Should I open a commitfest entry, or is it too soon ?
It is not too soon. (The next commitfest is open for new patch
submissions as soon as the current one is "in progress", which closes it
for new patches.)
On Mon, Jan 30, 2023 at 9:35 PM Sébastien Lardière
<sebastien@lardiere.net> wrote:
On 27/01/2023 15:55, Peter Eisentraut wrote:
On 27.01.23 14:52, Sébastien Lardière wrote:
The attached patch proposes to change the format of timelineid from
%u to %X.I think your complaint has merit. But note that if we did a change
like this, then log files or reports from different versions would
have different meaning without a visual difference, which is kind of
what you complained about in the first place. At least we should do
something like 0x%X.Hi,
Here's the patch with the suggested format ; plus, I add some note in
the documentation about recovery_target_timeline, because I don't get
how strtoul(), with the special 0 base parameter can work without 0x
prefix ; I suppose that nobody use it.I also change pg_controldata and the usage of this output by pg_upgrade.
I let internal usages unchanded : content of backup manifest and content
of history file.
The patch seems to have some special/unprintable characters in it. I
see a lot ^[[ in there. I can't read the patch because of that.
--
Best Wishes,
Ashutosh Bapat
On 31/01/2023 12:26, Ashutosh Bapat wrote:
On Mon, Jan 30, 2023 at 9:35 PM Sébastien Lardière
<sebastien@lardiere.net> wrote:On 27/01/2023 15:55, Peter Eisentraut wrote:
On 27.01.23 14:52, Sébastien Lardière wrote:
The attached patch proposes to change the format of timelineid from
%u to %X.I think your complaint has merit. But note that if we did a change
like this, then log files or reports from different versions would
have different meaning without a visual difference, which is kind of
what you complained about in the first place. At least we should do
something like 0x%X.Hi,
Here's the patch with the suggested format ; plus, I add some note in
the documentation about recovery_target_timeline, because I don't get
how strtoul(), with the special 0 base parameter can work without 0x
prefix ; I suppose that nobody use it.I also change pg_controldata and the usage of this output by pg_upgrade.
I let internal usages unchanded : content of backup manifest and content
of history file.The patch seems to have some special/unprintable characters in it. I
see a lot ^[[ in there. I can't read the patch because of that.
Sorry for that, it was the --color from git diff, it's fixed, I hope,
thank you
regards,
--
Sébastien
Attachments:
v3_0001_timelineid_hexadecimal_format.patchtext/x-patch; charset=UTF-8; name=v3_0001_timelineid_hexadecimal_format.patchDownload
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index be05a33205..7e26b51031 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1332,7 +1332,8 @@ restore_command = 'cp /mnt/server/archivedir/%f %p'
you like, add comments to a history file to record your own notes about
how and why this particular timeline was created. Such comments will be
especially valuable when you have a thicket of different timelines as
- a result of experimentation.
+ a result of experimentation. In both WAL segment file names and history files,
+ the timeline ID number is expressed in hexadecimal.
</para>
<para>
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 1cf53c74ea..508774cfee 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4110,7 +4110,9 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
current when the base backup was taken. The
value <literal>latest</literal> recovers
to the latest timeline found in the archive, which is useful in
- a standby server. <literal>latest</literal> is the default.
+ a standby server. A numerical value expressed in hexadecimal must be
+ prefixed with <literal>0x</literal>, for example <literal>0x11</literal>.
+ <literal>latest</literal> is the default.
</para>
<para>
diff --git a/src/backend/access/rmgrdesc/xlogdesc.c b/src/backend/access/rmgrdesc/xlogdesc.c
index f390c177e4..bdbe993877 100644
--- a/src/backend/access/rmgrdesc/xlogdesc.c
+++ b/src/backend/access/rmgrdesc/xlogdesc.c
@@ -45,7 +45,7 @@ xlog_desc(StringInfo buf, XLogReaderState *record)
CheckPoint *checkpoint = (CheckPoint *) rec;
appendStringInfo(buf, "redo %X/%X; "
- "tli %u; prev tli %u; fpw %s; xid %u:%u; oid %u; multi %u; offset %u; "
+ "tli 0x%X; prev tli 0x%X; fpw %s; xid %u:%u; oid %u; multi %u; offset %u; "
"oldest xid %u in DB %u; oldest multi %u in DB %u; "
"oldest/newest commit timestamp xid: %u/%u; "
"oldest running xid %u; %s",
@@ -135,7 +135,7 @@ xlog_desc(StringInfo buf, XLogReaderState *record)
xl_end_of_recovery xlrec;
memcpy(&xlrec, rec, sizeof(xl_end_of_recovery));
- appendStringInfo(buf, "tli %u; prev tli %u; time %s",
+ appendStringInfo(buf, "tli 0x%X; prev tli 0x%X; time %s",
xlrec.ThisTimeLineID, xlrec.PrevTimeLineID,
timestamptz_to_str(xlrec.end_time));
}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index fb4c860bde..c22cf4b2a1 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7819,7 +7819,7 @@ xlog_redo(XLogReaderState *record)
(void) GetCurrentReplayRecPtr(&replayTLI);
if (checkPoint.ThisTimeLineID != replayTLI)
ereport(PANIC,
- (errmsg("unexpected timeline ID %u (should be %u) in shutdown checkpoint record",
+ (errmsg("unexpected timeline ID 0x%X (should be 0x%X) in shutdown checkpoint record",
checkPoint.ThisTimeLineID, replayTLI)));
RecoveryRestartPoint(&checkPoint, record);
@@ -7906,7 +7906,7 @@ xlog_redo(XLogReaderState *record)
(void) GetCurrentReplayRecPtr(&replayTLI);
if (xlrec.ThisTimeLineID != replayTLI)
ereport(PANIC,
- (errmsg("unexpected timeline ID %u (should be %u) in end-of-recovery record",
+ (errmsg("unexpected timeline ID 0x%X (should be 0x%X) in end-of-recovery record",
xlrec.ThisTimeLineID, replayTLI)));
}
else if (info == XLOG_NOOP)
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index aa6c929477..1643d0d98c 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -1329,7 +1329,7 @@ XLogReaderValidatePageHeader(XLogReaderState *state, XLogRecPtr recptr,
XLogFileName(fname, state->seg.ws_tli, segno, state->segcxt.ws_segsize);
report_invalid_record(state,
- "out-of-sequence timeline ID %u (after %u) in WAL segment %s, LSN %X/%X, offset %u",
+ "out-of-sequence timeline ID 0x%X (after 0x%X) in WAL segment %s, LSN %X/%X, offset %u",
hdr->xlp_tli,
state->latestPageTLI,
fname,
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 2a5352f879..72087b4cf9 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -1108,7 +1108,7 @@ validateRecoveryParameters(void)
if (rtli != 1 && !existsTimeLineHistory(rtli))
ereport(FATAL,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("recovery target timeline %u does not exist",
+ errmsg("recovery target timeline 0x%X does not exist",
rtli)));
recoveryTargetTLI = rtli;
}
@@ -3080,7 +3080,7 @@ ReadRecord(XLogPrefetcher *xlogprefetcher, int emode,
XLogFileName(fname, xlogreader->seg.ws_tli, segno,
wal_segment_size);
ereport(emode_for_corrupt_record(emode, xlogreader->EndRecPtr),
- (errmsg("unexpected timeline ID %u in WAL segment %s, LSN %X/%X, offset %u",
+ (errmsg("unexpected timeline ID 0x%X in WAL segment %s, LSN %X/%X, offset %u",
xlogreader->latestPageTLI,
fname,
LSN_FORMAT_ARGS(xlogreader->latestPagePtr),
@@ -3719,7 +3719,7 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
tli = tliOfPointInHistory(tliRecPtr, expectedTLEs);
if (curFileTLI > 0 && tli < curFileTLI)
- elog(ERROR, "according to history file, WAL location %X/%X belongs to timeline %u, but previous recovered WAL file came from timeline %u",
+ elog(ERROR, "according to history file, WAL location %X/%X belongs to timeline 0x%X, but previous recovered WAL file came from timeline 0x%X",
LSN_FORMAT_ARGS(tliRecPtr),
tli, curFileTLI);
}
@@ -4019,7 +4019,7 @@ rescanLatestTimeLine(TimeLineID replayTLI, XLogRecPtr replayLSN)
if (!found)
{
ereport(LOG,
- (errmsg("new timeline %u is not a child of database system timeline %u",
+ (errmsg("new timeline 0x%X is not a child of database system timeline 0x%X",
newtarget,
replayTLI)));
return false;
@@ -4033,7 +4033,7 @@ rescanLatestTimeLine(TimeLineID replayTLI, XLogRecPtr replayLSN)
if (currentTle->end < replayLSN)
{
ereport(LOG,
- (errmsg("new timeline %u forked off current database system timeline %u before current recovery point %X/%X",
+ (errmsg("new timeline 0x%X forked off current database system timeline 0x%X before current recovery point %X/%X",
newtarget,
replayTLI,
LSN_FORMAT_ARGS(replayLSN))));
@@ -4052,7 +4052,7 @@ rescanLatestTimeLine(TimeLineID replayTLI, XLogRecPtr replayLSN)
restoreTimeLineHistoryFiles(oldtarget + 1, newtarget);
ereport(LOG,
- (errmsg("new target timeline is %u",
+ (errmsg("new target timeline is 0x%X",
recoveryTargetTLI)));
return true;
diff --git a/src/backend/backup/backup_manifest.c b/src/backend/backup/backup_manifest.c
index fabd2ca299..a405dad984 100644
--- a/src/backend/backup/backup_manifest.c
+++ b/src/backend/backup/backup_manifest.c
@@ -250,7 +250,7 @@ AddWALInfoToBackupManifest(backup_manifest_info *manifest, XLogRecPtr startptr,
*/
if (first_wal_range && endtli != entry->tli)
ereport(ERROR,
- errmsg("expected end timeline %u but found timeline %u",
+ errmsg("expected end timeline 0x%X but found timeline 0x%X",
starttli, entry->tli));
/*
@@ -274,7 +274,7 @@ AddWALInfoToBackupManifest(backup_manifest_info *manifest, XLogRecPtr startptr,
*/
if (XLogRecPtrIsInvalid(entry->begin))
ereport(ERROR,
- errmsg("expected start timeline %u but found timeline %u",
+ errmsg("expected start timeline 0x%X but found timeline 0x%X",
starttli, entry->tli));
}
@@ -301,7 +301,7 @@ AddWALInfoToBackupManifest(backup_manifest_info *manifest, XLogRecPtr startptr,
*/
if (!found_start_timeline)
ereport(ERROR,
- errmsg("start timeline %u not found in history of timeline %u",
+ errmsg("start timeline 0x%X not found in history of timeline 0x%X",
starttli, endtli));
/* Terminate the list of WAL ranges. */
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index b0cfddd548..f6ef5f75ed 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -361,7 +361,7 @@ WalReceiverMain(void)
if (primaryTLI < startpointTLI)
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("highest timeline %u of the primary is behind recovery timeline %u",
+ errmsg("highest timeline 0x%X of the primary is behind recovery timeline 0x%X",
primaryTLI, startpointTLI)));
/*
@@ -414,11 +414,11 @@ WalReceiverMain(void)
{
if (first_stream)
ereport(LOG,
- (errmsg("started streaming WAL from primary at %X/%X on timeline %u",
+ (errmsg("started streaming WAL from primary at %X/%X on timeline 0x%X",
LSN_FORMAT_ARGS(startpoint), startpointTLI)));
else
ereport(LOG,
- (errmsg("restarted WAL streaming at %X/%X on timeline %u",
+ (errmsg("restarted WAL streaming at %X/%X on timeline 0x%X",
LSN_FORMAT_ARGS(startpoint), startpointTLI)));
first_stream = false;
@@ -499,7 +499,7 @@ WalReceiverMain(void)
{
ereport(LOG,
(errmsg("replication terminated by primary server"),
- errdetail("End of WAL reached on timeline %u at %X/%X.",
+ errdetail("End of WAL reached on timeline 0x%X at %X/%X.",
startpointTLI,
LSN_FORMAT_ARGS(LogstreamResult.Write))));
endofwal = true;
@@ -621,7 +621,7 @@ WalReceiverMain(void)
}
else
ereport(LOG,
- (errmsg("primary server contains no more WAL on requested timeline %u",
+ (errmsg("primary server contains no more WAL on requested timeline 0x%X",
startpointTLI)));
/*
@@ -756,7 +756,7 @@ WalRcvFetchTimeLineHistoryFiles(TimeLineID first, TimeLineID last)
char expectedfname[MAXFNAMELEN];
ereport(LOG,
- (errmsg("fetching timeline history file for timeline %u from primary server",
+ (errmsg("fetching timeline history file for timeline 0x%X from primary server",
tli)));
walrcv_readtimelinehistoryfile(wrconn, tli, &fname, &content, &len);
@@ -770,7 +770,7 @@ WalRcvFetchTimeLineHistoryFiles(TimeLineID first, TimeLineID last)
if (strcmp(fname, expectedfname) != 0)
ereport(ERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
- errmsg_internal("primary reported unexpected file name for timeline history file of timeline %u",
+ errmsg_internal("primary reported unexpected file name for timeline history file of timeline 0x%X",
tli)));
/*
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 1d3ef2c694..c9491de0df 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1921,7 +1921,7 @@ BaseBackup(char *compression_algorithm, char *compression_detail,
PQclear(res);
if (verbose && includewal != NO_WAL)
- pg_log_info("write-ahead log start point: %s on timeline %u",
+ pg_log_info("write-ahead log start point: %s on timeline 0x%X",
xlogstart, starttli);
/*
diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c
index c390ec51ce..436435c737 100644
--- a/src/bin/pg_controldata/pg_controldata.c
+++ b/src/bin/pg_controldata/pg_controldata.c
@@ -241,9 +241,9 @@ main(int argc, char *argv[])
LSN_FORMAT_ARGS(ControlFile->checkPointCopy.redo));
printf(_("Latest checkpoint's REDO WAL file: %s\n"),
xlogfilename);
- printf(_("Latest checkpoint's TimeLineID: %u\n"),
+ printf(_("Latest checkpoint's TimeLineID: 0x%X\n"),
ControlFile->checkPointCopy.ThisTimeLineID);
- printf(_("Latest checkpoint's PrevTimeLineID: %u\n"),
+ printf(_("Latest checkpoint's PrevTimeLineID: 0x%X\n"),
ControlFile->checkPointCopy.PrevTimeLineID);
printf(_("Latest checkpoint's full_page_writes: %s\n"),
ControlFile->checkPointCopy.fullPageWrites ? _("on") : _("off"));
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 858d8d9f2f..7f9b7b663d 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -350,7 +350,7 @@ main(int argc, char **argv)
XLogRecPtr chkptendrec;
findCommonAncestorTimeline(&divergerec, &lastcommontliIndex);
- pg_log_info("servers diverged at WAL location %X/%X on timeline %u",
+ pg_log_info("servers diverged at WAL location %X/%X on timeline 0x%X",
LSN_FORMAT_ARGS(divergerec),
targetHistory[lastcommontliIndex].tli);
@@ -856,7 +856,7 @@ getTimelineHistory(ControlFileData *controlFile, int *nentries)
TimeLineHistoryEntry *entry;
entry = &history[i];
- pg_log_debug("%u: %X/%X - %X/%X", entry->tli,
+ pg_log_debug("0x%X: %X/%X - %X/%X", entry->tli,
LSN_FORMAT_ARGS(entry->begin),
LSN_FORMAT_ARGS(entry->end));
}
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 9071a6fd45..b04347d801 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -241,7 +241,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
pg_fatal("%d: controldata retrieval problem", __LINE__);
p++; /* remove ':' char */
- tli = str2uint(p);
+ tli = strtoul(p, NULL, 0);
got_tli = true;
}
else if ((p = strstr(bufin, "First log file ID after reset:")) != NULL)
diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c
index 7634dfc285..2a54d7ab52 100644
--- a/src/bin/pg_verifybackup/pg_verifybackup.c
+++ b/src/bin/pg_verifybackup/pg_verifybackup.c
@@ -807,14 +807,14 @@ parse_required_wal(verifier_context *context, char *pg_waldump_path,
{
char *pg_waldump_cmd;
- pg_waldump_cmd = psprintf("\"%s\" --quiet --path=\"%s\" --timeline=%u --start=%X/%X --end=%X/%X\n",
+ pg_waldump_cmd = psprintf("\"%s\" --quiet --path=\"%s\" --timeline=%X --start=%X/%X --end=%X/%X\n",
pg_waldump_path, wal_directory, this_wal_range->tli,
LSN_FORMAT_ARGS(this_wal_range->start_lsn),
LSN_FORMAT_ARGS(this_wal_range->end_lsn));
fflush(NULL);
if (system(pg_waldump_cmd) != 0)
report_backup_error(context,
- "WAL parsing failed for timeline %u",
+ "WAL parsing failed for timeline %X",
this_wal_range->tli);
this_wal_range = this_wal_range->next;
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 44b5c8726e..23dc7354ba 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1007,7 +1007,7 @@ main(int argc, char **argv)
private.startptr = (uint64) xlogid << 32 | xrecoff;
break;
case 't':
- if (sscanf(optarg, "%u", &private.timeline) != 1)
+ if (sscanf(optarg, "%X", &private.timeline) != 1)
{
pg_log_error("invalid timeline specification: \"%s\"", optarg);
goto bad_argument;
On 31/01/2023 10:53, Peter Eisentraut wrote:
On 30.01.23 17:05, Sébastien Lardière wrote:
Here's the patch with the suggested format ; plus, I add some note in
the documentation about recovery_target_timeline, because I don't get
how strtoul(), with the special 0 base parameter can work without 0x
prefix ; I suppose that nobody use it.I also change pg_controldata and the usage of this output by
pg_upgrade. I let internal usages unchanded : content of backup
manifest and content of history file.Should I open a commitfest entry, or is it too soon ?
It is not too soon. (The next commitfest is open for new patch
submissions as soon as the current one is "in progress", which closes
it for new patches.)
Done : https://commitfest.postgresql.org/42/4155/
--
Sébastien
I actually find it kind of annoying that we use hex strings for a lot
of things where they don't add any value. Namely Transaction ID and
LSNs. As a result it's always a bit of a pain to ingest these in other
tools or do arithmetic on them. Neither is referring to memory or
anything where powers of 2 are significant so it really doesn't buy
anything in making them easier to interpret either.
I don't see any advantage in converting every place where we refer to
timelines into hex and then having to refer to things like timeline
1A. It doesn't seem any more intuitive to someone understanding what's
going on than referring to timeline 26.
The fact that the *filename* has it encoded in hex is an
implementation detail and really gets exposed here because it's giving
you the underlying system error that caused the problem. The confusion
only arises when the two are juxtaposed. A hint or something just in
that case might be enough?
On 31/01/2023 20:16, Greg Stark wrote:
The fact that the *filename* has it encoded in hex is an
implementation detail and really gets exposed here because it's giving
you the underlying system error that caused the problem.
It's an implementation detail, but an exposed detail, so, people refer
to the filename to find the timeline ID (That's why it happened to me)
The confusion
only arises when the two are juxtaposed. A hint or something just in
that case might be enough?
Thanks, i got your point.
Note that my proposal was to remove the ambiguous notation which
happen in some case (as in 11 <-> 17). A hint is useless in most of the
case, because there is no ambiguous. That's why i though format
hexadecimal everywhere.
At least, can I propose to improve the documentation to expose the fact
that the timeline ID is exposed in hexadecimal in filenames but must be
used in decimal in recovery_target_timeline and pg_waldump ?
regards,
--
Sébastien
On 31/01/2023 20:16, Greg Stark wrote:
A hint or something just in
that case might be enough?
It seems to be a -1 ;
let's try to improve the documentation, with the attached patch
best regards,
--
Sébastien
Attachments:
v4_0001_timelineid_hexadecimal_format.patchtext/x-patch; charset=UTF-8; name=v4_0001_timelineid_hexadecimal_format.patchDownload
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index be05a33205..7e26b51031 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1332,7 +1332,8 @@ restore_command = 'cp /mnt/server/archivedir/%f %p'
you like, add comments to a history file to record your own notes about
how and why this particular timeline was created. Such comments will be
especially valuable when you have a thicket of different timelines as
- a result of experimentation.
+ a result of experimentation. In both WAL segment file names and history files,
+ the timeline ID number is expressed in hexadecimal.
</para>
<para>
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e5c41cc6c6..3b5d041d92 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4110,7 +4110,9 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
current when the base backup was taken. The
value <literal>latest</literal> recovers
to the latest timeline found in the archive, which is useful in
- a standby server. <literal>latest</literal> is the default.
+ a standby server. A numerical value expressed in hexadecimal must be
+ prefixed with <literal>0x</literal>, for example <literal>0x11</literal>.
+ <literal>latest</literal> is the default.
</para>
<para>
diff --git a/doc/src/sgml/ref/pg_waldump.sgml b/doc/src/sgml/ref/pg_waldump.sgml
index 343f0482a9..4ae8f2ebdd 100644
--- a/doc/src/sgml/ref/pg_waldump.sgml
+++ b/doc/src/sgml/ref/pg_waldump.sgml
@@ -215,7 +215,8 @@ PostgreSQL documentation
<para>
Timeline from which to read WAL records. The default is to use the
value in <replaceable>startseg</replaceable>, if that is specified; otherwise, the
- default is 1.
+ default is 1. The value must be expressed in decimal, contrary to the hexadecimal
+ value given in WAL segment file names and history files.
</para>
</listitem>
</varlistentry>
On 24.02.23 17:27, Sébastien Lardière wrote:
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml index be05a33205..7e26b51031 100644 --- a/doc/src/sgml/backup.sgml +++ b/doc/src/sgml/backup.sgml @@ -1332,7 +1332,8 @@ restore_command = 'cp/mnt/server/archivedir/%f %p' you like, add comments to a history file to record your own notes about how and why this particular timeline was created. Such comments will be especially valuable when you have a thicket of different timelines as - a result of experimentation. + a result of experimentation. In both WAL segment file names and history files, + the timeline ID number is expressed in hexadecimal. </para><para>
I think here it would be more helpful to show actual examples. Like,
here is a possible file name, this is what the different parts mean.
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index e5c41cc6c6..3b5d041d92 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -4110,7 +4110,9 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows current when the base backup was taken. The value <literal>latest</literal> recovers to the latest timeline found in the archive, which is useful in - a standby server. <literal>latest</literal> is the default. + a standby server. A numerical value expressed in hexadecimal must be + prefixed with <literal>0x</literal>, for example <literal>0x11</literal>. + <literal>latest</literal> is the default. </para><para>
This applies to all configuration parameters, so it doesn't need to be
mentioned explicitly for individual ones.
diff --git a/doc/src/sgml/ref/pg_waldump.sgml b/doc/src/sgml/ref/pg_waldump.sgml index 343f0482a9..4ae8f2ebdd 100644 --- a/doc/src/sgml/ref/pg_waldump.sgml +++ b/doc/src/sgml/ref/pg_waldump.sgml @@ -215,7 +215,8 @@ PostgreSQL documentation <para> Timeline from which to read WAL records. The default is to use the value in <replaceable>startseg</replaceable>, if that is specified; otherwise, the - default is 1. + default is 1. The value must be expressed in decimal, contrary to the hexadecimal + value given in WAL segment file names and history files. </para> </listitem> </varlistentry>
Maybe this could be fixed instead?
On 02/03/2023 09:12, Peter Eisentraut wrote:
On 24.02.23 17:27, Sébastien Lardière wrote:
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml index be05a33205..7e26b51031 100644 --- a/doc/src/sgml/backup.sgml +++ b/doc/src/sgml/backup.sgml @@ -1332,7 +1332,8 @@ restore_command = 'cp/mnt/server/archivedir/%f %p' you like, add comments to a history file to record your own notes about how and why this particular timeline was created. Such comments will be especially valuable when you have a thicket of different timelines as - a result of experimentation. + a result of experimentation. In both WAL segment file names and history files, + the timeline ID number is expressed in hexadecimal. </para> <para>I think here it would be more helpful to show actual examples. Like,
here is a possible file name, this is what the different parts mean.
So you mean explain the WAL filename and the history filename ? Is it
the good place for it ?
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index e5c41cc6c6..3b5d041d92 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -4110,7 +4110,9 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows current when the base backup was taken. The value <literal>latest</literal> recovers to the latest timeline found in the archive, which is useful in - a standby server. <literal>latest</literal> is the default. + a standby server. A numerical value expressed in hexadecimal must be + prefixed with <literal>0x</literal>, for example <literal>0x11</literal>. + <literal>latest</literal> is the default. </para> <para>This applies to all configuration parameters, so it doesn't need to be
mentioned explicitly for individual ones.
Probably, but is there another parameter with the same consequence ?
worth it to document this point globally ?
diff --git a/doc/src/sgml/ref/pg_waldump.sgml b/doc/src/sgml/ref/pg_waldump.sgml index 343f0482a9..4ae8f2ebdd 100644 --- a/doc/src/sgml/ref/pg_waldump.sgml +++ b/doc/src/sgml/ref/pg_waldump.sgml @@ -215,7 +215,8 @@ PostgreSQL documentation <para> Timeline from which to read WAL records. The default is to use the value in <replaceable>startseg</replaceable>, if that is specified; otherwise, the - default is 1. + default is 1. The value must be expressed in decimal, contrary to the hexadecimal + value given in WAL segment file names and history files. </para> </listitem> </varlistentry>Maybe this could be fixed instead?
Indeed, and strtoul is probably a better option than sscanf, don't you
think ?
--
Sébastien
On Tue, Jan 31, 2023 at 2:16 PM Greg Stark <stark@mit.edu> wrote:
I don't see any advantage in converting every place where we refer to
timelines into hex and then having to refer to things like timeline
1A. It doesn't seem any more intuitive to someone understanding what's
going on than referring to timeline 26.
The point, though, is that the WAL files we have on disk already say
1A. If we change the log messages to match, that's easier for users.
We could alternatively change the naming convention for WAL files on
disk, but that feels like a much bigger compatibility break.
--
Robert Haas
EDB: http://www.enterprisedb.com
On 03.03.23 16:52, Sébastien Lardière wrote:
On 02/03/2023 09:12, Peter Eisentraut wrote:
On 24.02.23 17:27, Sébastien Lardière wrote:
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml index be05a33205..7e26b51031 100644 --- a/doc/src/sgml/backup.sgml +++ b/doc/src/sgml/backup.sgml @@ -1332,7 +1332,8 @@ restore_command = 'cp/mnt/server/archivedir/%f %p' you like, add comments to a history file to record your own notes about how and why this particular timeline was created. Such comments will be especially valuable when you have a thicket of different timelines as - a result of experimentation. + a result of experimentation. In both WAL segment file names and history files, + the timeline ID number is expressed in hexadecimal. </para> <para>I think here it would be more helpful to show actual examples. Like,
here is a possible file name, this is what the different parts mean.So you mean explain the WAL filename and the history filename ? Is it
the good place for it ?
Well, your patch says, by the way, the timeline ID in the file is
hexadecimal. Then one might ask, what file, what is a timeline, what
are the other numbers in the file, etc. It seems very specific in this
context. I don't know if the format of these file names is actually
documented somewhere.
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index e5c41cc6c6..3b5d041d92 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -4110,7 +4110,9 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows current when the base backup was taken. The value <literal>latest</literal> recovers to the latest timeline found in the archive, which is useful in - a standby server. <literal>latest</literal> is the default. + a standby server. A numerical value expressed in hexadecimal must be + prefixed with <literal>0x</literal>, for example <literal>0x11</literal>. + <literal>latest</literal> is the default. </para> <para>This applies to all configuration parameters, so it doesn't need to be
mentioned explicitly for individual ones.Probably, but is there another parameter with the same consequence ?
worth it to document this point globally ?
It's ok to mention it again. We do something similar for example at
unix_socket_permissions. But maybe with more context, like "If you want
to specify a timeline ID hexadecimal (for example, if extracted from a
WAL file name), then prefix it with a 0x".
diff --git a/doc/src/sgml/ref/pg_waldump.sgml b/doc/src/sgml/ref/pg_waldump.sgml index 343f0482a9..4ae8f2ebdd 100644 --- a/doc/src/sgml/ref/pg_waldump.sgml +++ b/doc/src/sgml/ref/pg_waldump.sgml @@ -215,7 +215,8 @@ PostgreSQL documentation <para> Timeline from which to read WAL records. The default is to use the value in <replaceable>startseg</replaceable>, if that is specified; otherwise, the - default is 1. + default is 1. The value must be expressed in decimal, contrary to the hexadecimal + value given in WAL segment file names and history files. </para> </listitem> </varlistentry>Maybe this could be fixed instead?
Indeed, and strtoul is probably a better option than sscanf, don't you
think ?
Yeah, the use of sscanf() is kind of weird here. We have been moving
the option parsing to use option_parse_int(). Maybe hex support could
be added there. Or just use strtoul().
On 06/03/2023 18:04, Peter Eisentraut wrote:
On 03.03.23 16:52, Sébastien Lardière wrote:
On 02/03/2023 09:12, Peter Eisentraut wrote:
I think here it would be more helpful to show actual examples. Like,
here is a possible file name, this is what the different parts mean.So you mean explain the WAL filename and the history filename ? Is it
the good place for it ?Well, your patch says, by the way, the timeline ID in the file is
hexadecimal. Then one might ask, what file, what is a timeline, what
are the other numbers in the file, etc. It seems very specific in
this context. I don't know if the format of these file names is
actually documented somewhere.
Well, in the context of this patch, the usage both filename are
explained juste before, so it seems understandable to me
Timelines are explained in this place :
https://www.postgresql.org/docs/current/continuous-archiving.html#BACKUP-TIMELINES
so the patch explains the format there
This applies to all configuration parameters, so it doesn't need to
be mentioned explicitly for individual ones.Probably, but is there another parameter with the same consequence ?
worth it to document this point globally ?
It's ok to mention it again. We do something similar for example at
unix_socket_permissions. But maybe with more context, like "If you
want to specify a timeline ID hexadecimal (for example, if extracted
from a WAL file name), then prefix it with a 0x".
Ok, I've improved the message
Maybe this could be fixed instead?
Indeed, and strtoul is probably a better option than sscanf, don't
you think ?Yeah, the use of sscanf() is kind of weird here. We have been moving
the option parsing to use option_parse_int(). Maybe hex support could
be added there. Or just use strtoul().
I've made the change with strtoul
About option_parse_int(), actually, strtoint() is used, do we need a
option_parse_ul() fonction ?
patch attached,
best regards,
--
Sébastien
Attachments:
v5_0001_timelineid_hexadecimal_format.patchtext/x-patch; charset=UTF-8; name=v5_0001_timelineid_hexadecimal_format.patchDownload
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index be05a33205..fb86a3fec5 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1332,7 +1332,8 @@ restore_command = 'cp /mnt/server/archivedir/%f %p'
you like, add comments to a history file to record your own notes about
how and why this particular timeline was created. Such comments will be
especially valuable when you have a thicket of different timelines as
- a result of experimentation.
+ a result of experimentation. The timeline identifier is an integer which
+ is used in hexadecimal format in both WAL segment file names and history files.
</para>
<para>
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e5c41cc6c6..6c0d63b73d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4110,7 +4110,11 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
current when the base backup was taken. The
value <literal>latest</literal> recovers
to the latest timeline found in the archive, which is useful in
- a standby server. <literal>latest</literal> is the default.
+ a standby server. A numerical value expressed in hexadecimal, by exemple
+ from WAL filename or history file, must be prefixed with <literal>0x</literal>,
+ for example <literal>0x11</literal> if the WAL filename
+ is <filename>00000011000000A10000004F</filename>.
+ <literal>latest</literal> is the default.
</para>
<para>
diff --git a/doc/src/sgml/ref/pg_waldump.sgml b/doc/src/sgml/ref/pg_waldump.sgml
index 343f0482a9..d92948c68a 100644
--- a/doc/src/sgml/ref/pg_waldump.sgml
+++ b/doc/src/sgml/ref/pg_waldump.sgml
@@ -215,7 +215,9 @@ PostgreSQL documentation
<para>
Timeline from which to read WAL records. The default is to use the
value in <replaceable>startseg</replaceable>, if that is specified; otherwise, the
- default is 1.
+ default is 1. The value can be expressed in decimal or hexadecimal format.
+ The hexadecimal format, given by WAL filename, must be preceded by <literal>0x</literal>,
+ by example <literal>0x11</literal>.
</para>
</listitem>
</varlistentry>
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 44b5c8726e..b29d5223ce 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -770,6 +770,7 @@ usage(void)
printf(_(" -R, --relation=T/D/R only show records that modify blocks in relation T/D/R\n"));
printf(_(" -s, --start=RECPTR start reading at WAL location RECPTR\n"));
printf(_(" -t, --timeline=TLI timeline from which to read WAL records\n"
+ " hexadecimal value, from WAL filename, must be preceded by 0x\n"
" (default: 1 or the value used in STARTSEG)\n"));
printf(_(" -V, --version output version information, then exit\n"));
printf(_(" -w, --fullpage only show records with a full page write\n"));
@@ -1007,7 +1008,7 @@ main(int argc, char **argv)
private.startptr = (uint64) xlogid << 32 | xrecoff;
break;
case 't':
- if (sscanf(optarg, "%u", &private.timeline) != 1)
+ if ( !(private.timeline = strtoul(optarg, NULL, 0)) )
{
pg_log_error("invalid timeline specification: \"%s\"", optarg);
goto bad_argument;
I have committed the two documentation changes, with some minor adjustments.
On 07.03.23 18:14, Sébastien Lardière wrote:
Maybe this could be fixed instead?
Indeed, and strtoul is probably a better option than sscanf, don't
you think ?Yeah, the use of sscanf() is kind of weird here. We have been moving
the option parsing to use option_parse_int(). Maybe hex support could
be added there. Or just use strtoul().I've made the change with strtoul
About option_parse_int(), actually, strtoint() is used, do we need a
option_parse_ul() fonction ?
For the option parsing, I propose the attached patch. This follows the
structure of option_parse_int(), so in the future it could be extracted
and refactored in the same way, if there is more need.
Attachments:
v6-0001-pg_waldump-Allow-hexadecimal-values-for-t-timelin.patchtext/plain; charset=UTF-8; name=v6-0001-pg_waldump-Allow-hexadecimal-values-for-t-timelin.patchDownload
From a19f9aea4fdeed473d490db6c79a5425bc2bcafd Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 20 Mar 2023 09:11:35 +0100
Subject: [PATCH v6] pg_waldump: Allow hexadecimal values for -t/--timeline
option
Discussion: https://www.postgresql.org/message-id/flat/8fef346e-2541-76c3-d768-6536ae052993@lardiere.net
---
doc/src/sgml/ref/pg_waldump.sgml | 3 ++-
src/bin/pg_waldump/pg_waldump.c | 37 ++++++++++++++++++++++++++++----
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/ref/pg_waldump.sgml b/doc/src/sgml/ref/pg_waldump.sgml
index 343f0482a9..7685d3d15b 100644
--- a/doc/src/sgml/ref/pg_waldump.sgml
+++ b/doc/src/sgml/ref/pg_waldump.sgml
@@ -215,7 +215,8 @@ <title>Options</title>
<para>
Timeline from which to read WAL records. The default is to use the
value in <replaceable>startseg</replaceable>, if that is specified; otherwise, the
- default is 1.
+ default is 1. The value can be specified in decimal or hexadecimal,
+ for example <literal>17</literal> or <literal>0x11</literal>.
</para>
</listitem>
</varlistentry>
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 44b5c8726e..8630000ef0 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -13,6 +13,7 @@
#include "postgres.h"
#include <dirent.h>
+#include <limits.h>
#include <signal.h>
#include <sys/stat.h>
#include <unistd.h>
@@ -1007,12 +1008,40 @@ main(int argc, char **argv)
private.startptr = (uint64) xlogid << 32 | xrecoff;
break;
case 't':
- if (sscanf(optarg, "%u", &private.timeline) != 1)
+
+ /*
+ * This is like option_parse_int() but needs to handle
+ * unsigned 32-bit int. Also, we accept both decimal and
+ * hexadecimal specifications here.
+ */
{
- pg_log_error("invalid timeline specification: \"%s\"", optarg);
- goto bad_argument;
+ char *endptr;
+ unsigned long val;
+
+ errno = 0;
+ val = strtoul(optarg, &endptr, 0);
+
+ while (*endptr != '\0' && isspace((unsigned char) *endptr))
+ endptr++;
+
+ if (*endptr != '\0')
+ {
+ pg_log_error("invalid value \"%s\" for option %s",
+ optarg, "-t/--timeline");
+ goto bad_argument;
+ }
+
+ if (errno == ERANGE || val < 1 || val > UINT_MAX)
+ {
+ pg_log_error("%s must be in range %u..%u",
+ "-t/--timeline", 1, UINT_MAX);
+ goto bad_argument;
+ }
+
+ private.timeline = val;
+
+ break;
}
- break;
case 'w':
config.filter_by_fpw = true;
break;
base-commit: 0b51d423e974557e821d890c0a3a49e419a19caa
--
2.39.2
On 20/03/2023 09:17, Peter Eisentraut wrote:
I have committed the two documentation changes, with some minor
adjustments.
Thank you,
On 07.03.23 18:14, Sébastien Lardière wrote:
Maybe this could be fixed instead?
Indeed, and strtoul is probably a better option than sscanf, don't
you think ?Yeah, the use of sscanf() is kind of weird here. We have been
moving the option parsing to use option_parse_int(). Maybe hex
support could be added there. Or just use strtoul().I've made the change with strtoul
About option_parse_int(), actually, strtoint() is used, do we need a
option_parse_ul() fonction ?For the option parsing, I propose the attached patch. This follows
the structure of option_parse_int(), so in the future it could be
extracted and refactored in the same way, if there is more need.
ok for me, it accept 0x values and refuse wrong values
thank you,
--
Sébastien
On 20.03.23 10:40, Sébastien Lardière wrote:
About option_parse_int(), actually, strtoint() is used, do we need a
option_parse_ul() fonction ?For the option parsing, I propose the attached patch. This follows
the structure of option_parse_int(), so in the future it could be
extracted and refactored in the same way, if there is more need.ok for me, it accept 0x values and refuse wrong values
committed
On 21/03/2023 08:15, Peter Eisentraut wrote:
On 20.03.23 10:40, Sébastien Lardière wrote:
About option_parse_int(), actually, strtoint() is used, do we need
a option_parse_ul() fonction ?For the option parsing, I propose the attached patch. This follows
the structure of option_parse_int(), so in the future it could be
extracted and refactored in the same way, if there is more need.ok for me, it accept 0x values and refuse wrong values
committed
thanks,
--
Sébastien