On Tue, Feb 9, 2016 at 5:37 AM, Michael Paquier <michael.paqu...@gmail.com> wrote: > > On Mon, Feb 8, 2016 at 11:24 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Feb 8, 2016 at 12:28 PM, Michael Paquier < michael.paqu...@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 the need for same. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com