Re: BUG #4879: bgwriter fails to fsync the file in recovery mode

On Thu, 2009-06-25 at 22:29 +0300, Heikki Linnakangas wrote:
> Tom Lane wrote:
> > While nosing around the problem areas, I think I've found yet another
> > issue here. The global bool InRecovery is only maintained correctly
> > in the startup process, which wasn't a problem before 8.4. However,
> > if we are making the bgwriter execute the end-of-recovery checkpoint,
> > there are multiple places where it is tested that are going to be
> > executed by bgwriter. I think (but am not 100% sure) that these
> > are all the at-risk references:
> > XLogFlush
> > CheckPointMultiXact
> > CreateCheckPoint (2 places)
> > Heikki's latest patch deals with the tests in CreateCheckPoint (rather
> > klugily IMO) but not the others. I think it might be better to fix
> > things so that InRecovery is maintained correctly in the bgwriter too.
>
> We could set InRecovery=true in CreateCheckPoint if it's a startup
> checkpoint, and reset it afterwards.

That seems like a bad idea.

As you said earlier,

On Thu, 2009-06-25 at 23:15 +0300, Heikki Linnakangas wrote:

> Well, we have RecoveryInProgress() now that answers the question "is
> recovery still in progress in the system". InRecovery now means "am I
> a process that's performing WAL replay?".

so it would be a mistake to do as you propose above because it changes
the meaning of these well defined parts of the system.

* XLogFlush mentions InRecovery right at the end and the correct usage
would be RecoveryIsInProgress() rather than InRecovery.

* The usage of InRecovery in CheckPointMultiXact() should also be
replaced with RecoveryIsInProgress()

* The two usages of InRecovery can also be replaced by
RecoveryIsInProgress()

So, yes, there are some places where InRecovery is used in code executed
by the bgwriter, but the correct fix is to use RecoveryIsInProgress().
It is a hack to try to set the InRecovery state flag in bgwriter and one
that would change the clear meaning of InRecovery, to no good purpose.

Subsequent discussion on this subthread may no longer be relevant.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support