On Thursday, October 14, 2021 11:21 AM Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > At Mon, 11 Oct 2021 15:27:41 +0900, Masahiko Sawada > <sawada.m...@gmail.com> wrote in > > > > On Fri, Oct 8, 2021 at 4:50 PM Kyotaro Horiguchi > > <horikyota....@gmail.com> wrote: > > > I came up with the third way. SnapBuildCommitTxn already properly > > > handles the case where a ReorderBufferTXN with > > > RBTXN_HAS_CATALOG_CHANGES. So this issue can be resolved by > create > > > such ReorderBufferTXNs in SnapBuildProcessRunningXacts. > > > > Thank you for the idea and patch! > > > > It's much simpler than mine. I think that creating an entry of a > > catalog-changed transaction in the reorder buffer before > > SunapBuildCommitTxn() is the right direction. > > > > After more thought, given DDLs are not likely to happen than DML in > > practice, probably we can always mark both the top transaction and its > > subtransactions as containing catalog changes if the commit record has > > XACT_XINFO_HAS_INVALS? I believe this is not likely to lead to > > overhead in practice. That way, the patch could be more simple and > > doesn't need the change of AssertTXNLsnOrder(). > > > > I've attached another PoC patch. Also, I've added the tests for this > > issue in test_decoding. > > Thanks for the test script. (I did that with TAP framework but isolation > tester > version is simpler.) > > It adds a call to ReorderBufferAssignChild but usually subtransactions are > assigned to top level elsewherae. Addition to that > ReorderBufferCommitChild() called just later does the same thing. We are > adding the third call to the same function, which looks a bit odd. It can be odd. However, we have a check at the top of ReorderBufferAssignChild to judge if the sub transaction is already associated or not and skip the processings if it is.
> And I'm not sure it is wise to mark all subtransactions as "catalog changed" > always when the top transaction is XACT_XINFO_HAS_INVALS. In order to avoid this, can't we have a new flag (for example, in reorderbuffer struct) to check if we start decoding from RUNNING_XACTS, which is similar to the first patch of [1] and use it at DecodeCommit ? This still leads to some extra specific codes added to DecodeCommit and this solution becomes a bit similar to other previous patches though. [1] - https://www.postgresql.org/message-id/81D0D8B0-E7C4-4999-B616-1E5004DBDCD2%40amazon.com Best Regards, Takamichi Osumi
