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.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.

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

Nice catch.
-- 
Michael