Re: Duplicated LSN in ReorderBuffer

On Fri, Jan 31, 2020 at 12:35 AM Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
>
> On 2019-Jul-26, Andres Freund wrote:
>
> > Petr, Simon, see the potential issue related to fast forward at the
> > bottom.
>
> I think we neglected this bit. I looked at the patch Simon submitted
> downthread, and while I vaguely understand that we need to process
> NEW_CID records during fast-forwarding,
>

Right, IIUC, that is mainly to mark txn has catalog changes
(ReorderBufferXidSetCatalogChanges) so that later such a transaction
can be used to build historic snapshots (during SnapBuildCommitTxn).
Now, if that is true, then won't we need a similar change in
DecodeHeapOp for XLOG_HEAP_INPLACE case as well? Also, I am not sure
if SnapBuildProcessNewCid needs to create Cid map in such a case.

> I don't quite understand why we
> still can skip XLOG_INVALIDATION messages. I *think* we should process
> those too.
>

I also think so. If you are allowing to execute invalidation
irrespective of fast_forward in DecodeStandbyOp, then why not do the
same in DecodeCommit where we add
ReorderBufferAddInvalidations?

> Here's a patch that also contains that change; I also
> reworded Simon's proposed comment. I appreciate reviews.
>

Even though, in theory, the changes look to be in the right direction,
but it is better if we can create a test case to reproduce the
problem. I am not sure, but I think we need to generate a few DDLs
for the transaction for which we want to fast forward and then after
moving forward the DMLs dependent on those WAL should create some
problem as we have skipped executing invalidations and addition of
such a transaction in the historic snapshot.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com