Hello.
At Thu, 16 Sep 2021 11:23:46 +0300, Heikki Linnakangas <hlinnaka@iki.fi> wrote in
> Here is another rebase.
I have several comments on this.
0001:
I understand this is almost simple relocation of code fragments. But
it seems introducing some behavioral changes.
PublishStartProcessInformation() was changed to be called while
crash recovery or on standalone server. Maybe it is harmless and
might be more consistent, so I'm fine with it.
Another call to ResetUnloggedRelations is added before redo start,
that seems fine.
recoveryStopReason is always acquired but it is used only after
archive recovery. I'm not sure about reason for the variable to
live in that wide context. Couldn't we remove the variable then
call getRecoveryStopReason() directly at the required place?
0002:
heapam.c, clog.c, twophase.c, dbcommands.c doesn't need xlogrecvoer.h.
> XLogRecCtl
"Rec" looks like Record. Couldn't we use "Rcv", "Recov" or just
"Recovery" instead?
> TimeLineID PrevTimeLineID;
> TransactionId oldestActiveXID;
> bool promoted = false;
> EndOfWalRecoveryInfo *endofwal;
> bool haveTblspcMap;
This is just a matter of taste but the "endofwal" looks somewhat
alien in the variables.
xlog.c:
+void
+SwitchIntoArchiveRecovery(XLogRecPtr EndRecPtr)
Isn't this a function of xlogrecovery.c? Or rather isn't
minRecoveryPoint-related stuff of xlogrecovery.c?
0003;
Just looks fine. I might want to remove the parameter xlogreader
from ApplyWalRecord, but that seems cause more harm than good.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center