On Tue, Aug 18, 2026 at 1:37 PM Imran Zaheer <[email protected]> wrote: > I was looking into the patch
Thanks! > * Instead of setting the shared state in StartupXLOG, why not set it > in the StartupSUBTRANS itself? *If* we take this approach, I think the flag should no longer mean that hot standby initialization has started pg_subtrans, but rather that StartupSUBTRANS() has completed. We would therefore also need to rename the flag and update the comments accordingly. For a bug fix in stable branches, I would prefer to keep the change as small as possible. Also, subtrans.c says that there are no XLOG interactions, so having it update shared recovery state in xlogrecovery.c feels a bit odd to me. I therefore prefer the current patch and approach. > * Other than that we also call TruncateSUBTRANS() while creating a > checkpoint; maybe we can also improve the guard here, although the > assertion under TruncateSUBTRANS could be enough? > > @@ -7879,7 +7878,7 @@ CreateCheckPoint(int flags) > * in subtrans.c). During recovery, though, we mustn't do this > because > * StartupSUBTRANS hasn't been called yet. > */ > - if (!RecoveryInProgress()) > + if (!RecoveryInProgress() && RecoverySubtransInitialized()) > TruncateSUBTRANS(GetOldestTransactionIdConsideredRunning()); I don't think this extra check is necessary. If !RecoveryInProgress(), StartupSUBTRANS() should already have been called, so the existing guard should be sufficient. Regards, -- Fujii Masao
