On 2016-02-02 10:12:25 +0530, Amit Kapila wrote: > @@ -8239,7 +8262,7 @@ CreateCheckPoint(int flags) > if ((flags & (CHECKPOINT_IS_SHUTDOWN | > CHECKPOINT_END_OF_RECOVERY | > CHECKPOINT_FORCE)) == 0) > { > - if > (prevPtr == ControlFile->checkPointCopy.redo && > + if (GetProgressRecPtr() == prevPtr && > > prevPtr / XLOG_SEG_SIZE == curInsert / XLOG_SEG_SIZE) > { > > I think such a check won't consider all the WAL-activity happened > during checkpoint (after checkpoint start, till it finishes) which was > not the intention of this code without your patch.
Precisely. > I think both this and previous patch (hs-checkpoints-v1 ) approach > won't fix the issue in all kind of scenario's. Agreed. > Let me try to explain what I think should fix this issue based on > discussion above, suggestions by Andres and some of my own > thoughts: > Have a new variable in WALInsertLock that would indicate the last > insertion position (last_insert_pos) we write to after holding that lock. > Ensure that we don't update last_insert_pos while logging standby > snapshot (simple way is to pass a flag in XLogInsert or distinguish > it based on type of record (XLOG_RUNNING_XACTS) or if you can > think of a more smarter way). Now both during checkpoint and > in bgwriter, to find out whether there is any activity since last > time, we need to check all the WALInsertLocks for latest insert position > (by referring last_insert_pos) and compare it with the latest position > we have noted during last such check and decide whether to proceed > or not based on comparison result. If you think it is not adequate to > store last_insert_pos in WALInsertLock, then we can think of having > it in PGPROC. Yes, that's pretty much what I was thinking of. > Yet another idea that occurs to me this morning is that why not > have a variable in shared memory in XLogCtlInsert or XLogCtl > similar to CurrBytePos/PrevBytePos which will be updated on > each XLOGInsert apart from the XLOGInsert for standby snapshots > and use that in a patch somewhat close to what you have in > hs-checkpoints-v1. That'll require holding locks longer... Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers