Tom Lane wrote:
> Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
>> Here's a patch taking that approach, and I think it's better than the
>> previous one. I was afraid we would lose robustness if we have to set
>> the shared state as "out of recovery" before requesting the checkpoint,
>> but we can use the same trick we were using in startup process and set
>> LocalRecoveryInProgress=false before setting the shared variable. I
>> introduced a new CHECKPOINT_IS_STARTUP flag,
>
> This isn't a "startup" checkpoint, it's an "end of recovery" checkpoint,
> because we don't do it during normal startup. Not sure about an equally
> concise name based on that, but I don't like IS_STARTUP.
>
> On the other point: are we going to eliminate mdunlink's isRedo
> parameter? Maybe the better thing is to have its callers pass the value
> of InArchiveRecovery?
I think my initial analysis of this bug was bogus. There's nothing wrong
with mdunlink() as it is, it's calling ForgetRelationFsyncRequests()
before it unlinks anything, regardless of the isRedo parameter. In
Fujii-san's scenario, it was just going to the pendingOpsTable of the
startup process and not sent to bgwriter as it should. Setting
pendingOpsTable=NULL when bgwriter is launched will fix that.
I somehow confused register_unlink() and ForgetRelationFsyncRequests()
and thought that we need the register_unlink() call when bgwriter is
active, but we don't.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com