Re: pg_rewind does not rewind diverging timelines - Mailing list pgsql-hackers

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?