Re: END_OF_RECOVERY shutdowns and ResetUnloggedRelations()

On Thu, Sep 18, 2014 at 4:31 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> On 2014-09-12 14:44:48 -0400, Robert Haas wrote:
>> On Fri, Sep 12, 2014 at 7:22 AM, Andres Freund <andres@2ndquadrant.com> wrote:
>> > What I like even less is that pg_control is actually marked as
>> > DB_SHUTDOWNED due to END_OF_RECOVERY. That's just plain wrong. Obviously
>> > the database was *NOT* shutdown peacefully. I don't see active bugs due
>> > it besides this, but I think it's likely to either have or create futher
>> > ones.
>>
>> I agree.  The database clearly isn't shut down at end of recovery;
>> it's still active and we're still doing things to it.  If we crash at
>> that point, we need to go back into recovery on restart.  This seems
>> open and shut, but maybe I'm missing something.  Why shouldn't we fix
>> *that*?
>
> Well, I think we might want to do both. There doesn't seem to be a
> reason to *not* do the ResetUnloggedRelation(UNLOGGED_RELATION_INIT)
> around the ShutdownWalRcv(). That seems much closer where it, for me,
> logically belongs. And it'd fix the concrete problem.

That might be all right.  I've booted enough things in this area of
the code to not have a lot of confidence in my ability to not boot
things in this area of the code ... but I don't see a problem with it.
It does seem a little odd to do that before checking whether we
reached the minimum recovery point, or deciding whether to assign a
new TLI, but I don't see a concrete problem with putting it there.

>> With regard to your second email, I agree that
>> ResetUnloggedRelation(UNLOGGED_RELATION_INIT) needs to issue fsyncs.
>
> Good.
>
> The topic reminds me of the fact that I actually think at the very least
> pg_xlog/ and pg_control needs the same treatment. Consider the following
> sequence:
> 1) postgres writes loads of stuff. Lots of it to *not* fsynced WAL
>    segments
> 2) postgres crashes and crash recovery happens. Replays *not* fsynced
>    WAL
> 3) the OS crashes
> 4) Bad. We now might hava pg_control with a minRecovery that's *later*
>    than some potentially unsynced WAL segments
>
> I think the easiest would be to just fsync() the entire data directory
> at the start when ControlFile->state !=
> DB_SHUTDOWNED/DB_SHUTDOWNED_IN_RECOVERY
>
> Note that that's independent of the fsync for unlogged relations.

Seems reasonable.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company