Skip site navigation (1) Skip section navigation (2)

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

From: Simon Riggs <simon(at)2ndQuadrant(dot)com>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, pgsql-bugs(at)postgresql(dot)org
Subject: Re: BUG #4879: bgwriter fails to fsync the file in recoverymode
Date: 2009-06-25 21:10:42
Message-ID: 1245964242.4038.227.camel@ebony.2ndQuadrant (view raw or flat)
Thread:
Lists: pgsql-bugs
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


In response to

Responses

pgsql-bugs by date

Next:From: Tom LaneDate: 2009-06-25 21:11:02
Subject: Re: BUG #4879: bgwriter fails to fsync the file in recovery mode
Previous:From: Alfred MonticelloDate: 2009-06-25 21:04:02
Subject: BUG #4883: tar xf fails on NFS4 mounts

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group