Re: [HACKERS] WAL logging problem in 9.4.3?

Hello.

At Mon, 19 Aug 2019 18:59:59 +0900 (Tokyo Standard Time), Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> wrote in <20190819(dot)185959(dot)118543656(dot)horikyota(dot)ntt(at)gmail(dot)com>
> > The comment material being deleted is still correct, so don't delete it.
> > Moreover, the code managing rd_firstRelfilenodeSubid has a similar bug. The
> > attached patch adds an assertion that RelationNeedsWAL() and the
> > pendingDeletes array have the same opinion about the relfilenode, and it
> > expands a test case to fail that assertion.
>
> (Un?)Fortunately, that doesn't fail.. (with rebased version on
> the recent master) I'll recheck that tomorrow.

I saw the assertion failure. It's a part of intended
behavior. In this patch, relcache doesn't hold the whole history
of relfilenodes so we cannot remove useless pending syncs
perfectly. On the other hand they are harmless except that they
cause extra sync of files that are removed immediately. So I
choosed that once registered pending syncs are not removed.

If we want consistency here, we need to record creator subxid in
PendingRelOps (PendingRelDelete) struct and rather large work at
subtransaction end.

> > > --- a/src/include/utils/rel.h
> > > +++ b/src/include/utils/rel.h
> > > @@ -74,11 +74,13 @@ typedef struct RelationData
> > > SubTransactionId rd_createSubid; /* rel was created in current xact */
> > > SubTransactionId rd_newRelfilenodeSubid; /* new relfilenode assigned in
> > > * current xact */
> > > + SubTransactionId rd_firstRelfilenodeSubid; /* new relfilenode assigned
> > > + * first in current xact */
> >
> > In general, to add a field like this, run "git grep -n 'rd_.*Subid'" and audit
> > all the lines printed. Many bits of code need to look at all three,
> > e.g. RelationClose().
>
> Agreed. I'll recheck that.
>
> > This field needs to be 100% reliable. In other words,
> > it must equal InvalidSubTransactionId if and only if the relfilenode matches
> > the relfilenode that would be in place if the top transaction rolled back.
>
> I don't get this. I think the variable moves as you suggested. It
> is handled same way with fd_new* in AtEOSubXact_cleanup but the
> difference is in assignment but rollback. rd_fist* won't change
> after the first assignment so rollback of the subid means
> relfilenode is also rolled back to the initial value at the
> beginning of the top transaction.

So I'll add this in the next version to see how it looks.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center