On Tue, Oct 18, 2022 at 6:29 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Mon, Oct 17, 2022 at 4:40 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > IIUC, here you are speaking of three different changes. Change-1: Add > > a check in AssertTXNLsnOrder() to skip assert checking till we reach > > start_decoding_at. Change-2: Set needs_timetravel to true in one of > > the else if branches in SnapBuildCommitTxn(). Change-3: Remove the > > call to ReorderBufferAssignChild() from SnapBuildXidSetCatalogChanges > > in PG-14/15 as that won't be required after Change-1. > > Yes. > > > > > AFAIU, Change-1 is required till v10; Change-2 and Change-3 are > > required in HEAD/v15/v14 to fix the problem. > > IIUC Change-2 is required in v16 and HEAD >
Why are you referring v16 and HEAD separately? > but not mandatory in v15 and > v14. The reason why we need Change-2 is that there is a case where we > mark only subtransactions as containing catalog change while not doing > that for its top-level transaction. In v15 and v14, since we mark both > subtransactions and top-level transaction in > SnapBuildXidSetCatalogChanges() as containing catalog changes, we > don't get the assertion failure at "Assert(!needs_snapshot || > needs_timetravel)". > > Regarding Change-3, it's required in v15 and v14 but not in HEAD and > v16. Since we didn't add SnapBuildXidSetCatalogChanges() to v16 and > HEAD, Change-3 cannot be applied to the two branches. > > > Now, the second and third > > changes are not required in branches prior to v14 because we don't > > record invalidations via XLOG_XACT_INVALIDATIONS record. However, if > > we want, we can even back-patch Change-2 and Change-3 to keep the code > > consistent or maybe just Change-3. > > Right. I don't think it's a good idea to back-patch Change-2 in > branches prior to v14 as it's not a relevant issue. > Fair enough but then why to even backpatch it to v15 and v14? > Regarding > back-patching Change-3 to branches prior 14, I think it may be okay > til v11, but I'd be hesitant for v10 as the final release comes in a > month. > So to fix the issue in all branches, what we need to do is to backpatch change-1: in all branches till v10, change-2: in HEAD, and change-3: in V15 and V14. Additionally, we think, it is okay to backpatch change-3 till v11 as it is mainly done to avoid the problem fixed by change-1 and it makes code consistent in back branches. I think because the test case proposed needs all three changes, we can push the change-1 without a test case and then as a second patch have change-2 for HEAD and change-3 for back branches with the test case. Do you have any other ideas to proceed here? -- With Regards, Amit Kapila.
