On Tue, 2023-01-17 at 10:32 -0500, Tom Lane wrote: > Laurenz Albe <laurenz.a...@cybertec.at> writes: > > On Mon, 2023-01-16 at 19:59 +0100, Torsten Förtsch wrote: > > > So, the timestamp displayed in the log message is certainly wrong. > > > If recovery stops at a WAL record that has no timestamp, you get this > > bogus recovery stop time. I think we should show the recovery stop time > > only if time was the target, as in the attached patch. > > I don't think that is a tremendously useful definition: the user > already knows what recoveryStopTime is, or can find it out from > their settings easily enough. > > I seem to recall that the original idea was to report the timestamp > of the commit/abort record we are stopping at. Maybe my memory is > faulty, but I think that'd be significantly more useful than the > current behavior, *especially* when the replay stopping point is > defined by something other than time. > > (Also, the wording of the log message suggests that that's what > the reported time is supposed to be. I wonder if somebody messed > this up somewhere along the way.)
recoveryStopTime is set to recordXtime (the time of the xlog record) a few lines above that patch, so this is useful information if it is present. I realized that my original patch might be a problem for translation; here is an updated version that does not take any shortcuts. Yours, Laurenz Albe
From b01428486f7795f757edd14f66362ee575d2f168 Mon Sep 17 00:00:00 2001 From: Laurenz Albe <laurenz.a...@cybertec.at> Date: Wed, 18 Jan 2023 09:23:50 +0100 Subject: [PATCH] Don't show bogus recovery stop time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If target for archive recovery was different from time, the WAL record that ended recovery might not have a timestamp. In that case, the log message at the end of recovery would show a recovery time of midnight on 2000-01-01. Reported-by: Torsten Förtsch Author: Laurenz Albe Discussion: https://postgr.es/m/cakkg4_kuevpqbmyoflajx7opaqk6cvwkvx0hrcfjspfrptx...@mail.gmail.com --- src/backend/access/transam/xlogrecovery.c | 35 +++++++++++++++++------ 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index bc3c3eb3e7..73929a535e 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -2570,19 +2570,38 @@ recoveryStopsBefore(XLogReaderState *record) recoveryStopLSN = InvalidXLogRecPtr; recoveryStopName[0] = '\0'; + /* this could be simplified, but that might break translatability */ if (isCommit) { - ereport(LOG, - (errmsg("recovery stopping before commit of transaction %u, time %s", - recoveryStopXid, - timestamptz_to_str(recoveryStopTime)))); + if (recoveryStopTime != 0) + { + ereport(LOG, + (errmsg("recovery stopping before commit of transaction %u, time %s", + recoveryStopXid, + timestamptz_to_str(recoveryStopTime)))); + } + else + { + ereport(LOG, + (errmsg("recovery stopping before commit of transaction %u", + recoveryStopXid))); + } } else { - ereport(LOG, - (errmsg("recovery stopping before abort of transaction %u, time %s", - recoveryStopXid, - timestamptz_to_str(recoveryStopTime)))); + if (recoveryStopTime != 0) + { + ereport(LOG, + (errmsg("recovery stopping before abort of transaction %u, time %s", + recoveryStopXid, + timestamptz_to_str(recoveryStopTime)))); + } + else + { + ereport(LOG, + (errmsg("recovery stopping before abort of transaction %u", + recoveryStopXid))); + } } } -- 2.39.0
