On Wed, Feb 3, 2016 at 3:08 PM, Andres Freund <andres@anarazel.de> wrote: > > 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. >
I think generally it is good idea, but one thing worth a thought is that
by doing so, we need to acquire all WAL Insertion locks every
LOG_SNAPSHOT_INTERVAL_MS to check the last_insert_pos for
every slot, do you think it is matter of concern in any way for write
workloads or it won't effect as we need to do this periodically?
