On Tue, Feb 9, 2016 at 5:37 AM, Michael Paquier <michael.paquier@gmail.com> wrote: > > On Mon, Feb 8, 2016 at 11:24 PM, Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Mon, Feb 8, 2016 at 12:28 PM, Michael Paquier <michael.paquier@gmail.com> > > wrote: > >> > >> > >> >> /* > >> >> + * Fetch the progress position before taking any WAL insert lock. > >> >> This > >> >> + * is normally an operation that does not take long, but leaving > >> >> this > >> >> + * lookup out of the section taken an exclusive lock saves a > >> >> couple > >> >> + * of instructions. > >> >> + */ > >> >> + progress_lsn = GetProgressRecPtr(); > >> > > >> > too long for my taste. How about: > >> > /* get progress, before acuiring insert locks to shorten locked section > >> > */ > >> > >> Check. > >> > > > > What is the need of holding locks one-by-one during checkpoint when > > we anyway are going to take lock on all the insertion slots. > > A couple of records can slip in while scanning the progress LSN > through all the locks. >
Do you see any benefit in allowing checkpoints for such cases considering
bgwriter will anyway take care of logging standby snapshot for such
cases?
I don't think there is any reasonable benefit by improving the situation of
idle-system check for checkpoint other than just including
standbysnapshot WAL record. OTOH as checkpoint is not so seldom
operation, so allowing such a change should be okay, but I don't see