pg_rewind does not rewind diverging timelines

On Thu, Apr 30, 2026 at 10:19 AM Mats Kindahl <mats.kindahl@gmail.com>
wrote:

Hi all,

I have been playing around with various promotion scenarios to check if it
is possible to lose writes in more complicated scenarios involving
promotions and uses of synchronous_standby_names and decided to create a
TLA+ model for streaming replication involving promotions and check those
with TLC. You can find the models at [1] if you're interested.

There is one scenario that I assume is known that TLC found, but does not
seem to be fixed. It is a relatively rare case, but since the fix is quite
easy, I thought I'd share it with you and get feedback.

The scenario can occur if you're unlucky and have more than one crash when
promoting standbys to be primaries, and goes like this:

You have three servers, S1, S2, and S3. S1 is primary and S2 and S3 are
standbys. All are on timeline (TLI) 1.

1. S1 crashes
2. S1 recovers and starts promotion. It writes XLOG_END_OF_RECOVERY (EOR)
for TLI 2 to the WAL.
3. S1 It manages to write some records W1 to the WAL.
4. Before the EOR is replicated to any standby, S1 crashes again. It is
now on TLI 2 and has some changes that are not elsewhere.
5. S2 is promoted. It writes an EOR for TLI 2 (since it is not aware of
any other timeline) to the WAL.
6. S2 writes some records W2 to WAL and now S1 has a record of TLI 2
version 1 (TLI 2.1) and S2 is on TLI 2.2.
7. S1 recovers and wants to join as a standby. You run pg_rewind to get
rid of the extra data, but since S2 is also on TLI 2, pg_rewind will
happily assume that both are on the same timeline.
8. S2 is now a standby but has that extra record for W2 both in the WAL
and in the database.

The fix (see attached draft) is quite simple: add a UUID to the EOR and to
the history file. When comparing timelines, don't only check the TLI, also
check the UUID. If not both match, go back further until you find a
timeline where both the TLI and the timeline UUID matches and do the usual
fandango to find the good LSN to rewind to.

[1]: https://github.com/mkindahl/tla-postgres

Here is an updated version of the patch. It seems like it is not necessary
to extend the XLOG_END_OF_RECOVERY record with the UUID, just the history
files. The scenario is still the same though, and can trigger diverging
servers, possibly silent. I have an additional test case using a divergence
going back three promotions.
--
Best wishes,
Mats Kindahl, Multigres Developer, Supabase

Hi Mats,

Thanks for picking this up -- the scenario is a real one and I think the
UUID-tagging approach is a clean way to solve it. v2 applies and builds
without trouble, and the core algorithm reads well to me.
I have a handful of observations that I'd love your thoughts.

Regarding Correctness I have the below thoughts

1. UUIDv7 timestamp epoch.
In StartupXLOG():
TimestampTz now = GetCurrentTimestamp();
generate_uuidv7_r(&uuid_buf, (uint64)(now / 1000),
(uint32)(now % 1000) * 1000);

I think there might be a small mismatch here: GetCurrentTimestamp() returns
microseconds since the Postgres epoch (2000-01-01),
whereas generate_uuidv7_r describes its first argument as milliseconds
since the Unix epoch.
As written that 30-year offset would land in the UUID's timestamp field, so
the resulting UUID wouldn't be a conformant UUIDv7 and wouldn't
time-order against UUIDv7s generated through the SQL functions.

Uniqueness is preserved either way, so the rewind logic still works as
intended but it seemed worth flagging.

I see conversion that's used elsewhere as:
us = ts + (POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE)
* SECS_PER_DAY * USECS_PER_SEC;

Or, since promotion isn't on a hot path, gettimeofday() / time(NULL)
directly would also be fine.

2. EOR-record path, the intent is unclear.

The comment above generate_uuidv7_r() at says:

"The same UUID is written into the history file and later into the
XLOG_END_OF_RECOVERY record so that pg_rewind can distinguish two
servers..."

But from what I can see only the history-file part actually lands.
xl_end_of_recovery is unchanged, CreateEndOfRecoveryRecord() doesn't add
the UUID, and XLogCtl->ThisTimeLineUUID is written under info_lck without a
reader (I couldn't grep it).

The xlog_redo() memset() + Min(rec_len, sizeof(...)) change reads like
preparation for an EOR-struct extension that ended up not being part of the
patch.

Was the EOR-record piece something you intended to keep for a follow-up, or
has it been superseded by the history-file approach?

3. Malformed UUID handling in readTimeLineHistory().

The optional field-4 path is:

if (nfields == 4 && strlen(uuid_str) == UUID_STR_LEN)
{
Datum datum = DirectFunctionCall1(uuid_in,
CStringGetDatum(uuid_str));
...
}

uuid_in() raises ereport(ERROR) on a malformed input, while the surrounding
syntax-error paths in readTimeLineHistory() use FATAL deliberately.
In practice an ERROR during startup ends up being fatal too, so this isn't
strictly a bug but it would be nicer to stay consistent.

Regarding the Tests I have the following thoughts

The two new cases are nice, a few extensions that I think would strengthen
them:
1. A mixed-version case where one side has a zero UUID. That's the path
we're claiming is graceful, but nothing currently exercises it
2. A deeper-divergence case (e.g. TLI1->2->3 vs TLI1->2->3') so that
findCommonAncestorTimeline's loop walks past matching entries
before hitting the mismatch. The 0002 test puts the divergence at
depth 1.
3. A small assertion against the on-disk 00000002.history contents, to pin
down the file format.
4. On 0002 the dependency on restore_command pointing at node_x's pg_wal is
the kind of thing that tends to break under
environment changes. A CHECKPOINT on node_x before the backup, or
wal_keep_size as in 0001, would let the test stand on its own.

I'm happy to keep reviewing/contributing, thanks again for working on it.

Regards,
Surya Poondla

On Fri, May 22, 2026 at 12:09 AM surya poondla <suryapoondla4@gmail.com>
wrote:

Hi Mats,

Thanks for picking this up -- the scenario is a real one and I think the
UUID-tagging approach is a clean way to solve it. v2 applies and builds
without trouble, and the core algorithm reads well to me.
I have a handful of observations that I'd love your thoughts.

Hi Surya,

Thank you for the review. It is a quite rare scenario, but it is real and
the fix is simple.

Regarding Correctness I have the below thoughts

1. UUIDv7 timestamp epoch.
In StartupXLOG():
TimestampTz now = GetCurrentTimestamp();
generate_uuidv7_r(&uuid_buf, (uint64)(now / 1000),
(uint32)(now % 1000) * 1000);

I think there might be a small mismatch here: GetCurrentTimestamp()
returns microseconds since the Postgres epoch (2000-01-01),
whereas generate_uuidv7_r describes its first argument as milliseconds
since the Unix epoch.
As written that 30-year offset would land in the UUID's timestamp field,
so the resulting UUID wouldn't be a conformant UUIDv7 and wouldn't
time-order against UUIDv7s generated through the SQL functions.

Uniqueness is preserved either way, so the rewind logic still works as
intended but it seemed worth flagging.

I see conversion that's used elsewhere as:
us = ts + (POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE)
* SECS_PER_DAY * USECS_PER_SEC;

Or, since promotion isn't on a hot path, gettimeofday() / time(NULL)
directly would also be fine.

Yes, the intention was to use a proper timestamp to allow debugging servers
if necessary. Switched to gettimeofday() and used 0 for sub-ms since this
is not going to be critical. (We could use ns here as well, but that would
only solve a race if you have two servers being promoted in the same ms,
which I find unlikely, and there is a random number added for that
situation.)

2. EOR-record path, the intent is unclear.

The comment above generate_uuidv7_r() at says:

"The same UUID is written into the history file and later into the
XLOG_END_OF_RECOVERY record so that pg_rewind can distinguish two
servers..."

But from what I can see only the history-file part actually lands.
xl_end_of_recovery is unchanged, CreateEndOfRecoveryRecord() doesn't add
the UUID, and XLogCtl->ThisTimeLineUUID is written under info_lck without a
reader (I couldn't grep it).

The xlog_redo() memset() + Min(rec_len, sizeof(...)) change reads like
preparation for an EOR-struct extension that ended up not being part of the
patch.

Was the EOR-record piece something you intended to keep for a follow-up,
or has it been superseded by the history-file approach?

No, the EOR changes are not needed for the promotion, contrary to what I
originally thought. Cleaned up the comment and the code and removed all
traces of changes to the EOR (I hope).

3. Malformed UUID handling in readTimeLineHistory().

The optional field-4 path is:

if (nfields == 4 && strlen(uuid_str) == UUID_STR_LEN)
{
Datum datum = DirectFunctionCall1(uuid_in,
CStringGetDatum(uuid_str));
...
}

uuid_in() raises ereport(ERROR) on a malformed input, while the
surrounding syntax-error paths in readTimeLineHistory() use FATAL
deliberately.
In practice an ERROR during startup ends up being fatal too, so this isn't
strictly a bug but it would be nicer to stay consistent.

Agree. I added code to capture the error and raise a FATAL instead (with
the error message from the uuid_in, in case it is modified it makes sense
to show this).

Regarding the Tests I have the following thoughts

The two new cases are nice, a few extensions that I think would strengthen
them:
1. A mixed-version case where one side has a zero UUID. That's the path
we're claiming is graceful, but nothing currently exercises it

Yes, that should work regardless of whether the source or the target has
the zero UUID.

I realized one thing: if two timelines have identical TLI but one has zero
UUID and one has not, it seems they could not come from the same promotion
(one promotion happened on an old server and the other one on a new
server), that is, they should be treated as different. Does that make
sense? I made the necessary changes in the attached patches for testing.
Please have a look.

2. A deeper-divergence case (e.g. TLI1->2->3 vs TLI1->2->3') so that
findCommonAncestorTimeline's loop walks past matching entries
before hitting the mismatch. The 0002 test puts the divergence at
depth 1.

I was unsure if this test was necessary or interesting, hence a separate
commit. Since you thought it was useful, it's now rolled into the patch and
I extended the tests with the scenarios you suggested.

I also did some refactorings of the tests to avoid duplication. More below.

3. A small assertion against the on-disk 00000002.history contents, to pin
down the file format.
4. On 0002 the dependency on restore_command pointing at node_x's pg_wal
is the kind of thing that tends to break under
environment changes. A CHECKPOINT on node_x before the backup, or
wal_keep_size as in 0001, would let the test stand on its own.

Good point.

I refactored the code to avoid some duplication and make the test flow
self-explanatory and as part of that I set the wal_keep_size for all nodes.

In the process I noticed that many of the functions in RewindTest.pm do the
same job as the primitives I wrote, but have hard-coded variable names. I
could rewrite them to take parameters, but that would be quite a big patch
to add additional changes to each call site, so I did not do that and
rather added small wrappers specific for the tests in 005_same_timeline.pl.

Attached a new version of the now single patch.

I'm happy to keep reviewing/contributing, thanks again for working on it.

Thank you for reviewing it.
--
Best wishes,
Mats Kindahl, Multigres Developer, Supabase

Hi, Mats

On Sun, 24 May 2026 at 20:30, Mats Kindahl <mats.kindahl@gmail.com> wrote:

On Fri, May 22, 2026 at 12:09 AM surya poondla <suryapoondla4@gmail.com> wrote:

Hi Mats,

Thanks for picking this up -- the scenario is a real one and I think the UUID-tagging approach is a clean way to
solve it. v2 applies and builds without trouble, and the core algorithm reads well to me.
I have a handful of observations that I'd love your thoughts.

Hi Surya,

Thank you for the review. It is a quite rare scenario, but it is real and the fix is simple.

Regarding Correctness I have the below thoughts

1. UUIDv7 timestamp epoch.
In StartupXLOG():
TimestampTz now = GetCurrentTimestamp();
generate_uuidv7_r(&uuid_buf, (uint64)(now / 1000),
(uint32)(now % 1000) * 1000);

I think there might be a small mismatch here: GetCurrentTimestamp() returns microseconds since the Postgres epoch
(2000-01-01),
whereas generate_uuidv7_r describes its first argument as milliseconds since the Unix epoch.
As written that 30-year offset would land in the UUID's timestamp field, so the resulting UUID wouldn't be a
conformant UUIDv7 and wouldn't
time-order against UUIDv7s generated through the SQL functions.

Uniqueness is preserved either way, so the rewind logic still works as intended but it seemed worth flagging.

I see conversion that's used elsewhere as:
us = ts + (POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE)
* SECS_PER_DAY * USECS_PER_SEC;

Or, since promotion isn't on a hot path, gettimeofday() / time(NULL) directly would also be fine.

Yes, the intention was to use a proper timestamp to allow debugging servers if necessary. Switched to gettimeofday() and
used 0 for sub-ms since this is not going to be critical. (We could use ns here as well, but that would only solve a race
if you have two servers being promoted in the same ms, which I find unlikely, and there is a random number added for that
situation.)

2. EOR-record path, the intent is unclear.

The comment above generate_uuidv7_r() at says:

"The same UUID is written into the history file and later into the XLOG_END_OF_RECOVERY record so that pg_rewind can
distinguish two servers..."

But from what I can see only the history-file part actually lands.
xl_end_of_recovery is unchanged, CreateEndOfRecoveryRecord() doesn't add the UUID, and XLogCtl->ThisTimeLineUUID is
written under info_lck without a
reader (I couldn't grep it).

The xlog_redo() memset() + Min(rec_len, sizeof(...)) change reads like preparation for an EOR-struct extension that
ended up not being part of the patch.

Was the EOR-record piece something you intended to keep for a follow-up, or has it been superseded by the
history-file approach?

No, the EOR changes are not needed for the promotion, contrary to what I originally thought. Cleaned up the comment and
the code and removed all traces of changes to the EOR (I hope).

3. Malformed UUID handling in readTimeLineHistory().

The optional field-4 path is:

if (nfields == 4 && strlen(uuid_str) == UUID_STR_LEN)
{
Datum datum = DirectFunctionCall1(uuid_in,
CStringGetDatum(uuid_str));
...
}

uuid_in() raises ereport(ERROR) on a malformed input, while the surrounding syntax-error paths in readTimeLineHistory
() use FATAL deliberately.
In practice an ERROR during startup ends up being fatal too, so this isn't strictly a bug but it would be nicer to
stay consistent.

Agree. I added code to capture the error and raise a FATAL instead (with the error message from the uuid_in, in case it
is modified it makes sense to show this).

Regarding the Tests I have the following thoughts

The two new cases are nice, a few extensions that I think would strengthen them:
1. A mixed-version case where one side has a zero UUID. That's the path we're claiming is graceful, but nothing
currently exercises it

Yes, that should work regardless of whether the source or the target has the zero UUID.

I realized one thing: if two timelines have identical TLI but one has zero UUID and one has not, it seems they could not
come from the same promotion (one promotion happened on an old server and the other one on a new server), that is, they
should be treated as different. Does that make sense? I made the necessary changes in the attached patches for testing.
Please have a look.

2. A deeper-divergence case (e.g. TLI1->2->3 vs TLI1->2->3') so that findCommonAncestorTimeline's loop walks past
matching entries
before hitting the mismatch. The 0002 test puts the divergence at depth 1.

I was unsure if this test was necessary or interesting, hence a separate commit. Since you thought it was useful, it's
now rolled into the patch and I extended the tests with the scenarios you suggested.

I also did some refactorings of the tests to avoid duplication. More below.

3. A small assertion against the on-disk 00000002.history contents, to pin down the file format.
4. On 0002 the dependency on restore_command pointing at node_x's pg_wal is the kind of thing that tends to break
under
environment changes. A CHECKPOINT on node_x before the backup, or wal_keep_size as in 0001, would let the test
stand on its own.

Good point.

I refactored the code to avoid some duplication and make the test flow self-explanatory and as part of that I set the
wal_keep_size for all nodes.

In the process I noticed that many of the functions in RewindTest.pm do the same job as the primitives I wrote, but have
hard-coded variable names. I could rewrite them to take parameters, but that would be quite a big patch to add additional
changes to each call site, so I did not do that and rather added small wrappers specific for the tests in
005_same_timeline.pl⚠️.

Attached a new version of the now single patch.

I'm happy to keep reviewing/contributing, thanks again for working on it.

Thank you for reviewing it.

Thank you for your work. I have one comment.

+	a = &tlh->source[tlh->sourceNentries - 2].tluuid;
+	b = &tlh->target[tlh->targetNentries - 2].tluuid;
+
+	if (memcmp(a, &zero, UUID_LEN) == 0 && memcmp(b, &zero, UUID_LEN) == 0)
+		return true;
+
+	return memcmp(a, b, UUID_LEN) == 0;

Since we already have matchingTimelineUUID(), the above code can be simplified
using it.

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Hi Japin,

On Mon, May 25, 2026 at 7:21 AM Japin Li <japinli@hotmail.com> wrote:

Hi, Mats

On Sun, 24 May 2026 at 20:30, Mats Kindahl <mats.kindahl@gmail.com> wrote:

On Fri, May 22, 2026 at 12:09 AM surya poondla <suryapoondla4@gmail.com>

wrote:

Hi Mats,

Thanks for picking this up -- the scenario is a real one and I think

the UUID-tagging approach is a clean way to

solve it. v2 applies and builds without trouble, and the core algorithm

reads well to me.

I have a handful of observations that I'd love your thoughts.

Hi Surya,

Thank you for the review. It is a quite rare scenario, but it is real

and the fix is simple.

Regarding Correctness I have the below thoughts

1. UUIDv7 timestamp epoch.
In StartupXLOG():
TimestampTz now = GetCurrentTimestamp();
generate_uuidv7_r(&uuid_buf, (uint64)(now / 1000),
(uint32)(now % 1000) * 1000);

I think there might be a small mismatch here: GetCurrentTimestamp()

returns microseconds since the Postgres epoch

(2000-01-01),
whereas generate_uuidv7_r describes its first argument as milliseconds

since the Unix epoch.

As written that 30-year offset would land in the UUID's timestamp

field, so the resulting UUID wouldn't be a

conformant UUIDv7 and wouldn't
time-order against UUIDv7s generated through the SQL functions.

Uniqueness is preserved either way, so the rewind logic still works as

intended but it seemed worth flagging.

I see conversion that's used elsewhere as:
us = ts + (POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE)
* SECS_PER_DAY * USECS_PER_SEC;

Or, since promotion isn't on a hot path, gettimeofday() / time(NULL)

directly would also be fine.

Yes, the intention was to use a proper timestamp to allow debugging

servers if necessary. Switched to gettimeofday() and

used 0 for sub-ms since this is not going to be critical. (We could use

ns here as well, but that would only solve a race

if you have two servers being promoted in the same ms, which I find

unlikely, and there is a random number added for that

situation.)

2. EOR-record path, the intent is unclear.

The comment above generate_uuidv7_r() at says:

"The same UUID is written into the history file and later into the

XLOG_END_OF_RECOVERY record so that pg_rewind can

distinguish two servers..."

But from what I can see only the history-file part actually lands.
xl_end_of_recovery is unchanged, CreateEndOfRecoveryRecord() doesn't

add the UUID, and XLogCtl->ThisTimeLineUUID is

written under info_lck without a
reader (I couldn't grep it).

The xlog_redo() memset() + Min(rec_len, sizeof(...)) change reads like

preparation for an EOR-struct extension that

ended up not being part of the patch.

Was the EOR-record piece something you intended to keep for a

follow-up, or has it been superseded by the

history-file approach?

No, the EOR changes are not needed for the promotion, contrary to what I

originally thought. Cleaned up the comment and

the code and removed all traces of changes to the EOR (I hope).

3. Malformed UUID handling in readTimeLineHistory().

The optional field-4 path is:

if (nfields == 4 && strlen(uuid_str) == UUID_STR_LEN)
{
Datum datum = DirectFunctionCall1(uuid_in,

CStringGetDatum(uuid_str));

...
}

uuid_in() raises ereport(ERROR) on a malformed input, while the

surrounding syntax-error paths in readTimeLineHistory

() use FATAL deliberately.
In practice an ERROR during startup ends up being fatal too, so this

isn't strictly a bug but it would be nicer to

stay consistent.

Agree. I added code to capture the error and raise a FATAL instead (with

the error message from the uuid_in, in case it

is modified it makes sense to show this).

Regarding the Tests I have the following thoughts

The two new cases are nice, a few extensions that I think would

strengthen them:

1. A mixed-version case where one side has a zero UUID. That's the path

we're claiming is graceful, but nothing

currently exercises it

Yes, that should work regardless of whether the source or the target has

the zero UUID.

I realized one thing: if two timelines have identical TLI but one has

zero UUID and one has not, it seems they could not

come from the same promotion (one promotion happened on an old server

and the other one on a new server), that is, they

should be treated as different. Does that make sense? I made the

necessary changes in the attached patches for testing.

Please have a look.

2. A deeper-divergence case (e.g. TLI1->2->3 vs TLI1->2->3') so that

findCommonAncestorTimeline's loop walks past

matching entries
before hitting the mismatch. The 0002 test puts the divergence at

depth 1.

I was unsure if this test was necessary or interesting, hence a separate

commit. Since you thought it was useful, it's

now rolled into the patch and I extended the tests with the scenarios

you suggested.

I also did some refactorings of the tests to avoid duplication. More

below.

3. A small assertion against the on-disk 00000002.history contents, to

pin down the file format.

4. On 0002 the dependency on restore_command pointing at node_x's

pg_wal is the kind of thing that tends to break

under
environment changes. A CHECKPOINT on node_x before the backup, or

wal_keep_size as in 0001, would let the test

stand on its own.

Good point.

I refactored the code to avoid some duplication and make the test flow

self-explanatory and as part of that I set the

wal_keep_size for all nodes.

In the process I noticed that many of the functions in RewindTest.pm do

the same job as the primitives I wrote, but have

hard-coded variable names. I could rewrite them to take parameters, but

that would be quite a big patch to add additional

changes to each call site, so I did not do that and rather added small

wrappers specific for the tests in

005_same_timeline.pl⚠️.

Attached a new version of the now single patch.

I'm happy to keep reviewing/contributing, thanks again for working on

it.

Thank you for reviewing it.

Thank you for your work. I have one comment.

+       a = &tlh->source[tlh->sourceNentries - 2].tluuid;
+       b = &tlh->target[tlh->targetNentries - 2].tluuid;
+
+       if (memcmp(a, &zero, UUID_LEN) == 0 && memcmp(b, &zero, UUID_LEN)
== 0)
+               return true;
+
+       return memcmp(a, b, UUID_LEN) == 0;

Since we already have matchingTimelineUUID(), the above code can be
simplified
using it.

Thank you for the review. I switched to using the matchingTimelineUUID()
for this part of the code and made some other minor improvements as well.
--
Best wishes,
Mats Kindahl, Multigres Developer, Supabase

Hi, Mats

Thanks for updating the patch.

On Mon, 25 May 2026 at 20:59, Mats Kindahl <mats.kindahl@gmail.com> wrote:

Hi Japin,

On Mon, May 25, 2026 at 7:21 AM Japin Li <japinli@hotmail.com> wrote:

Hi, Mats

On Sun, 24 May 2026 at 20:30, Mats Kindahl <mats.kindahl@gmail.com> wrote:

On Fri, May 22, 2026 at 12:09 AM surya poondla <suryapoondla4@gmail.com> wrote:

Hi Mats,

Thanks for picking this up -- the scenario is a real one and I think the UUID-tagging approach is a clean way to
solve it. v2 applies and builds without trouble, and the core algorithm reads well to me.
I have a handful of observations that I'd love your thoughts.

Hi Surya,

Thank you for the review. It is a quite rare scenario, but it is real and the fix is simple.

Regarding Correctness I have the below thoughts

1. UUIDv7 timestamp epoch.
In StartupXLOG():
TimestampTz now = GetCurrentTimestamp();
generate_uuidv7_r(&uuid_buf, (uint64)(now / 1000),
(uint32)(now % 1000) * 1000);

I think there might be a small mismatch here: GetCurrentTimestamp() returns microseconds since the Postgres epoch
(2000-01-01),
whereas generate_uuidv7_r describes its first argument as milliseconds since the Unix epoch.
As written that 30-year offset would land in the UUID's timestamp field, so the resulting UUID wouldn't be a
conformant UUIDv7 and wouldn't
time-order against UUIDv7s generated through the SQL functions.

Uniqueness is preserved either way, so the rewind logic still works as intended but it seemed worth flagging.

I see conversion that's used elsewhere as:
us = ts + (POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE)
* SECS_PER_DAY * USECS_PER_SEC;

Or, since promotion isn't on a hot path, gettimeofday() / time(NULL) directly would also be fine.

Yes, the intention was to use a proper timestamp to allow debugging servers if necessary. Switched to gettimeofday

() and

used 0 for sub-ms since this is not going to be critical. (We could use ns here as well, but that would only solve

a race

if you have two servers being promoted in the same ms, which I find unlikely, and there is a random number added

for that

situation.)

2. EOR-record path, the intent is unclear.

The comment above generate_uuidv7_r() at says:

"The same UUID is written into the history file and later into the XLOG_END_OF_RECOVERY record so that pg_rewind

can

distinguish two servers..."

But from what I can see only the history-file part actually lands.
xl_end_of_recovery is unchanged, CreateEndOfRecoveryRecord() doesn't add the UUID, and XLogCtl->ThisTimeLineUUID

is

written under info_lck without a
reader (I couldn't grep it).

The xlog_redo() memset() + Min(rec_len, sizeof(...)) change reads like preparation for an EOR-struct extension

that

ended up not being part of the patch.

Was the EOR-record piece something you intended to keep for a follow-up, or has it been superseded by the
history-file approach?

No, the EOR changes are not needed for the promotion, contrary to what I originally thought. Cleaned up the comment

and

the code and removed all traces of changes to the EOR (I hope).

3. Malformed UUID handling in readTimeLineHistory().

The optional field-4 path is:

if (nfields == 4 && strlen(uuid_str) == UUID_STR_LEN)
{
Datum datum = DirectFunctionCall1(uuid_in,
CStringGetDatum(uuid_str));
...
}

uuid_in() raises ereport(ERROR) on a malformed input, while the surrounding syntax-error paths in

readTimeLineHistory

() use FATAL deliberately.
In practice an ERROR during startup ends up being fatal too, so this isn't strictly a bug but it would be nicer to
stay consistent.

Agree. I added code to capture the error and raise a FATAL instead (with the error message from the uuid_in, in

case it

is modified it makes sense to show this).

Regarding the Tests I have the following thoughts

The two new cases are nice, a few extensions that I think would strengthen them:
1. A mixed-version case where one side has a zero UUID. That's the path we're claiming is graceful, but nothing
currently exercises it

Yes, that should work regardless of whether the source or the target has the zero UUID.

I realized one thing: if two timelines have identical TLI but one has zero UUID and one has not, it seems they

could not

come from the same promotion (one promotion happened on an old server and the other one on a new server), that is,

they

should be treated as different. Does that make sense? I made the necessary changes in the attached patches for

testing.

Please have a look.

2. A deeper-divergence case (e.g. TLI1->2->3 vs TLI1->2->3') so that findCommonAncestorTimeline's loop walks past
matching entries
before hitting the mismatch. The 0002 test puts the divergence at depth 1.

I was unsure if this test was necessary or interesting, hence a separate commit. Since you thought it was useful,

it's

now rolled into the patch and I extended the tests with the scenarios you suggested.

I also did some refactorings of the tests to avoid duplication. More below.

3. A small assertion against the on-disk 00000002.history contents, to pin down the file format.
4. On 0002 the dependency on restore_command pointing at node_x's pg_wal is the kind of thing that tends to break
under
environment changes. A CHECKPOINT on node_x before the backup, or wal_keep_size as in 0001, would let the

test

stand on its own.

Good point.

I refactored the code to avoid some duplication and make the test flow self-explanatory and as part of that I set

the

wal_keep_size for all nodes.

In the process I noticed that many of the functions in RewindTest.pm do the same job as the primitives I wrote, but

have

hard-coded variable names. I could rewrite them to take parameters, but that would be quite a big patch to add

additional

changes to each call site, so I did not do that and rather added small wrappers specific for the tests in
005_same_timeline.pl⚠️⚠️.

Attached a new version of the now single patch.

I'm happy to keep reviewing/contributing, thanks again for working on it.

Thank you for reviewing it.

Thank you for your work. I have one comment.

+       a = &tlh->source[tlh->sourceNentries - 2].tluuid;
+       b = &tlh->target[tlh->targetNentries - 2].tluuid;
+
+       if (memcmp(a, &zero, UUID_LEN) == 0 && memcmp(b, &zero, UUID_LEN) == 0)
+               return true;
+
+       return memcmp(a, b, UUID_LEN) == 0;

Since we already have matchingTimelineUUID(), the above code can be simplified
using it.

Thank you for the review. I switched to using the matchingTimelineUUID() for this part of the code and made some other
minor improvements as well.

Here are some comments on v4.

1.
+/*
+ * Timeline histories for both clusters, populated by timelines_match().
+ */

I don't see a timelines_match() function. Does this refer to
matchAndFetchTimelines()?

2.
+typedef struct TimelineHistoriesData
+{
+       TimeLineHistoryEntry *source,
+                          *target;
+       int                     sourceNentries,
+                               targetNentries;
+}                      TimelineHistoriesData;

I'd prefer to use TimeLineHistoriesData to stay consistent with
TimeLineHistoryEntry. Anyway I'm not instant on it.

3.
+typedef TimelineHistoriesData * TimelineHistories;

The space between * and TimelineHistories is unnecessary — see
StringInfoData and other typedefs.

4.
+# node_x and node_b both start from the same TLI 1 baseline.
+my ($node_x, $node_b2) =
+  setup_standbys_from_origin($node_origin2, 'node_x', 'node_b2');

There appears to be a typo in the comment. The node_b should be node_b2.

Everything else looks good. Thank you again for updating the patch!

--
Best wishes,
Mats Kindahl, Multigres Developer, Supabase

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Hi Japin,

Thanks for reviewing the patch.

On Tue, May 26, 2026 at 8:56 AM Japin Li <japinli@hotmail.com> wrote:

Hi, Mats

Thanks for updating the patch.

On Mon, 25 May 2026 at 20:59, Mats Kindahl <mats.kindahl@gmail.com> wrote:

Hi Japin,

On Mon, May 25, 2026 at 7:21 AM Japin Li <japinli@hotmail.com> wrote:

Hi, Mats

On Sun, 24 May 2026 at 20:30, Mats Kindahl <mats.kindahl@gmail.com>

wrote:

On Fri, May 22, 2026 at 12:09 AM surya poondla <

suryapoondla4@gmail.com> wrote:

Hi Mats,

Thanks for picking this up -- the scenario is a real one and I think

the UUID-tagging approach is a clean way to

solve it. v2 applies and builds without trouble, and the core

algorithm reads well to me.

I have a handful of observations that I'd love your thoughts.

Hi Surya,

Thank you for the review. It is a quite rare scenario, but it is real

and the fix is simple.

Regarding Correctness I have the below thoughts

1. UUIDv7 timestamp epoch.
In StartupXLOG():
TimestampTz now = GetCurrentTimestamp();
generate_uuidv7_r(&uuid_buf, (uint64)(now / 1000),
(uint32)(now % 1000) * 1000);

I think there might be a small mismatch here: GetCurrentTimestamp()

returns microseconds since the Postgres epoch

(2000-01-01),
whereas generate_uuidv7_r describes its first argument as

milliseconds since the Unix epoch.

As written that 30-year offset would land in the UUID's timestamp

field, so the resulting UUID wouldn't be a

conformant UUIDv7 and wouldn't
time-order against UUIDv7s generated through the SQL functions.

Uniqueness is preserved either way, so the rewind logic still works

as intended but it seemed worth flagging.

I see conversion that's used elsewhere as:
us = ts + (POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE)
* SECS_PER_DAY * USECS_PER_SEC;

Or, since promotion isn't on a hot path, gettimeofday() / time(NULL)

directly would also be fine.

Yes, the intention was to use a proper timestamp to allow debugging

servers if necessary. Switched to gettimeofday

() and

used 0 for sub-ms since this is not going to be critical. (We could

use ns here as well, but that would only solve

a race

if you have two servers being promoted in the same ms, which I find

unlikely, and there is a random number added

for that

situation.)

2. EOR-record path, the intent is unclear.

The comment above generate_uuidv7_r() at says:

"The same UUID is written into the history file and later into the

XLOG_END_OF_RECOVERY record so that pg_rewind

can

distinguish two servers..."

But from what I can see only the history-file part actually lands.
xl_end_of_recovery is unchanged, CreateEndOfRecoveryRecord() doesn't

add the UUID, and XLogCtl->ThisTimeLineUUID

is

written under info_lck without a
reader (I couldn't grep it).

The xlog_redo() memset() + Min(rec_len, sizeof(...)) change reads

like preparation for an EOR-struct extension

that

ended up not being part of the patch.

Was the EOR-record piece something you intended to keep for a

follow-up, or has it been superseded by the

history-file approach?

No, the EOR changes are not needed for the promotion, contrary to

what I originally thought. Cleaned up the comment

and

the code and removed all traces of changes to the EOR (I hope).

3. Malformed UUID handling in readTimeLineHistory().

The optional field-4 path is:

if (nfields == 4 && strlen(uuid_str) == UUID_STR_LEN)
{
Datum datum = DirectFunctionCall1(uuid_in,

CStringGetDatum(uuid_str));

...
}

uuid_in() raises ereport(ERROR) on a malformed input, while the

surrounding syntax-error paths in

readTimeLineHistory

() use FATAL deliberately.
In practice an ERROR during startup ends up being fatal too, so this

isn't strictly a bug but it would be nicer to

stay consistent.

Agree. I added code to capture the error and raise a FATAL instead

(with the error message from the uuid_in, in

case it

is modified it makes sense to show this).

Regarding the Tests I have the following thoughts

The two new cases are nice, a few extensions that I think would

strengthen them:

1. A mixed-version case where one side has a zero UUID. That's the

path we're claiming is graceful, but nothing

currently exercises it

Yes, that should work regardless of whether the source or the target

has the zero UUID.

I realized one thing: if two timelines have identical TLI but one has

zero UUID and one has not, it seems they

could not

come from the same promotion (one promotion happened on an old server

and the other one on a new server), that is,

they

should be treated as different. Does that make sense? I made the

necessary changes in the attached patches for

testing.

Please have a look.

2. A deeper-divergence case (e.g. TLI1->2->3 vs TLI1->2->3') so that

findCommonAncestorTimeline's loop walks past

matching entries
before hitting the mismatch. The 0002 test puts the divergence

at depth 1.

I was unsure if this test was necessary or interesting, hence a

separate commit. Since you thought it was useful,

it's

now rolled into the patch and I extended the tests with the scenarios

you suggested.

I also did some refactorings of the tests to avoid duplication. More

below.

3. A small assertion against the on-disk 00000002.history contents,

to pin down the file format.

4. On 0002 the dependency on restore_command pointing at node_x's

pg_wal is the kind of thing that tends to break

under
environment changes. A CHECKPOINT on node_x before the backup,

or wal_keep_size as in 0001, would let the

test

stand on its own.

Good point.

I refactored the code to avoid some duplication and make the test

flow self-explanatory and as part of that I set

the

wal_keep_size for all nodes.

In the process I noticed that many of the functions in RewindTest.pm

do the same job as the primitives I wrote, but

have

hard-coded variable names. I could rewrite them to take parameters,

but that would be quite a big patch to add

additional

changes to each call site, so I did not do that and rather added

small wrappers specific for the tests in

005_same_timeline.pl⚠️⚠️.

Attached a new version of the now single patch.

I'm happy to keep reviewing/contributing, thanks again for working

on it.

Thank you for reviewing it.

Thank you for your work. I have one comment.

+       a = &tlh->source[tlh->sourceNentries - 2].tluuid;
+       b = &tlh->target[tlh->targetNentries - 2].tluuid;
+
+       if (memcmp(a, &zero, UUID_LEN) == 0 && memcmp(b, &zero,

UUID_LEN) == 0)

+               return true;
+
+       return memcmp(a, b, UUID_LEN) == 0;

Since we already have matchingTimelineUUID(), the above code can be

simplified

using it.

Thank you for the review. I switched to using the matchingTimelineUUID()

for this part of the code and made some other

minor improvements as well.

Here are some comments on v4.

1.
+/*
+ * Timeline histories for both clusters, populated by timelines_match().
+ */

I don't see a timelines_match() function. Does this refer to
matchAndFetchTimelines()?

Correct. Updated.

2.
+typedef struct TimelineHistoriesData
+{
+       TimeLineHistoryEntry *source,
+                          *target;
+       int                     sourceNentries,
+                               targetNentries;
+}                      TimelineHistoriesData;

I'd prefer to use TimeLineHistoriesData to stay consistent with
TimeLineHistoryEntry. Anyway I'm not instant on it.

Makes sense to be consistent. Updated.

3.
+typedef TimelineHistoriesData * TimelineHistories;

The space between * and TimelineHistories is unnecessary — see
StringInfoData and other typedefs.

My mistake. FIxed.

4.
+# node_x and node_b both start from the same TLI 1 baseline.
+my ($node_x, $node_b2) =
+  setup_standbys_from_origin($node_origin2, 'node_x', 'node_b2');

There appears to be a typo in the comment. The node_b should be node_b2.

Right. Fixed.

Everything else looks good. Thank you again for updating the patch!

Thank you again for reviewing the patch. :)

Attached a new version of the patch with the changes you suggested.

--
Best wishes,
Mats Kindahl, Multigres Developer, Supabase

Hi, Mats

On Tue, 26 May 2026 at 18:03, Mats Kindahl <mats.kindahl@gmail.com> wrote:

Attached a new version of the patch with the changes you suggested.

I found an error on the Windows platform [1]https://cirrus-ci.com/task/6228217159221248.

[07:08:28.538] >>> MALLOC_PERTURB_=168 PG_REGRESS=C:\cirrus\build\src/test\regress\pg_regress.exe REGRESS_SHLIB=C:\cirrus\build\src/test\regress\regress.dll MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 top_builddir=C:\cirrus\build UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MESON_TEST_ITERATION=1 PATH=C:\cirrus\build\tmp_install\usr\local\pgsql\bin;C:\cirrus\build\src\bin\pg_rewind;C:/cirrus/build/src/bin/pg_rewind/test;C:\VS_2019\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64;C:\VS_2019\MSBuild\Current\bin\Roslyn;C:\Program Files (x86)\Windows Kits\10\bin\10.0.22621.0\x64;C:\Program Files (x86)\Windows Kits\10\bin\x64;C:\VS_2019\\MSBuild\Current\Bin;C:\Windows\Microsoft.NET\Framework64\v4.0.30319;C:\VS_2019\Common7\IDE\;C:\VS_2019\Common7\Tools\;C:\VS_2019\VC\Auxiliary\Build;C:\zstd\zstd-v1.5.2-win64;C:\zlib;C:\lz4;C:\icu;C:\winflexbison;C:\strawberry\5.42.0.1\perl\bin;C:\python\Scripts\;C:\python\;C:\Windows Kits\10\Debuggers\x64;C:\Program Files\Git\usr\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\ProgramData\GooGet;C:\Program Files\Google\Compute Engine\metadata_scripts;C:\Program Files (x86)\Google\Cloud SDK\google-cloud-sdk\bin;C:\Program Files\PowerShell\7\;C:\Program Files\Google\Compute Engine\sysprep;C:\ProgramData\chocolatey\bin;C:\Program Files\Git\cmd;C:\Program Files\Git\mingw64\bin;C:\Program Files\Git\usr\bin;C:\Windows\system32\config\systemprofile\AppData\Local\Microsoft\WindowsApps INITDB_TEMPLATE=C:/cirrus/build/tmp_install/initdb-template ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 share_contrib_dir=C:/cirrus/build/tmp_install//usr/local/pgsql/share/contrib C:\python\python3.EXE C:\cirrus\build\..\src/tools/testwrap --basedir C:\cirrus\build --srcdir C:\cirrus\src\bin\pg_rewind --pg-test-extra --testgroup pg_rewind --testname 005_same_timeline -- C:\strawberry\5.42.0.1\perl\bin\perl.EXE -I C:/cirrus/src/test/perl -I C:\cirrus\src\bin\pg_rewind C:/cirrus/src/bin/pg_rewind/t/005_same_timeline.pl
[07:08:28.538] ------------------------------------- 8< -------------------------------------
[07:08:28.538] stderr:
[07:08:28.538] # Failed test 'pg_rewind rewinds across mismatched TLI 2 / TLI 2-prime to TLI 1'
[07:08:28.538] # at C:/cirrus/src/bin/pg_rewind/t/005_same_timeline.pl line 45.
[07:08:28.538] # ---------- command failed ----------
[07:08:28.538] # pg_rewind --debug --source-pgdata C:\cirrus\build/testrun/pg_rewind/005_same_timeline\data/t_005_same_timeline_node_b2_data/pgdata --target-pgdata C:\cirrus\build/testrun/pg_rewind/005_same_timeline\data/t_005_same_timeline_node_a2_data/pgdata --no-sync --config-file C:\cirrus\build\testrun\pg_rewind\005_same_timeline\data\tmp_test_ZCeZ/target-postgresql.conf.tmp --restore-target-wal
[07:08:28.538] # -------------- stderr --------------
[07:08:28.538] # pg_rewind: using for rewind "restore_command = 'cp "C:cirrusuild/testrun/pg_rewind/005_same_timelinedata/t_005_same_timeline_node_x_data/pgdata/pg_wal/%f" "%p"'"
[07:08:28.538] # pg_rewind: Source timeline history:
[07:08:28.538] # pg_rewind: 1: 0/00000000 - 0/040000E0
[07:08:28.538] # pg_rewind: 2: 0/040000E0 - 0/00000000
[07:08:28.538] # pg_rewind: Target timeline history:
[07:08:28.538] # pg_rewind: 1: 0/00000000 - 0/040000E0
[07:08:28.538] # pg_rewind: 2: 0/040000E0 - 0/060000E0
[07:08:28.538] # pg_rewind: 3: 0/060000E0 - 0/00000000
[07:08:28.538] # pg_rewind: servers diverged at WAL location 0/040000E0 on timeline 1
[07:08:28.538] # cp: cannot stat 'C:cirrus'$'\b''uild/testrun/pg_rewind/005_same_timelinedata/t_005_same_timeline_node_x_data/pgdata/pg_wal/000000020000000000000004': No such file or directory
[07:08:28.538] # pg_rewind: error: could not restore file "000000020000000000000004" from archive
[07:08:28.538] # pg_rewind: error: could not find previous WAL record at 0/040000E0
[07:08:28.538] # ------------------------------------
[07:08:28.538] # Failed test 'rewound node reflects source history, not target TLI 2/TLI 3 data'
[07:08:28.538] # at C:/cirrus/src/bin/pg_rewind/t/005_same_timeline.pl line 260.
[07:08:28.538] # got: 'origin2
[07:08:28.538] # x'
[07:08:28.538] # expected: 'b
[07:08:28.538] # origin2'
[07:08:28.538] # Looks like you failed 2 tests of 11.
[07:08:28.538]
[07:08:28.538] (test program exited with status code 2)
[07:08:28.538] ------------------------------------------------------------------------------
[07:08:28.538]

[1]: https://cirrus-ci.com/task/6228217159221248

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Hi Japin,

On 5/29/26 04:01, Japin Li wrote:

Hi, Mats

On Tue, 26 May 2026 at 18:03, Mats Kindahl <mats.kindahl@gmail.com> wrote:

Attached a new version of the patch with the changes you suggested.

I found an error on the Windows platform [1].

[07:08:28.538] >>> MALLOC_PERTURB_=168 PG_REGRESS=C:\cirrus\build\src/test\regress\pg_regress.exe REGRESS_SHLIB=C:\cirrus\build\src/test\regress\regress.dll MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 top_builddir=C:\cirrus\build UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MESON_TEST_ITERATION=1 PATH=C:\cirrus\build\tmp_install\usr\local\pgsql\bin;C:\cirrus\build\src\bin\pg_rewind;C:/cirrus/build/src/bin/pg_rewind/test;C:\VS_2019\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64;C:\VS_2019\MSBuild\Current\bin\Roslyn;C:\Program Files (x86)\Windows Kits\10\bin\10.0.22621.0\x64;C:\Program Files (x86)\Windows Kits\10\bin\x64;C:\VS_2019\\MSBuild\Current\Bin;C:\Windows\Microsoft.NET\Framework64\v4.0.30319;C:\VS_2019\Common7\IDE\;C:\VS_2019\Common7\Tools\;C:\VS_2019\VC\Auxiliary\Build;C:\zstd\zstd-v1.5.2-win64;C:\zlib;C:\lz4;C:\icu;C:\winflexbison;C:\strawberry\5.42.0.1\perl\bin;C:\python\Scripts\;C:\python\;C:\Windows Kits\10\Debuggers\x64;C:\Program Files\Git\usr\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\ProgramData\GooGet;C:\Program Files\Google\Compute Engine\metadata_scripts;C:\Program Files (x86)\Google\Cloud SDK\google-cloud-sdk\bin;C:\Program Files\PowerShell\7\;C:\Program Files\Google\Compute Engine\sysprep;C:\ProgramData\chocolatey\bin;C:\Program Files\Git\cmd;C:\Program Files\Git\mingw64\bin;C:\Program Files\Git\usr\bin;C:\Windows\system32\config\systemprofile\AppData\Local\Microsoft\WindowsApps INITDB_TEMPLATE=C:/cirrus/build/tmp_install/initdb-template ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 share_contrib_dir=C:/cirrus/build/tmp_install//usr/local/pgsql/share/contrib C:\python\python3.EXE C:\cirrus\build\..\src/tools/testwrap --basedir C:\cirrus\build --srcdir C:\cirrus\src\bin\pg_rewind --pg-test-extra --testgroup pg_rewind --testname 005_same_timeline -- C:\strawberry\5.42.0.1\perl\bin\perl.EXE -I C:/cirrus/src/test/perl -I C:\cirrus\src\bin\pg_rewind C:/cirrus/src/bin/pg_rewind/t/005_same_timeline.pl
[07:08:28.538] ------------------------------------- 8< -------------------------------------
[07:08:28.538] stderr:
[07:08:28.538] # Failed test 'pg_rewind rewinds across mismatched TLI 2 / TLI 2-prime to TLI 1'
[07:08:28.538] # at C:/cirrus/src/bin/pg_rewind/t/005_same_timeline.pl line 45.
[07:08:28.538] # ---------- command failed ----------
[07:08:28.538] # pg_rewind --debug --source-pgdata C:\cirrus\build/testrun/pg_rewind/005_same_timeline\data/t_005_same_timeline_node_b2_data/pgdata --target-pgdata C:\cirrus\build/testrun/pg_rewind/005_same_timeline\data/t_005_same_timeline_node_a2_data/pgdata --no-sync --config-file C:\cirrus\build\testrun\pg_rewind\005_same_timeline\data\tmp_test_ZCeZ/target-postgresql.conf.tmp --restore-target-wal
[07:08:28.538] # -------------- stderr --------------
[07:08:28.538] # pg_rewind: using for rewind "restore_command = 'cp "C:cirrusuild/testrun/pg_rewind/005_same_timelinedata/t_005_same_timeline_node_x_data/pgdata/pg_wal/%f" "%p"'"
[07:08:28.538] # pg_rewind: Source timeline history:
[07:08:28.538] # pg_rewind: 1: 0/00000000 - 0/040000E0
[07:08:28.538] # pg_rewind: 2: 0/040000E0 - 0/00000000
[07:08:28.538] # pg_rewind: Target timeline history:
[07:08:28.538] # pg_rewind: 1: 0/00000000 - 0/040000E0
[07:08:28.538] # pg_rewind: 2: 0/040000E0 - 0/060000E0
[07:08:28.538] # pg_rewind: 3: 0/060000E0 - 0/00000000
[07:08:28.538] # pg_rewind: servers diverged at WAL location 0/040000E0 on timeline 1
[07:08:28.538] # cp: cannot stat 'C:cirrus'$'\b''uild/testrun/pg_rewind/005_same_timelinedata/t_005_same_timeline_node_x_data/pgdata/pg_wal/000000020000000000000004': No such file or directory
[07:08:28.538] # pg_rewind: error: could not restore file "000000020000000000000004" from archive
[07:08:28.538] # pg_rewind: error: could not find previous WAL record at 0/040000E0
[07:08:28.538] # ------------------------------------
[07:08:28.538] # Failed test 'rewound node reflects source history, not target TLI 2/TLI 3 data'
[07:08:28.538] # at C:/cirrus/src/bin/pg_rewind/t/005_same_timeline.pl line 260.
[07:08:28.538] # got: 'origin2
[07:08:28.538] # x'
[07:08:28.538] # expected: 'b
[07:08:28.538] # origin2'
[07:08:28.538] # Looks like you failed 2 tests of 11.
[07:08:28.538]
[07:08:28.538] (test program exited with status code 2)
[07:08:28.538] ------------------------------------------------------------------------------
[07:08:28.538]

[1] https://cirrus-ci.com/task/6228217159221248

Thanks for testing it on Windows.

It seems like the path needs to be cleaned on Windows. I checked
Cluster.pm and created a version of that code and added that to the test
that should work. See attached patch.

I noted that many of the paths are not platform-agnostic. It an idea to
switch to use something like File::Spec instead and build paths using
that, but it's out of scope for this patch.

Best wishes,
Mats Kindahl, Multigres Engineer, Supabase

Hi, Mats

On Sat, 30 May 2026 at 22:26, Mats Kindahl <mats.kindahl@gmail.com> wrote:

Hi Japin,

On 5/29/26 04:01, Japin Li wrote:

Hi, Mats

On Tue, 26 May 2026 at 18:03, Mats Kindahl <mats.kindahl@gmail.com> wrote:

Attached a new version of the patch with the changes you suggested.

I found an error on the Windows platform [1].

[07:08:28.538] >>> MALLOC_PERTURB_=168
PG_REGRESS=C:\cirrus\build\src/test\regress\pg_regress.exe
REGRESS_SHLIB=C:\cirrus\build\src/test\regress\regress.dll
MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1
top_builddir=C:\cirrus\build
UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1
MESON_TEST_ITERATION=1
PATH=C:\cirrus\build\tmp_install\usr\local\pgsql\bin;C:\cirrus\build\src\bin\pg_rewind;C:/cirrus/build/src/bin/pg_rewind/test;C:\VS_2019\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64;C:\VS_2019\MSBuild\Current\bin\Roslyn;C:\Program
Files (x86)\Windows Kits\10\bin\10.0.22621.0\x64;C:\Program Files
(x86)\Windows
Kits\10\bin\x64;C:\VS_2019\\MSBuild\Current\Bin;C:\Windows\Microsoft.NET\Framework64\v4.0.30319;C:\VS_2019\Common7\IDE\;C:\VS_2019\Common7\Tools\;C:\VS_2019\VC\Auxiliary\Build;C:\zstd\zstd-v1.5.2-win64;C:\zlib;C:\lz4;C:\icu;C:\winflexbison;C:\strawberry\5.42.0.1\perl\bin;C:\python\Scripts\;C:\python\;C:\Windows
Kits\10\Debuggers\x64;C:\Program
Files\Git\usr\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\ProgramData\GooGet;C:\Program
Files\Google\Compute Engine\metadata_scripts;C:\Program Files
(x86)\Google\Cloud SDK\google-cloud-sdk\bin;C:\Program
Files\PowerShell\7\;C:\Program Files\Google\Compute
Engine\sysprep;C:\ProgramData\chocolatey\bin;C:\Program
Files\Git\cmd;C:\Program Files\Git\mingw64\bin;C:\Program
Files\Git\usr\bin;C:\Windows\system32\config\systemprofile\AppData\Local\Microsoft\WindowsApps
INITDB_TEMPLATE=C:/cirrus/build/tmp_install/initdb-template
ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1
share_contrib_dir=C:/cirrus/build/tmp_install//usr/local/pgsql/share/contrib
C:\python\python3.EXE C:\cirrus\build\..\src/tools/testwrap
--basedir C:\cirrus\build --srcdir C:\cirrus\src\bin\pg_rewind
--pg-test-extra --testgroup pg_rewind --testname 005_same_timeline
-- C:\strawberry\5.42.0.1\perl\bin\perl.EXE -I
C:/cirrus/src/test/perl -I C:\cirrus\src\bin\pg_rewind
C:/cirrus/src/bin/pg_rewind/t/005_same_timeline.pl
[07:08:28.538] ------------------------------------- 8< -------------------------------------
[07:08:28.538] stderr:
[07:08:28.538] # Failed test 'pg_rewind rewinds across mismatched TLI 2 / TLI 2-prime to TLI 1'
[07:08:28.538] # at C:/cirrus/src/bin/pg_rewind/t/005_same_timeline.pl line 45.
[07:08:28.538] # ---------- command failed ----------
[07:08:28.538] # pg_rewind --debug --source-pgdata
C:\cirrus\build/testrun/pg_rewind/005_same_timeline\data/t_005_same_timeline_node_b2_data/pgdata
--target-pgdata
C:\cirrus\build/testrun/pg_rewind/005_same_timeline\data/t_005_same_timeline_node_a2_data/pgdata
--no-sync --config-file
C:\cirrus\build\testrun\pg_rewind\005_same_timeline\data\tmp_test_ZCeZ/target-postgresql.conf.tmp
--restore-target-wal
[07:08:28.538] # -------------- stderr --------------
[07:08:28.538] # pg_rewind: using for rewind "restore_command = 'cp "C:cirrusuild/testrun/pg_rewind/005_same_timelinedata/t_005_same_timeline_node_x_data/pgdata/pg_wal/%f" "%p"'"
[07:08:28.538] # pg_rewind: Source timeline history:
[07:08:28.538] # pg_rewind: 1: 0/00000000 - 0/040000E0
[07:08:28.538] # pg_rewind: 2: 0/040000E0 - 0/00000000
[07:08:28.538] # pg_rewind: Target timeline history:
[07:08:28.538] # pg_rewind: 1: 0/00000000 - 0/040000E0
[07:08:28.538] # pg_rewind: 2: 0/040000E0 - 0/060000E0
[07:08:28.538] # pg_rewind: 3: 0/060000E0 - 0/00000000
[07:08:28.538] # pg_rewind: servers diverged at WAL location 0/040000E0 on timeline 1
[07:08:28.538] # cp: cannot stat 'C:cirrus'$'\b''uild/testrun/pg_rewind/005_same_timelinedata/t_005_same_timeline_node_x_data/pgdata/pg_wal/000000020000000000000004': No such file or directory
[07:08:28.538] # pg_rewind: error: could not restore file "000000020000000000000004" from archive
[07:08:28.538] # pg_rewind: error: could not find previous WAL record at 0/040000E0
[07:08:28.538] # ------------------------------------
[07:08:28.538] # Failed test 'rewound node reflects source history, not target TLI 2/TLI 3 data'
[07:08:28.538] # at C:/cirrus/src/bin/pg_rewind/t/005_same_timeline.pl line 260.
[07:08:28.538] # got: 'origin2
[07:08:28.538] # x'
[07:08:28.538] # expected: 'b
[07:08:28.538] # origin2'
[07:08:28.538] # Looks like you failed 2 tests of 11.
[07:08:28.538]
[07:08:28.538] (test program exited with status code 2)
[07:08:28.538] ------------------------------------------------------------------------------
[07:08:28.538]

[1] https://cirrus-ci.com/task/6228217159221248

Thanks for testing it on Windows.

It seems like the path needs to be cleaned on Windows. I checked
Cluster.pm and created a version of that code and added that to the
test that should work. See attached patch.

I noted that many of the paths are not platform-agnostic. It an idea
to switch to use something like File::Spec instead and build paths
using that, but it's out of scope for this patch.

Thanks for updating the patch. LGTM.

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Sorry, I only just noticed this thread.

I may be missing something, but UUID feels somewhat heavyweight to me
for this problem.

I wonder whether strengthening the history-based matching would be
sufficient instead. If timelines with the same TLI but different
histories can be treated as distinct and pg_rewind continues walking
the history chain until it finds a common ancestor, that seems like a
fairly natural fit with the existing timeline model.

UUIDs would certainly make identification straightforward, although
they would also introduce longer identifiers that are a bit less
convenient for humans to work with. My initial thought is that it may
be worth exploring how far we can get with the existing history
information before introducing a new identifier.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Hi Mats, Japin

Nice points were discussed and addressed.

The latest patch looks good to me.

Regards,
Surya Poondla

On 6/1/26 08:30, Kyotaro Horiguchi wrote:

Sorry, I only just noticed this thread.

I may be missing something, but UUID feels somewhat heavyweight to me
for this problem.

I wonder whether strengthening the history-based matching would be
sufficient instead. If timelines with the same TLI but different
histories can be treated as distinct and pg_rewind continues walking
the history chain until it finds a common ancestor, that seems like a
fairly natural fit with the existing timeline model.
UUIDs would certainly make identification straightforward, although
they would also introduce longer identifiers that are a bit less
convenient for humans to work with. My initial thought is that it may
be worth exploring how far we can get with the existing history
information before introducing a new identifier.

It is a good idea, but unfortunately there are positions in the timeline
that have same TLI, same LSN, but are still different timelines because
they originate from different promotions.

Just to summarize the situation: the timeline history file contains a
TLI (which is a number), and a switchpoint (which is an LSN). Each time
pg_promote is called, a new timeline is created based on the previous
TLI (it is increased by 1) and the LSN at that point. (The actual
history file is written by StartupXLOG, not by pg_promote, but
pg_promote triggers the process by writing a marker file.)

If two servers go through the same sequence, e.g., start at the same
timeline, does a promote, and write same length but different data
(e.g., add a line to a table, but with different contents), they might
end up with same TLI, same LSN, but different pg_promote calls, and
different database contents, hence it is not possible to distinguish them.

LSNs are usually different, so it is not a very likely scenario, but it
is still there.

The UUID is just generated and written when pg_promote is called, which
is not very often, hence does not affect the server and replication very
often. Note that the UUID is _not_ in the EOR (EndOfRecovery) record,
just in the timeline history file.

Best wishes,
Mats Kindahl

At Tue, 2 Jun 2026 04:13:45 +0200, Mats Kindahl <mats.kindahl@gmail.com> wrote in

If two servers go through the same sequence, e.g., start at the same
timeline, does a promote, and write same length but different data
(e.g., add a line to a table, but with different contents), they might
end up with same TLI, same LSN, but different pg_promote calls, and
different database contents, hence it is not possible to distinguish
them.

Thanks for the explanation.

When I described UUIDs as somewhat heavyweight, I was thinking less
about the runtime overhead and more about operational convenience for
humans. Your clarification that the UUID only lives in the timeline
history file addresses most of the implementation concerns I had in
mind.

My earlier suggestion was based on the assumption that two independent
histories ending up with the same TLI and switchpoint LSN would be
rare enough to be ignored in practice. If the goal is to rule out that
possibility entirely rather than merely make it extremely unlikely,
then I can see why some additional identifier would be needed.

In that context, a UUID certainly seems like a viable option.

Regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On 30 Apr 2026, at 13:19, Mats Kindahl <mats.kindahl@gmail.com> wrote:

There is one scenario that I assume is known that TLC found, but does not seem to be fixed. It is a relatively rare case, but since the fix is quite easy, I thought I'd share it with you and get feedback.

Hi Mats,

Thanks for working on this. I think the problem is real, but I wonder if
adding a separate UUID to timeline history files is solving it one step
too late.

If two independent promotions manage to choose the same numeric TLI, then
we already have two different histories with the same timeline identifier.
Their history files will also have the same name. A UUID in the file lets
tools detect the mismatch afterwards, but it does not prevent the archive
namespace from containing two different meanings for the same TLI.

In normal deployments with a shared archive this should only be possible
when the history file is not visible to the other promoting server:
either there is no usable restore_command/shared archive, or there is a
race around publishing and observing the history file. In other words, TLI
allocation is not atomic, but it is intended to be coordinated through the
archive.

Maybe we should keep TimelineID as the actual branch identifier and make
that allocation harder to collide instead of adding a second identifier.
For example, when choosing a new TLI, add some randomness rather than just
using the next sequential value. That would make the race window much less
dangerous: two independent promotions would be extremely unlikely to
choose the same TLI, the history file names would remain distinct, and TLI
would keep its current role as the timeline identifier.

This also keeps the operational model simpler. TimelineID is already the
identifier exposed in WAL file names, history file names, logs, and
recovery configuration. If we add UUIDs, we effectively introduce another
identity for the same object, and tools then need to reason about both.
If instead we make TLI allocation less deterministic under races, the
existing model remains intact.

Does that framing make sense, or am I missing a case where duplicate TLIs
are unavoidable even with a shared archive and a less collision-prone
allocation scheme?

Best regards, Andrey Borodin.

Hello

I know there's still ongoing discussion on the direction itself, but I focused on just testing and looking at the latest patch, in case the fix remains the same.

+			PG_CATCH();
+			{
+				ErrorData  *edata = CopyErrorData();
+
+				FlushErrorState();
+				ereport(FATAL,
+						errmsg("invalid UUID in history file \"%s\"", path),
+						errdetail("%s", edata->message));
+			}

This is missing a MemoryContextSwitchTo before CopyErrorData, and results in an assertion with debug builds.

+		memset(&entry->tluuid, 0, sizeof(pg_uuid_t));
+		if (nfields == 4 && strlen(uuid_str) == UUID_STR_LEN)
+			rewind_parse_uuid(uuid_str, &entry->tluuid);

This ignores the return value of rewind_parse_uuid, possibly writing partial garbage to tluuid on incorrect input.

Also, it seems like that with this patch, pg rewind requires the target's history file to be always there - is this an intended change? If yes, then it should be at least mentioned somewhere.

On master:
exit 0
"source and target cluster are on the same timeline"
"no rewind required"

On patched rewind:
exit 1
error: could not open file
".../tgt/pg_wal/00000002.history" for reading: No such file or directory

writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
+ const pg_uuid_t *newTLUUID,
XLogRecPtr switchpoint, char *reason)

In case this is a bug that should be backported, wouldn't this be an ABI break?

+#endif							/* !FRONTEND */
+
+extern pg_uuid_t *generate_uuidv7(uint64 unix_ts_ms, uint32 sub_ms);
+extern pg_uuid_t *generate_uuidv7_r(pg_uuid_t *uuid, uint64 unix_ts_ms, uint32 sub_ms);

Shouldn't these go before the endif?

On 6/8/26 12:48, Andrey Borodin wrote:

On 30 Apr 2026, at 13:19, Mats Kindahl<mats.kindahl@gmail.com> wrote:

There is one scenario that I assume is known that TLC found, but does not seem to be fixed. It is a relatively rare case, but since the fix is quite easy, I thought I'd share it with you and get feedback.

Hi Mats,

Hi Andrey,

Thanks for looking at this.

Thanks for working on this. I think the problem is real, but I wonder if
adding a separate UUID to timeline history files is solving it one step
too late.

If two independent promotions manage to choose the same numeric TLI, then
we already have two different histories with the same timeline identifier.
Their history files will also have the same name. A UUID in the file lets
tools detect the mismatch afterwards, but it does not prevent the archive
namespace from containing two different meanings for the same TLI.

Yes, that is correct.

In normal deployments with a shared archive this should only be possible
when the history file is not visible to the other promoting server:
either there is no usable restore_command/shared archive, or there is a
race around publishing and observing the history file. In other words, TLI
allocation is not atomic, but it is intended to be coordinated through the
archive.

Yes, that is the ideal way it should work when you have a shared
archive. This works because you have a central authority that
synchronizes the timelines (in theory, not counting bugs).

Maybe we should keep TimelineID as the actual branch identifier and make
that allocation harder to collide instead of adding a second identifier.
For example, when choosing a new TLI, add some randomness rather than just
using the next sequential value.
That would make the race window much less
dangerous: two independent promotions would be extremely unlikely to
choose the same TLI, the history file names would remain distinct, and TLI
would keep its current role as the timeline identifier.
This also keeps the operational model simpler. TimelineID is already the
identifier exposed in WAL file names, history file names, logs, and
recovery configuration. If we add UUIDs, we effectively introduce another
identity for the same object, and tools then need to reason about both.
If instead we make TLI allocation less deterministic under races, the
existing model remains intact.

Does that framing make sense, or am I missing a case where duplicate TLIs
are unavoidable even with a shared archive and a less collision-prone
allocation scheme?

I considered using some random increment of the TLI in the manner you
describe but there are some issues that makes this solution more
complicated from an operational perspective:

* If you skip some TLIs (in the sense pick a TLI that is "random but
larger"), then it is not clear what the relation between them are.
o The history files contain the complete linkage of the timelines,
so that is covered, but the naming would be strange.
+ For example, if you have history files 1, 5, 7, and 8, then
these can all belong to different timelines, (except 1), or
be a single timeline and it is hard to understand which one
without looking through the files.
o With more promotions, the relation becomes even more strange,
and the risk of collisions increases. (For example, imagine one
timeline with 1, 5, 7, 8, 11, and one timeline that forks off 1.
Then any increment of 4, 6, 7, or 10 will result in a collision.)
* To actually reduce the risk significantly, you need to have a very
wide range of the added randomness. Taking a smaller number is
easier to work with, but then you need to handle that some timelines
can collide in some manner.
* Normally, the history file with the highest number will be the only
relevant one. With this approach, you have to check the contents of
the files to understand which ones are relevant, which increases the
operational burden.

In contrast, if you use an UUID in this manner.

* Adding an UUID does not require a central coordinator and is not
likely to collide (on the level "impossible to collide") and is very
straightforward to add. It also comes with a low risk since the
places in the code that requires changes are very few and not likely
to have unexpected consequences elsewhere. This works both with and
without a shared archive.
* Normally, a shared archive should only contain a single timeline.
Anything else is an anomaly and should be corrected.
* I think it is still necessary to handle the case where you do not
have a shared archive; it would be an odd limitation to say that
promote only works if you have a shared archive
* The UUID still serves a purpose in capturing a situation where
things have gone wrong. Think of the UUID as similar to a "checksum"
safety and an extra precaution to prevent things from going wrong.

In short, I think the operational issues with random increment of the
history file number is worse, not better, and we should deal with the
name collisions correctly for shared archives instead. There is an issue
in that it need to work even in the case where you have a promotion that
generates a new UUID but the correct history file exists (reported in
the other message) that I will look into.

Best wishes,
Mats Kindahl

Hi, all

On Sun, 21 Jun 2026 at 11:09, Mats Kindahl <mats.kindahl@gmail.com> wrote:

On 6/8/26 12:48, Andrey Borodin wrote:

On 30 Apr 2026, at 13:19, Mats Kindahl <mats.kindahl@gmail.com> wrote:

There is one scenario that I assume is known that TLC found, but does not seem to be fixed. It is a relatively rare case, but since the fix is quite easy, I thought I'd share it with you and get feedback.

Hi Mats,

Hi Andrey,

Thanks for looking at this.

Thanks for working on this. I think the problem is real, but I wonder if
adding a separate UUID to timeline history files is solving it one step
too late.

If two independent promotions manage to choose the same numeric TLI, then
we already have two different histories with the same timeline identifier.
Their history files will also have the same name. A UUID in the file lets
tools detect the mismatch afterwards, but it does not prevent the archive
namespace from containing two different meanings for the same TLI.

Yes, that is correct.

In normal deployments with a shared archive this should only be possible
when the history file is not visible to the other promoting server:
either there is no usable restore_command/shared archive, or there is a
race around publishing and observing the history file. In other words, TLI
allocation is not atomic, but it is intended to be coordinated through the
archive.

Yes, that is the ideal way it should work when you have a shared archive. This works because you have a central authority
that synchronizes the timelines (in theory, not counting bugs).

Maybe we should keep TimelineID as the actual branch identifier and make
that allocation harder to collide instead of adding a second identifier.
For example, when choosing a new TLI, add some randomness rather than just
using the next sequential value.

That would make the race window much less
dangerous: two independent promotions would be extremely unlikely to
choose the same TLI, the history file names would remain distinct, and TLI
would keep its current role as the timeline identifier.
This also keeps the operational model simpler. TimelineID is already the
identifier exposed in WAL file names, history file names, logs, and
recovery configuration. If we add UUIDs, we effectively introduce another
identity for the same object, and tools then need to reason about both.
If instead we make TLI allocation less deterministic under races, the
existing model remains intact.

Does that framing make sense, or am I missing a case where duplicate TLIs
are unavoidable even with a shared archive and a less collision-prone
allocation scheme?

I considered using some random increment of the TLI in the manner you describe but there are some issues that makes this
solution more complicated from an operational perspective:

* If you skip some TLIs (in the sense pick a TLI that is "random but larger"), then it is not clear what the relation
between them are.

* The history files contain the complete linkage of the timelines, so that is covered, but the naming would be strange.

* For example, if you have history files 1, 5, 7, and 8, then these can all belong to different timelines, (except 1), or
be a single timeline and it is hard to understand which one without looking through the files.

* With more promotions, the relation becomes even more strange, and the risk of collisions increases. (For example,
imagine one timeline with 1, 5, 7, 8, 11, and one timeline that forks off 1. Then any increment of 4, 6, 7, or 10 will
result in a collision.)

* To actually reduce the risk significantly, you need to have a very wide range of the added randomness. Taking a smaller
number is easier to work with, but then you need to handle that some timelines can collide in some manner.
* Normally, the history file with the highest number will be the only relevant one. With this approach, you have to check
the contents of the files to understand which ones are relevant, which increases the operational burden.

In contrast, if you use an UUID in this manner.

* Adding an UUID does not require a central coordinator and is not likely to collide (on the level "impossible to
collide") and is very straightforward to add. It also comes with a low risk since the places in the code that requires
changes are very few and not likely to have unexpected consequences elsewhere. This works both with and without a
shared archive.
* Normally, a shared archive should only contain a single timeline. Anything else is an anomaly and should be corrected.
* I think it is still necessary to handle the case where you do not have a shared archive; it would be an odd limitation
to say that promote only works if you have a shared archive
* The UUID still serves a purpose in capturing a situation where things have gone wrong. Think of the UUID as similar to
a "checksum" safety and an extra precaution to prevent things from going wrong.

In short, I think the operational issues with random increment of the history file number is worse, not better, and we
should deal with the name collisions correctly for shared archives instead. There is an issue in that it need to work
even in the case where you have a promotion that generates a new UUID but the correct history file exists (reported in
the other message) that I will look into.

I would like to know the current status of this patch. I have encountered the
same issue in practice, and I think the proposed solution is reasonable.

I found that the v6 patch does not apply cleanly to the current master (1f414035135)
because commit 7f77b2a89bd4 changed the parameter type of writeTimeLineHistory().

I've rebased the patch and attached v7.

Best wishes,
Mats Kindahl

Best regards, Andrey Borodin.

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Hello Zsolt,

Thank you for reviewing this and sorry for the delay. I have attached a
new version with the issues you pointed to handled. See comments inline
below.

Best wishes,
Mats Kindahl

On 6/8/26 21:52, Zsolt Parragi wrote:

Hello

I know there's still ongoing discussion on the direction itself, but I
focused on just testing and looking at the latest patch, in case the
fix remains the same.

+			PG_CATCH();
+			{
+				ErrorData  *edata = CopyErrorData();
+
+				FlushErrorState();
+				ereport(FATAL,
+						errmsg("invalid UUID in history file \"%s\"", path),
+						errdetail("%s", edata->message));
+			}

This is missing a MemoryContextSwitchTo before CopyErrorData, and
results in an assertion with debug builds.

+		memset(&entry->tluuid, 0, sizeof(pg_uuid_t));
+		if (nfields == 4 && strlen(uuid_str) == UUID_STR_LEN)
+			rewind_parse_uuid(uuid_str, &entry->tluuid);

This ignores the return value of rewind_parse_uuid, possibly writing
partial garbage to tluuid on incorrect input.

Also, it seems like that with this patch, pg rewind requires the
target's history file to be always there - is this an intended change?
If yes, then it should be at least mentioned somewhere.

On master:
exit 0
"source and target cluster are on the same timeline"
"no rewind required"

On patched rewind:
exit 1
error: could not open file
".../tgt/pg_wal/00000002.history" for reading: No such file or directory

That was not the intention, so I added a check for a target history file
to the code and a test that checks the behavior without a timeline
history file and compared that with the behavior before the change.

writeTimeLineHistory(TimeLineID newTLI, TimeLineID parentTLI,
+ const pg_uuid_t *newTLUUID,
XLogRecPtr switchpoint, char *reason)

In case this is a bug that should be backported, wouldn't this be an ABI break?

+#endif							/* !FRONTEND */
+
+extern pg_uuid_t *generate_uuidv7(uint64 unix_ts_ms, uint32 sub_ms);
+extern pg_uuid_t *generate_uuidv7_r(pg_uuid_t *uuid, uint64
unix_ts_ms, uint32 sub_ms);

Shouldn't these go before the endif?

I thought the generate_uuidv7 should be possible to use for a frontend,
but looking at how it is written and linked, that is not possible to I
moved the #endif.