On Tue, Feb 9, 2016 at 6:08 PM, Michael Paquier <
michael.paquier@gmail.com> wrote:
>
> On Tue, Feb 9, 2016 at 2:24 PM, Amit Kapila <
amit.kapila16@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.