On Tue, Feb 9, 2016 at 6:08 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: > > On Tue, Feb 9, 2016 at 2:24 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: > > Do you see any benefit in allowing checkpoints for such cases considering > > bgwriter will anyway take care of logging standby snapshot for such > > cases? > > Well, the idea is to improve the system responsiveness. Imagine that > the call to GetProgressRecPtr() is done within the exclusive lock > portion, but that while scanning the WAL insert lock entries another > backend is waiting to take a lock on them, and this backend is trying > to insert the first record that updates the progress LSN since the > last checkpoint. In this case the checkpoint would be skipped. > However, imagine that such a record is able to get through it while > scanning the progress values in the WAL insert locks, in which case a > checkpoint would be generated.
Such case was not covered without your patch and I don't see the need of same especially at the cost of taking locks. > In any case, I think that we should try > to get exclusive lock areas as small as possible if we have ways to do > so. > Sure, but not when you are already going to take lock for longer duration. - last_snapshot_lsn != GetXLogInsertRecPtr()) + GetLastCheckpointRecPtr() < GetProgressRecPtr()) How the above check in patch suppose to work? I think it could so happen once bgwriter logs snapshot, both checkpoint and progressAt won't get updated any further and I think this check will allow to log snapshots in such a case as well. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
