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

On Thu, Feb 11, 2016 at 5:46 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Thu, Feb 11, 2016 at 5:20 PM, Andres Freund <andres@anarazel.de> wrote:
>> On 2016-02-11 09:25:30 +0530, Amit Kapila wrote:
>>> On Wed, Feb 10, 2016 at 1:42 PM, Michael Paquier <michael.paquier@gmail.com>
>>> wrote:
>>> >
>>> > On Wed, Feb 10, 2016 at 5:00 PM, Amit Kapila <amit.kapila16@gmail.com>
>>> wrote:
>>> > > Okay, but isn't it better that we remove the snapshot taken
>>> > > at checkpoint time in the main branch or till where this code is
>>> > > getting back-patched.   Do you see any need of same after
>>> > > having the logging of snapshot in bgwriter?
>>> >
>>> > But this one is necessary as well to allow hot standby faster to
>>> > initialize, no? Particularly in the case where a bgwriter snapshot
>>> > would have been taken just before the checkpoint, there may be up to
>>> > 15s until the next one.
>>> >
>>>
>>> It could be helpful if checkpoint is done at shorter intervals, otherwise
>>> we anyway log it at 15s interval and if need faster initialisation
>>> of hot-standby, then it is better to reduce the log snapshot interval
>>> in bgwriter.
>>
>> No. By emitting the first snapshot directly after the determination of
>> the redo pointer, we can get into STANDBY_SNAPSHOT_READY more
>> quickly. Especially if some of the snapshots are overflowed. During
>> startup 15s can be a *long* time; but on the other hand there's not much
>> benefit at logging snapshots more frequently when the system is up.
>> I don't think we should tinker with the frequency/logging points, while
>> fixing the issue here.
>
> Agreement from here on this point. Andres, what's still remaining on
> my side is to know how to we detect in XLoginsert() how we update the
> LSN progress. I was willing to add XLogInsertExtended() with a
> complementary int/bool flag and let XLogInsert() alone as you don't
> like much doing this decision making using just the record type as I
> did in one of the patch upthread, however you did not like this idea
> either. What exactly do you have in mind?

OK, here is attached a new version that I hope addresses all the
points raised until now. The following things are changed:
- Extend XLogInsert with a new uint8 argument to have flags. As of now
there is one flag: XLOG_INSERT_NO_PROGRESS that can be set to not
update the progress. By default, the progress LSN is updated.
- Add extra check in bgwriter to not log a snapshot to be sure that no
snapshots are logged when there is no activity since last snapshot
taken, and since last checkpoint.
--
Michael