On 11/20/23 15:03, Andres Freund wrote:
On 2023-11-20 11:35:15 +0100, Laurenz Albe wrote:
If we add a message for starting with "backup_label", shouldn't
we also add a corresponding message for starting from a checkpoint
found in the control file? If you see that in a problem report,
you immediately know what is going on.
Maybe - the reason I hesitate on that is that emitting an additional log
message when starting from a base backup just adds something "once once the
lifetime of a node". Whereas emitting something every start obviously doesn't
impose any limit.
Hmm, yeah, that would be a bit much.
Here's the state with my updated patch, when starting up from a base backup:
LOG: starting PostgreSQL 17devel on x86_64-linux, compiled by gcc-14.0.0,
64-bit
LOG: listening on IPv6 address "::1", port 5441
LOG: listening on IPv4 address "127.0.0.1", port 5441
LOG: listening on Unix socket "/tmp/.s.PGSQL.5441"
LOG: database system was interrupted; last known up at 2023-11-20 10:55:49 PST
LOG: starting recovery from base backup with redo LSN E/AFF07F20, checkpoint
LSN E/B01B17F0, on timeline ID 1
LOG: entering standby mode
LOG: redo starts at E/AFF07F20
LOG: completed recovery from base backup with redo LSN E/AFF07F20
LOG: consistent recovery state reached at E/B420FC80
Besides the phrasing and the additional log message (I have no opinion about
whether it should be backpatched or not), I used %u for TimelineID as
appropriate, and added a comma before "on timeline".
I still wonder if we need "base backup" in the messages? That sort of
implies (at least to me) you used pg_basebackup but that may not be the
case.
FWIW, I also prefer "backup recovery" over "recovery from backup".
"recovery from backup" reads fine here, but if gets more awkward when
you want to say something like "recovery from backup settings". In that
case, I think "backup recovery settings" reads better. Not important for
this patch, maybe, but the recovery in pg_control patch went the other
way and I definitely think it makes sense to keep them consistent,
whichever way we go.
Other than that, looks good for HEAD. Whether we back patch or not is
another question, of course.
Regards,
-David
