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

2009-06-30 Thread Tom Lane
Simon Riggs writes: > On Fri, 2009-06-26 at 16:48 -0400, Tom Lane wrote: >> * I find the RecoveryInProgress test in XLogNeedsFlush rather dubious. >> Why is it okay to disable that? For at least one of the two callers >> (SetHintBits) it seems like the safe answer is "true" not "false". >> This d

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

2009-06-30 Thread Simon Riggs
On Fri, 2009-06-26 at 16:48 -0400, Tom Lane wrote: > * I find the RecoveryInProgress test in XLogNeedsFlush rather dubious. > Why is it okay to disable that? For at least one of the two callers > (SetHintBits) it seems like the safe answer is "true" not "false". > This doesn't matter too much ye

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

2009-06-26 Thread Tom Lane
Heikki Linnakangas writes: > Here's a patch implementing the WALWriteAllowed() idea (I'm not wedded > to the name). There's two things that trouble me with this patch: I've committed this patch plus the mentioned revisions, along with a bunch of code review of my own (mostly doctoring comments th

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

2009-06-26 Thread Tom Lane
Heikki Linnakangas writes: > It's getting late again, and I'm afraid I have to stop for today :-(. OK, I'll start from your last patch and see what I can do. regards, tom lane -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscr

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

2009-06-26 Thread Heikki Linnakangas
Tom Lane wrote: > I wrote: >> Hmm ... this doesn't really feel cleaner to me, although I'm not sure >> why not. > > Oh, I thought of a more concrete point: InRecovery is inherently a > system-wide state, but XLogInsertAllowed is *not*. While we write > the EOR checkpoint, we really want only the

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

2009-06-26 Thread Alvaro Herrera
Tom Lane escribió: > I wrote: > > Hmm ... this doesn't really feel cleaner to me, although I'm not sure > > why not. > > Oh, I thought of a more concrete point: InRecovery is inherently a > system-wide state, but XLogInsertAllowed is *not*. While we write > the EOR checkpoint, we really want only

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

2009-06-26 Thread Tom Lane
I wrote: > Hmm ... this doesn't really feel cleaner to me, although I'm not sure > why not. Oh, I thought of a more concrete point: InRecovery is inherently a system-wide state, but XLogInsertAllowed is *not*. While we write the EOR checkpoint, we really want only the bgwriter to be authorized to

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

2009-06-26 Thread Tom Lane
Heikki Linnakangas writes: > One idea is to merge LocalWALWriteAllowed and LocalRecoveryInProgress > (and the corresponding shared variables too) into one XLogState variable > with three states > * in-recovery > * normal operation > * shutdown. > The variable would always move from a lower state

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

2009-06-26 Thread Heikki Linnakangas
Tom Lane wrote: > Heikki Linnakangas writes: >> - CreateCheckPoint() calls AdvanceXLInsertBuffer() before setting >> LocalWALWriteAllowed. I don't see anything in AdvanceXLInsertBuffer() >> that would fail, but it doesn't feel right. While strictly speaking it >> doesn't insert new WAL records, it

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

2009-06-26 Thread Tom Lane
Heikki Linnakangas writes: > Here's a patch implementing the WALWriteAllowed() idea (I'm not wedded > to the name). I'm not either ... this morning it seems to me that XLogWriteAllowed (or maybe XLogInsertAllowed?) would be more in keeping with the existing code names in this area. > There's two

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

2009-06-26 Thread Tom Lane
Heikki Linnakangas writes: > Tom Lane wrote: >> I observe that the substantial amount of care we have taken over >> XLogFlush's handling of bad-input-LSN scenarios has been completely >> destroyed by the UpdateMinRecoveryPoint patch, which will fail >> disastrously (leaving the database unstartabl

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

2009-06-26 Thread Heikki Linnakangas
Simon Riggs wrote: > On Fri, 2009-06-26 at 05:14 +0100, Simon Riggs wrote: >> On Thu, 2009-06-25 at 20:25 -0400, Tom Lane wrote: > >>> What I am thinking is that instead of the hack of clearing >>> LocalRecoveryInProgress to allow the current process to write WAL, >>> we should have a separate tes

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

2009-06-26 Thread Fujii Masao
Hi, On Fri, Jun 26, 2009 at 3:22 PM, Heikki Linnakangas wrote: > Are you actually seeing performance degradation caused by frequent > pg_control updates? In the simple test scenarios I've tested, pg_control > is updated only once every few WAL segments, and this with > shared_buffer=32MB. With lar

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

2009-06-25 Thread Heikki Linnakangas
Tom Lane wrote: > I observe that the substantial amount of care we have taken over > XLogFlush's handling of bad-input-LSN scenarios has been completely > destroyed by the UpdateMinRecoveryPoint patch, which will fail > disastrously (leaving the database unstartable/unrecoverable) if a > bogusly la

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

2009-06-25 Thread Heikki Linnakangas
Fujii Masao wrote: > On Fri, Jun 26, 2009 at 7:14 AM, Heikki > Linnakangas wrote: >> We certainly update it an order of magnitude more often than before, but >> I don't think that's an issue. We're talking about archive recovery >> here. It's not like in normal operation where a corrupt pg_control

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

2009-06-25 Thread Simon Riggs
On Fri, 2009-06-26 at 05:14 +0100, Simon Riggs wrote: > On Thu, 2009-06-25 at 20:25 -0400, Tom Lane wrote: > > What I am thinking is that instead of the hack of clearing > > LocalRecoveryInProgress to allow the current process to write WAL, > > we should have a separate test function WALWriteAllo

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

2009-06-25 Thread Simon Riggs
On Fri, 2009-06-26 at 10:56 +0900, Fujii Masao wrote: > Hi, > > Thank you for addressing the problem! > > On Fri, Jun 26, 2009 at 7:14 AM, Heikki > Linnakangas wrote: > > We certainly update it an order of magnitude more often than before, but > > I don't think that's an issue. We're talking abo

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 20:25 -0400, Tom Lane wrote: > Simon Riggs writes: > > On Thu, 2009-06-25 at 19:16 -0400, Tom Lane wrote: > >> Simon Riggs writes: > >>> On the patch, the kluge to set InRecovery is unnecessary and confusing. > >> > >> I'll look into a better way to do that tonight. Did y

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

2009-06-25 Thread Tom Lane
I wrote: > ... Never mind that complaint then. Be advised that I'm going to be on the warpath again in the morning. I observe that the substantial amount of care we have taken over XLogFlush's handling of bad-input-LSN scenarios has been completely destroyed by the UpdateMinRecoveryPoint patch, wh

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

2009-06-25 Thread Fujii Masao
Hi, Thank you for addressing the problem! On Fri, Jun 26, 2009 at 7:14 AM, Heikki Linnakangas wrote: > We certainly update it an order of magnitude more often than before, but > I don't think that's an issue. We're talking about archive recovery > here. It's not like in normal operation where a c

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

2009-06-25 Thread Tom Lane
Simon Riggs writes: > On Thu, 2009-06-25 at 19:16 -0400, Tom Lane wrote: >> Simon Riggs writes: >>> On the patch, the kluge to set InRecovery is unnecessary and confusing. >> >> I'll look into a better way to do that tonight. Did you have any >> specific improvement in mind? > Yes, all already

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 19:16 -0400, Tom Lane wrote: > Simon Riggs writes: > > On the patch, the kluge to set InRecovery is unnecessary and confusing. > > I'll look into a better way to do that tonight. Did you have any > specific improvement in mind? Yes, all already mentioned on this thread.

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

2009-06-25 Thread Tom Lane
Simon Riggs writes: > On the patch, the kluge to set InRecovery is unnecessary and confusing. I'll look into a better way to do that tonight. Did you have any specific improvement in mind? regards, tom lane -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 18:40 -0400, Tom Lane wrote: > Simon Riggs writes: > > On Fri, 2009-06-26 at 00:37 +0300, Heikki Linnakangas wrote: > >> Ok, I've committed the above fixes everyone agreed on. > > > At this stage of RC, I expected you to post the patch and have the other > > 2 people workin

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

2009-06-25 Thread Tom Lane
Simon Riggs writes: > On Fri, 2009-06-26 at 00:37 +0300, Heikki Linnakangas wrote: >> Ok, I've committed the above fixes everyone agreed on. > At this stage of RC, I expected you to post the patch and have the other > 2 people working actively on this issue review it before you commit. We're goi

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas writes: > Tom Lane wrote: >> The behavior you seem to have in mind would be completely disastrous >> from a performance standpoint, as we'd be writing and fsyncing >> pg_control constantly during a recovery. > Please define "constantly". We discussed that part of the patch here

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

2009-06-25 Thread Heikki Linnakangas
Heikki Linnakangas wrote: > Tom Lane wrote: >> I wouldn't consider that a >> good idea from a reliability standpoint either --- the more writes to >> pg_control, the more risk of fatal corruption of that file. > > We certainly update it an order of magnitude more often than before, but > I don't

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

2009-06-25 Thread Simon Riggs
On Fri, 2009-06-26 at 00:37 +0300, Heikki Linnakangas wrote: > Ok, I've committed the above fixes everyone agreed on. Sorry, but I haven't agreed to very much of what you just committed. There are some unnecessary and confusing hacks that really don't help anybody sort out why any of this has

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

2009-06-25 Thread Heikki Linnakangas
Tom Lane wrote: > The behavior you seem to have in mind would be completely disastrous > from a performance standpoint, as we'd be writing and fsyncing > pg_control constantly during a recovery. Please define "constantly". We discussed that part of the patch here: http://archives.postgresql.org/m

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas writes: > Simon Riggs wrote: >> AFAICS the problem Heikki is worried about exists 8.2+. If you stop >> recovery, edit recovery.conf to an earlier recovery target and then >> re-run recovery then it is possible that data that would not exist until >> after the (new) recovery poin

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

2009-06-25 Thread Heikki Linnakangas
Simon Riggs wrote: > AFAICS the problem Heikki is worried about exists 8.2+. If you stop > recovery, edit recovery.conf to an earlier recovery target and then > re-run recovery then it is possible that data that would not exist until > after the (new) recovery point has made its way to disk. The co

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 17:17 -0400, Tom Lane wrote: > Heikki Linnakangas writes: > > We do store the minimum recovery point required by the base backup in > > control file, but it's not advanced once the recovery starts. So if you > > start recovery, starting from say 123, let it progress to locat

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

2009-06-25 Thread Heikki Linnakangas
Simon Riggs wrote: > On Thu, 2009-06-25 at 15:10 -0400, Tom Lane wrote: > >> So to summarize the state of play, it seems >> we have these issues: >> >> * need to delete startup process's local pendingOpsTable once bgwriter >> is launched, so that requests go to bgwriter instead > > Need to ensure

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 17:11 -0400, Tom Lane wrote: > Simon Riggs writes: > > So, yes, there are some places where InRecovery is used in code executed > > by the bgwriter, but the correct fix is to use RecoveryIsInProgress(). > > Agreed, but this gets us no closer to solving the real problem, whi

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 22:29 +0300, Heikki Linnakangas wrote: > Hmm, I see another small issue. We now keep track of the "minimum > recovery point". Whenever a data page is flushed, we set minimum > recovery point to the LSN of the page in XLogFlush(), instead of > fsyncing WAL like we do in norma

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas writes: > We do store the minimum recovery point required by the base backup in > control file, but it's not advanced once the recovery starts. So if you > start recovery, starting from say 123, let it progress to location 456, > kill recovery and start it again, but only let it

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

2009-06-25 Thread Tom Lane
Simon Riggs writes: > So, yes, there are some places where InRecovery is used in code executed > by the bgwriter, but the correct fix is to use RecoveryIsInProgress(). Agreed, but this gets us no closer to solving the real problem, which is that when we perform the end-of-recovery checkpoint, we

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

2009-06-25 Thread Heikki Linnakangas
Tom Lane wrote: > Heikki Linnakangas writes: >> Tom Lane wrote: >>> I would like an explanation of why minimum recovery point needs to >>> be advanced at all. If there's any use for it, it is surely part >>> of functionality that is not there in 8.4. > >> It gives protection against starting up

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

2009-06-25 Thread Simon Riggs
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,

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas writes: > Tom Lane wrote: >> I would like an explanation of why minimum recovery point needs to >> be advanced at all. If there's any use for it, it is surely part >> of functionality that is not there in 8.4. > It gives protection against starting up the database too early.

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

2009-06-25 Thread Heikki Linnakangas
Tom Lane wrote: > Heikki Linnakangas writes: >> Tom Lane wrote: >>> Actually, what in the world is the purpose of that code at all? > >> Huh? The only other place where we advance minimum recovery point is >> CreateRestartPoint() when we skip the restartpoint. The >> UpdateMinRecoveryPoint() call

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 15:10 -0400, Tom Lane wrote: > So to summarize the state of play, it seems > we have these issues: > > * need to delete startup process's local pendingOpsTable once bgwriter > is launched, so that requests go to bgwriter instead Need to ensure that fsync requests are direc

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas writes: > Tom Lane wrote: >> We would want the end-of-recovery checkpoint to act like it's not in >> recovery anymore for this purpose, no? > For the purpose of updating min recovery point, we want it to act like > it *is* still in recovery. Well, without a clear explanation o

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 15:10 -0400, Tom Lane wrote: > Are you (Heikki and Simon) in a position to finish these things today? Certainly will try. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql

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

2009-06-25 Thread Heikki Linnakangas
Tom Lane wrote: > Heikki Linnakangas writes: >> Tom Lane wrote: >>> ... 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. I'm

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas writes: > Tom Lane wrote: >> Actually, what in the world is the purpose of that code at all? > Huh? The only other place where we advance minimum recovery point is > CreateRestartPoint() when we skip the restartpoint. The > UpdateMinRecoveryPoint() call in XLogFlush() is what w

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

2009-06-25 Thread Heikki Linnakangas
Tom Lane wrote: > I wrote: >> Heikki Linnakangas writes: >>> Hmm, I see another small issue. We now keep track of the "minimum >>> recovery point". Whenever a data page is flushed, we set minimum >>> recovery point to the LSN of the page in XLogFlush(), instead of >>> fsyncing WAL like we do in no

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

2009-06-25 Thread Tom Lane
I wrote: > Heikki Linnakangas writes: >> Hmm, I see another small issue. We now keep track of the "minimum >> recovery point". Whenever a data page is flushed, we set minimum >> recovery point to the LSN of the page in XLogFlush(), instead of >> fsyncing WAL like we do in normal operation. > We w

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas writes: > Tom Lane wrote: >> ... 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. I'm not 100% sure it's safe t

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

2009-06-25 Thread Heikki Linnakangas
Tom Lane wrote: > Heikki Linnakangas 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 tric

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

2009-06-25 Thread Heikki Linnakangas
Tom Lane wrote: > +1, I think that's okay. So to summarize the state of play, it seems > we have these issues: > > * need to delete startup process's local pendingOpsTable once bgwriter > is launched, so that requests go to bgwriter instead > > * need to push end-of-recovery checkpoint into bgwr

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

2009-06-25 Thread Heikki Linnakangas
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 checkpoin

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas writes: > Tom Lane wrote: >> 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 md

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 21:59 +0300, Heikki Linnakangas wrote: > I think my initial analysis of this bug was bogus. Perhaps, but the pendingOpsTable issue is a serious one, so we haven't wasted our time by fixing it. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services an

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

2009-06-25 Thread Tom Lane
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 pl

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

2009-06-25 Thread Heikki Linnakangas
Tom Lane wrote: > Heikki Linnakangas 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 tric

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 14:46 -0400, Tom Lane wrote: > 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? We have one caller that is using a parameter incorrectly. It seems we should cor

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas 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 sta

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

2009-06-25 Thread Heikki Linnakangas
Tom Lane wrote: > Heikki Linnakangas writes: >> Simon Riggs wrote: >>> On Thu, 2009-06-25 at 12:43 -0400, Tom Lane wrote: What about "revert the patch"? >>> That's probably just as dangerous. > >> I don't feel comfortable either reverting such a big patch at last >> minute. > > Yeah, I'm no

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

2009-06-25 Thread Heikki Linnakangas
Simon Riggs wrote: > On Thu, 2009-06-25 at 13:25 -0400, Tom Lane wrote: >> I looked through the callers of smgrdounlink(), and found that >> FinishPreparedTransaction passes isRedo = true. I'm not sure at the >> moment what the reasoning behind that was, but it does seem to mean that >> checking I

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 13:25 -0400, Tom Lane wrote: > Heikki Linnakangas writes: > > It's tempting to just remove the "!isRedo" condition, but then we have > > another problem: if bgwriter hasn't been started yet, and the shmem > > queue is full, we get stuck in register_unlink() trying to send th

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas writes: > Simon Riggs wrote: >> On Thu, 2009-06-25 at 12:43 -0400, Tom Lane wrote: >>> What about "revert the patch"? >> >> That's probably just as dangerous. > I don't feel comfortable either reverting such a big patch at last > minute. Yeah, I'm not very happy with that eit

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas writes: > It's tempting to just remove the "!isRedo" condition, but then we have > another problem: if bgwriter hasn't been started yet, and the shmem > queue is full, we get stuck in register_unlink() trying to send the > message and failing. > In archive recovery, we always s

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

2009-06-25 Thread Heikki Linnakangas
Simon Riggs wrote: > On Thu, 2009-06-25 at 12:43 -0400, Tom Lane wrote: > >> What about "revert the patch"? > > That's probably just as dangerous. I don't feel comfortable either reverting such a big patch at last minute. Would need a fair amount of testing to make sure that the revertion is cor

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 12:43 -0400, Tom Lane wrote: > What about "revert the patch"? That's probably just as dangerous. Much easier to just avoid that state altogether, if you must. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-bu

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 19:37 +0300, Heikki Linnakangas wrote: > Tom Lane wrote: > > Heikki Linnakangas writes: > >> This is completely untested still, but does anyone immediately see any > >> more problems? > > > > Isn't SetForwardFsyncRequests going to cause the final mdsync to fail? > > Yes, I

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas writes: > Tom Lane wrote: >> Isn't SetForwardFsyncRequests going to cause the final mdsync to fail? > Yes, I just noticed that myself, testing it for the first time. That > check needs to be suppressed in the startup checkpoint. Ugh. Better would be to move the responsibility

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 12:33 -0400, Tom Lane wrote: > Heikki Linnakangas writes: > > This is completely untested still, but does anyone immediately see any > > more problems? > > Isn't SetForwardFsyncRequests going to cause the final mdsync to fail? Yes it will, as it is now. I didn't read the

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

2009-06-25 Thread Heikki Linnakangas
Tom Lane wrote: > Heikki Linnakangas writes: >> This is completely untested still, but does anyone immediately see any >> more problems? > > Isn't SetForwardFsyncRequests going to cause the final mdsync to fail? Yes, I just noticed that myself, testing it for the first time. That check needs to

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas writes: > This is completely untested still, but does anyone immediately see any > more problems? Isn't SetForwardFsyncRequests going to cause the final mdsync to fail? regards, tom lane -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 19:20 +0300, Heikki Linnakangas wrote: > Simon Riggs wrote: > > > But we need to set LocalRecoveryInProgress appropriately > > also. > > Yeah, the trouble is to tell bgwriter that it's OK for it to create the > checkpoint, which includes writing a WAL record, while still ke

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

2009-06-25 Thread Heikki Linnakangas
Simon Riggs wrote: > On Thu, 2009-06-25 at 18:12 +0300, Heikki Linnakangas wrote: > >> A short fix would be to have bgwriter do the shutdown checkpoint instead >> in archive recovery. I don't recall if there was a reason it wasn't >> coded like that to begin with, though. > > I think the problem

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 18:57 +0300, Heikki Linnakangas wrote: > I came up with the attached patch, which includes Simon's patch to > have all fsync requests forwarded to bgwriter during archive recovery. > To fix the startup checkpoint issue, startup process requests a forced > restartpoint, which

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 18:12 +0300, Heikki Linnakangas wrote: > A short fix would be to have bgwriter do the shutdown checkpoint instead > in archive recovery. I don't recall if there was a reason it wasn't > coded like that to begin with, though. I think the problem was that it was coded both wa

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

2009-06-25 Thread Heikki Linnakangas
Tom Lane wrote: > Heikki Linnakangas writes: >> Heikki Linnakangas wrote: >>> Hmm, what happens when the startup process performs a write, and >>> bgwriter is not running? Do the fsync requests queue up in the shmem >>> queue until the end of recovery when bgwriter is launched? I guess I'll >>> ha

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 11:31 -0400, Tom Lane wrote: > Heikki Linnakangas writes: > > Heikki Linnakangas wrote: > >> Hmm, what happens when the startup process performs a write, and > >> bgwriter is not running? Do the fsync requests queue up in the shmem > >> queue until the end of recovery when b

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 17:02 +0300, Heikki Linnakangas wrote: > I think the real problem is this in mdunlink(): > > > /* Register request to unlink first segment later */ > > if (!isRedo && forkNum == MAIN_FORKNUM) > > register_unlink(rnode); > > When we replay the unlink of

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas writes: > Heikki Linnakangas wrote: >> Hmm, what happens when the startup process performs a write, and >> bgwriter is not running? Do the fsync requests queue up in the shmem >> queue until the end of recovery when bgwriter is launched? I guess I'll >> have to try it out... >

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

2009-06-25 Thread Heikki Linnakangas
Tom Lane wrote: > Heikki Linnakangas writes: >> In archive recovery, we always start bgwriter at the beginning of WAL >> replay. In crash recovery, we don't start bgwriter until the end of wAL >> replay. So we could change the "!isRedo" condition to >> "!InArchiveRecovery". It's not a very clean s

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 17:35 +0300, Heikki Linnakangas wrote: > Heikki Linnakangas wrote: > > Hmm, what happens when the startup process performs a write, and > > bgwriter is not running? Do the fsync requests queue up in the shmem > > queue until the end of recovery when bgwriter is launched? I gu

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 12:55 +, Fujii Masao wrote: > The restartpoint by bgwriter in recovery mode caused the following error. > > ERROR: could not fsync segment 0 of relation base/11564/16422_fsm: No > such file or directory I think I see a related bug also. register_dirty_segment() r

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

2009-06-25 Thread Tom Lane
Heikki Linnakangas writes: > In archive recovery, we always start bgwriter at the beginning of WAL > replay. In crash recovery, we don't start bgwriter until the end of wAL > replay. So we could change the "!isRedo" condition to > "!InArchiveRecovery". It's not a very clean solution, but it's simp

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

2009-06-25 Thread Heikki Linnakangas
Heikki Linnakangas wrote: > Hmm, what happens when the startup process performs a write, and > bgwriter is not running? Do the fsync requests queue up in the shmem > queue until the end of recovery when bgwriter is launched? I guess I'll > have to try it out... Oh dear, doesn't look good. The star

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

2009-06-25 Thread Heikki Linnakangas
Simon Riggs wrote: > On Thu, 2009-06-25 at 12:55 +, Fujii Masao wrote: >> The following bug has been logged online: >> >> Bug reference: 4879 >> Logged by: Fujii Masao >> Email address: masao.fu...@gmail.com >> PostgreSQL version: 8.4dev >> Operating system: RHEL5.1 x86_64

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

2009-06-25 Thread Simon Riggs
On Thu, 2009-06-25 at 12:55 +, Fujii Masao wrote: > The following bug has been logged online: > > Bug reference: 4879 > Logged by: Fujii Masao > Email address: masao.fu...@gmail.com > PostgreSQL version: 8.4dev > Operating system: RHEL5.1 x86_64 > Description:bgwr

[BUGS] BUG #4879: bgwriter fails to fsync the file in recovery mode

2009-06-25 Thread Fujii Masao
The following bug has been logged online: Bug reference: 4879 Logged by: Fujii Masao Email address: masao.fu...@gmail.com PostgreSQL version: 8.4dev Operating system: RHEL5.1 x86_64 Description:bgwriter fails to fsync the file in recovery mode Details: The restartpo