On Thu, Oct 28, 2021 at 11:39:52AM -0400, Robert Haas wrote: > I previously reported[1] that CreateReplicationSlot() was accessing > the global variable ThisTimeLineID when it might not be initialized. > Today I realized that the code in basebackup.c has the same problem. > perform_base_backup() blithely uses the variable six times without > doing anything at all to initialize it. In practice, it's always going > to be initialized to some non-zero value, because pg_basebackup is > always going to execute IDENTIFY_SYSTEM before it executes > BASE_BACKUP, and that's going to set ThisTimeLineID as a side effect. > But, if you hack pg_basebackup to not call IDENTIFY_SYSTEM before > calling BASE_BACKUP, then you can reach this code with ThisTimeLineID > == 0 using pg_basebackup -Xfetch. Even if you don't do that, you're > only guaranteed that ThisTimeLineID is initialized to something, not > that it's initialized to the correct thing. The timeline on the > standby can change any time.
Yes, I agree that it is a good idea to cut the dependency of those code paths with ThisTimeLineID, expecting IDENTIFY_SYSTEM to have done the job beforehand. One argument in favor of your change, though I'd like to think that nobody does so, is that users could run BASE_BACKUP with a replication connection. So no need to hack pg_basebackup to be able to finish with a WAL sender that has no TLI set in the backend. Your patch seems correct to me. -- Michael
signature.asc
Description: PGP signature
