Standalone backends run StartupXLOG in an incorrect environment

I realized the truth of $SUBJECT while reading this report:
http://archives.postgresql.org/pgsql-general/2009-09/msg00712.php

In a standalone backend, postgres.c tries to run StartupXLOG after
having done only BaseInit(), which means that we don't have a PGPROC
(hence can't take LWLocks much less heavyweight locks) and we have not
totally finished initializing the bufmgr either. This is apparently
enough for the "normal" case where there's no log replay to do; but
as the above report shows, it's completely inadequate for some of the
more complex code paths in replay. I suspect this has been broken
from the beginning.

Fixing this will require rearranging things around InitPostgres
(in particular, I think InitBufferPoolBackend will have to be
called directly from postgres.c). Since that code got rearranged
quite a bit last month, I'd be hesitant to try to back-patch whatever
fix we come up with for HEAD. Seeing that we'd never noticed the
problem before, I think it's okay to fix it just in HEAD and not
risk back-patching ... comments?

Also, does this have any impact on the Hot Standby stuff?

regards, tom lane

Tom Lane wrote:

> Fixing this will require rearranging things around InitPostgres
> (in particular, I think InitBufferPoolBackend will have to be
> called directly from postgres.c). Since that code got rearranged
> quite a bit last month, I'd be hesitant to try to back-patch whatever
> fix we come up with for HEAD. Seeing that we'd never noticed the
> problem before, I think it's okay to fix it just in HEAD and not
> risk back-patching ... comments?

So if you need to enter standalone mode, you'll have to start
postmaster, wait for replay to finish, stop it and restart standalone.
Would this be a problem when you need standalone mode in an emergency,
for example when the database won't start due to Xid wraparound?
Frankly I don't have a problem with no backpatching but ...

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> So if you need to enter standalone mode, you'll have to start
> postmaster, wait for replay to finish, stop it and restart standalone.

Yeah, that's the case at the moment.

> Would this be a problem when you need standalone mode in an emergency,
> for example when the database won't start due to Xid wraparound?

If it won't run through StartupXLOG under the postmaster, it's not going
to do so standalone either.

regards, tom lane

On Monday 21 September 2009 14:24:07 Tom Lane wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> > So if you need to enter standalone mode, you'll have to start
> > postmaster, wait for replay to finish, stop it and restart standalone.
>
> Yeah, that's the case at the moment.
>
> > Would this be a problem when you need standalone mode in an emergency,
> > for example when the database won't start due to Xid wraparound?
>
> If it won't run through StartupXLOG under the postmaster, it's not going
> to do so standalone either.
>

Hmm... istr cases where I couldn't startup regular postgres but could in
stand-alone mode that had system indexes disabled...I could be misremembering
that so that the postmaster would start, I just couldn't connect unless in
stand-alone. In any case this does seem less than ideal, but if there aren't
any better options...

--
Robert Treat
Conjecture: http://www.xzilla.net
Consulting: http://www.omniti.com

On Sat, 2009-09-19 at 13:21 -0400, Tom Lane wrote:
> I realized the truth of $SUBJECT while reading this report:
> http://archives.postgresql.org/pgsql-general/2009-09/msg00712.php

...

> Also, does this have any impact on the Hot Standby stuff?

It could potentially, but there is not much HS-related code in the early
boot sequence. It's mostly StartupXLog() or later.

HS won't work in standalone backends, by definition, so I don't see too
much to worry me.

[This is an open item for 9.0, hence the response to an apparently old
hackers thread]

--
Simon Riggs www.2ndQuadrant.com

Simon Riggs wrote:
> On Sat, 2009-09-19 at 13:21 -0400, Tom Lane wrote:
> > I realized the truth of $SUBJECT while reading this report:
> > http://archives.postgresql.org/pgsql-general/2009-09/msg00712.php
>
> ...
>
> > Also, does this have any impact on the Hot Standby stuff?
>
> It could potentially, but there is not much HS-related code in the early
> boot sequence. It's mostly StartupXLog() or later.
>
> HS won't work in standalone backends, by definition, so I don't see too
> much to worry me.
>
> [This is an open item for 9.0, hence the response to an apparently old
> hackers thread]

Thanks for the reply; 9.0 open item removed.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do

Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> [This is an open item for 9.0, hence the response to an apparently old
>> hackers thread]

> Thanks for the reply; 9.0 open item removed.

I think you misread his reply. Please put that back.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> >> [This is an open item for 9.0, hence the response to an apparently old
> >> hackers thread]
>
> > Thanks for the reply; 9.0 open item removed.
>
> I think you misread his reply. Please put that back.

OK, I re-read it and still don't understand, but I don't have to.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do

On Mon, Mar 22, 2010 at 9:32 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> Tom Lane wrote:
>> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> >> [This is an open item for 9.0, hence the response to an apparently old
>> >> hackers thread]
>>
>> > Thanks for the reply;  9.0 open item removed.
>>
>> I think you misread his reply.  Please put that back.
>
> OK,  I re-read it and still don't understand, but I don't have to.

I re-read it too and I don't understand either. This is LISTED as an
open item for 9.0, but it is apparently not a new regression, so I
think we should move it to the Todo list instead. This problem was
discovered six months ago, is not a new regression, and there is
apparently no movement toward a fix, so it doesn't make sense to me
that we should hold up either 9.0 beta or 9.0 final on account of it.
Or am I confused?

....Robert

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Mon, Mar 22, 2010 at 9:32 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>> OK, I re-read it and still don't understand, but I don't have to.

> I re-read it too and I don't understand either.

The point is that a standalone backend will fail to execute recovery
correctly:
http://archives.postgresql.org/pgsql-hackers/2009-09/msg01297.php
This is bad enough now but seems likely to be an even bigger foot-gun
in an HS/SR world.

> This is LISTED as an
> open item for 9.0, but it is apparently not a new regression, so I
> think we should move it to the Todo list instead. This problem was
> discovered six months ago, is not a new regression, and there is
> apparently no movement toward a fix, so it doesn't make sense to me
> that we should hold up either 9.0 beta or 9.0 final on account of it.

If you think we're at the point where this item is the main thing
standing between us and beta, I'll go do something about it. I've
been waiting for the HS code to settle before trying to design a
solution...

regards, tom lane

On Mon, 2010-04-19 at 11:19 -0400, Tom Lane wrote:

> If you think we're at the point where this item is the main thing
> standing between us and beta, I'll go do something about it. I've
> been waiting for the HS code to settle before trying to design a
> solution...

I'm not hugely interested in supporting HS in standalone backends. If it
works, that's nice. If it doesn't then I suggest we don't hold up beta
for it.

--
Simon Riggs www.2ndQuadrant.com

On Mon, Apr 19, 2010 at 11:19 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Mon, Mar 22, 2010 at 9:32 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>>> OK,  I re-read it and still don't understand, but I don't have to.
>
>> I re-read it too and I don't understand either.
>
> The point is that a standalone backend will fail to execute recovery
> correctly:
> http://archives.postgresql.org/pgsql-hackers/2009-09/msg01297.php
> This is bad enough now but seems likely to be an even bigger foot-gun
> in an HS/SR world.

OK.

>> This is LISTED as an
>> open item for 9.0, but it is apparently not a new regression, so I
>> think we should move it to the Todo list instead.  This problem was
>> discovered six months ago, is not a new regression, and there is
>> apparently no movement toward a fix, so it doesn't make sense to me
>> that we should hold up either 9.0 beta or 9.0 final on account of it.
>
> If you think we're at the point where this item is the main thing
> standing between us and beta, I'll go do something about it.  I've
> been waiting for the HS code to settle before trying to design a
> solution...

I'm not sure if this is the main thing, but I think it's probably in the top 5.

At present there are 8 items (not counting documentation issues) listed at:

http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items

...not all of which seem likely to get fixed, and probably 1-3
additional patches that are floating around out there without having
formally gotten added to the list. I think it's realistic to think
that we could be within 10 commits of beta.

...Robert

Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> On Mon, 2010-04-19 at 11:19 -0400, Tom Lane wrote:
>> If you think we're at the point where this item is the main thing
>> standing between us and beta, I'll go do something about it. I've
>> been waiting for the HS code to settle before trying to design a
>> solution...

> I'm not hugely interested in supporting HS in standalone backends.

I'm not either. The type of scenario that I'm worried about is someone
trying to use a standalone backend to clean up after a recovery failure.
Right now I'm afraid that could make things worse (ie create additional
corruption).

regards, tom lane

I wrote:
> The point is that a standalone backend will fail to execute recovery
> correctly:
> http://archives.postgresql.org/pgsql-hackers/2009-09/msg01297.php

After digging around a bit, it seems like the cleanest solution would be
to move the responsibility for calling StartupXLOG in a standalone
backend into InitPostgres. At the point where the latter currently has

/*
* Initialize local process's access to XLOG, if appropriate. In
* bootstrap case we skip this since StartupXLOG() was run instead.
*/
if (!bootstrap)
(void) RecoveryInProgress();

we'd add a couple of lines to call StartupXLOG if !IsUnderPostmaster,
and then remove the call from postgres.c. I haven't tested this yet
but it looks like the correct state has been set up at that point.
Anyone see any obvious holes in the idea?

regards, tom lane

On Mon, 2010-04-19 at 14:34 -0400, Tom Lane wrote:

> Anyone see any obvious holes in the idea?

Nothing springs to mind, so worth a try.

--
Simon Riggs www.2ndQuadrant.com

BTW, just for the archives' sake: it took a good long time to develop a
reproducible test case for this. It seems that 99% of the WAL replay
code does *not* depend on the missing state. I was eventually able to
reproduce the case originally reported, namely a crash during
btree_xlog_cleanup; but to get that you need (a) WAL to end between a
btree page split and insertion of the new parent record, and (b) have
the resulting insertion need to obtain a new btree page, ie there's
another split or newroot forced then, and (c) not have any available
pages in the index's FSM, so that we have to LockRelationForExtension,
which is what crashes for lack of a PGPROC.

So that probably explains why we went so long without recognizing the
bug.

This again points up the fact that WAL recovery isn't as well tested
as one could wish. Koichi-san's efforts to create a test with 100%
coverage of all types of WAL records are good, but that'd not have
helped us to find this. We should think about ways to provide better
test coverage of end-of-WAL cleanup.

regards, tom lane