On Thu, Sep 18, 2014 at 4:31 AM, Andres Freund <and...@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 <and...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers