Re: Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby

On Mon, Feb 8, 2016 at 11:24 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Mon, Feb 8, 2016 at 12:28 PM, Michael Paquier <michael(dot)paquier(at)gmail(dot)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.

> + * to not rely on taking an exclusive lock an all the WAL insertion locks,
>
> /an all/on all

Nice catch.
--
Michael