From: | "Fujii Masao" <masao(dot)fujii(at)gmail(dot)com> |
---|---|
To: | "Simon Riggs" <simon(at)2ndquadrant(dot)com> |
Cc: | "Heikki Linnakangas" <heikki(dot)linnakangas(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [PATCHES] Infrastructure changes for recovery (v8) |
Date: | 2008-12-29 06:06:58 |
Message-ID: | 3f0b79eb0812282206w5e930a54m8c9e7768cc3d819b@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers pgsql-patches |
Hi,
On Tue, Nov 18, 2008 at 12:39 AM, Simon Riggs <simon(at)2ndquadrant(dot)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;
>
> The assertion was mainly for documentation, its not protecting anything
> critical (IIRC).
This seems to have not been fixed yet in the latest patch.
http://archives.postgresql.org/message-id/494FF631.90908@enterprisedb.com
recovery-infra-separated-again-1.patch
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | Hitoshi Harada | 2008-12-29 07:08:28 | Re: TODO items for window functions |
Previous Message | Hitoshi Harada | 2008-12-29 04:18:35 | Re: Windowing Function Patch Review -> Standard Conformance |
From | Date | Subject | |
---|---|---|---|
Next Message | Simon Riggs | 2008-12-29 10:27:56 | Re: [PATCHES] Infrastructure changes for recovery (v8) |
Previous Message | Simon Riggs | 2008-12-23 09:54:33 | Re: [PATCHES] Infrastructure changes for recovery (v8) |