On 2016-04-06 13:50:24 +0100, Simon Riggs wrote:
> On 6 April 2016 at 13:27, Andres Freund <andres@anarazel.de> wrote:
>
> > On 2016-04-06 13:11:40 +0100, Simon Riggs wrote:
> > > On 6 April 2016 at 10:09, Andres Freund <andres@anarazel.de> wrote:
> > > > On 2016-04-06 10:04:42 +0100, Simon Riggs wrote:
> > > > The issue there is that we continue to issue checkpoints if the only
> > > > activity since the last checkpoint was emitting a standby
> > > > snapshot. That's because:
> > > >
> > >
> > > I agree this is the current situation in 9.4 and 9.5, hence the bug
> > report.
> > >
> > > This no longer occurs with the patches I have proposed. The snapshot is
> > > skipped, so a further checkpoint never triggers.
> >
> > Not if there's a longrunning/idle transaction.
> >
> > Note that skipping the snapshot is actually a *problem* in some
> > cases. As I've brought up upthread, to which you never replied. A
> > xl_running_xacts->xcnt == 0/!overflowed snapshot can be very important
> > for hot standby, because it allows ProcArrayApplyRecoveryInfo() to
> > switch to INITIALIZED state:
> >
>
> I replied by posting a patch to address your concern, how is that non-reply?
It doesn't address the problem? It's irrelevant that the last snapshot
had 0 xacts, if you start recovery from a later check/restartpoint;
recovery won't process earlier running_xacts records.
> > This obviously doesn't apply to WAL_LEVEL_LOGICAL as is (the if). And it
> > also obviously repeats to log the same snapshot in a system where the
> > state hasn't changed, but where running->xcnt != 0 or nlocks != 0.
> My understanding from your previous comments was that it would be incorrect
> to do that.
I said:
> For one it breaks cleanup with logical decoding which does *NEED* to
> know that nothing is happening. Although only once, not repeatedly.
The salient point is "Although only once, not repeatedly.". Which is
pretty much same thing as for HS; to become consistent after a
checkpoint.
> Not true. I have listened to everything you've said and been patient with
> the high number of mistakes in your replies.
Simon, this is utterly ridiculous. Missing an if in a post-commit
review, of a hastily committed patch, which hasn't previously been
posted for review, is entirely normal.
Greetings,
Andres Freund