Hi,
> some comments on v2.
Thanks for your comments. I have incorporated the changes
and updated a new patch. Please find the details below.
> On the timeline switch, setting a flag should be enough, I don't think
> that we need to wake up the archiver. Because it will just waste the
> scan cycle.
Yes, I modified it.
> Why do we need multi level interfaces? I mean instead of calling first
> XLogArchiveNotifyTLISwitch and then calling PgArchNotifyTLISwitch,
> can't we directly call PgArchNotifyTLISwitch()?
Yes, multilevel interfaces are not required. Removed extra interface.
> + if (timeline_switch)
> + {
> + /* Perform a full directory scan in next cycle */
> + dirScan = true;
> + timeline_switch = false;
> + }
> I suggest you can add some comments atop this check.
Added comment to specify the action required in case of a
timeline switch.
> I think you should use %m in the error message so that it also prints
> the OS error code.
Done.
> Why is this a global variable? I mean whenever you enter the function
> pgarch_ArchiverCopyLoop(), this can be set to true and after that you
> can pass this as inout parameter to pgarch_readyXlog() there in it can
> be conditionally set to false once we get some segment and whenever
> the timeline switch we can set it back to the true.
Yes, It is not necessary to have global scope for "dirScan". Changed
the scope to local for "dirScan" and "nextLogSegNo".
PFA patch v3.
Thanks,
Dipesh
