From: Fujii Masao <masao.fujii@gmail.com>
To: Simon Riggs <simon@2ndquadrant.com>
Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [PATCHES] Infrastructure changes for recovery (v8)
Date: Tue, 18 Nov 2008 10:53:13 +0900
Message-ID: <3f0b79eb0811171753u76c2b78du8e77acfc3b5280c0@mail.gmail.com> (raw)
In-Reply-To: <1226936382.3790.19.camel@ebony.2ndQuadrant>
References: <1222815151.4445.1397.camel@ebony.2ndQuadrant>
<48EC9CC5.6060803@enterprisedb.com>
<1223468660.4747.294.camel@ebony.2ndQuadrant>
<1223472898.4747.310.camel@ebony.2ndQuadrant>
<49217D3F.9050603@enterprisedb.com>
<1226936382.3790.19.camel@ebony.2ndQuadrant>
On Tue, Nov 18, 2008 at 12:39 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>
> On Mon, 2008-11-17 at 16:18 +0200, Heikki Linnakangas wrote:
>> Simon Riggs wrote:
>> > @@ -3845,6 +3850,52 @@ sigusr1_handler(SIGNAL_ARGS)
>> >
>> > PG_SETMASK(&BlockSig);
>> >
>> > + if (CheckPostmasterSignal(PMSIGNAL_RECOVERY_START))
>> > + {
>> > + Assert(pmState == PM_STARTUP);
>> > +
>> > + /*
>> > + * Go to shutdown mode if a shutdown request was pending.
>> > + */
>> > + if (Shutdown > NoShutdown)
>> > + {
>> > + pmState = PM_WAIT_BACKENDS;
>> > + /* PostmasterStateMachine logic does the rest */
>> > + }
>> > + else
>> > + {
>> > + /*
>> > + * Startup process has entered recovery
>> > + */
>> > + pmState = PM_RECOVERY;
>>
>>
>> Hmm, I smell a race condition here:
>>
>> 1. Startup process goes into consistent state, and signals postmaster
>> 2. Startup process finishes WAL replay and dies
>> 3. Postmaster wakes up in reaper(), noting that the startup process
>> dies, and goes into PM_RUN mode.
>> 4. The above signal handler for postmaster is run, causing an assertion
>> failure, or putting postmaster back into PM_RECOVERY mode if assertions
>> are disabled.
>>
>> Highly unlikely in practice, given how much code needs to run in the
>> startup process between signaling the postmaster and exiting, but it
>> seems theoretically possible. Do we care, and if we do, how can we fix it?
>
> Might be possible - it does depend on the sequence of actions its true.
> Agree not likely to happen, except as the result of another bug.
>
> I'll change it to a test for
>
> if (pmState == PM_STARTUP)
> pmState = PM_RECOVERY;
Likewise, should we also change the assertion against the pid of the
background process (BgWriterPID, PgStatPID)?
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-patches@postgresql.org
Cc: masao.fujii@gmail.com, simon@2ndquadrant.com, heikki.linnakangas@enterprisedb.com, pgsql-hackers@postgresql.org
Subject: Re: [PATCHES] Infrastructure changes for recovery (v8)
In-Reply-To: <3f0b79eb0811171753u76c2b78du8e77acfc3b5280c0@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox