On Wed, Jul 20, 2022 at 12:11 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, Jul 19, 2022 at 7:28 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > On Tue, Jul 19, 2022 at 9:25 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > On Tue, Jul 19, 2022 at 1:10 PM Masahiko Sawada <sawada.m...@gmail.com> > > > wrote: > > > > > > > > > > > BTW on backbranches, I think that the reason why we add > > > > initial_running_xacts stuff to ReorderBuffer is that we cannot modify > > > > SnapBuild that could be serialized. Can we add a (private) array for > > > > the initial running xacts in snapbuild.c instead of adding new > > > > variables to ReorderBuffer? > > > > > > > > > > While thinking about this, I wonder if the current patch for back > > > branches can lead to an ABI break as it changes the exposed structure? > > > If so, it may be another reason to change it to some other way > > > probably as you are suggesting. > > > > Yeah, it changes the size of ReorderBuffer, which is not good. > > > > So, are you planning to give a try with your idea of making a private > array for the initial running xacts?
Yes. > I am not sure but I guess you are > proposing to add it in SnapBuild structure, if so, that seems safe as > that structure is not exposed. We cannot add it in SnapBuild as it gets serialized to the disk. > > > Changing the function names and arguments would also break ABI. So > > probably we cannot do the above idea of removing > > ReorderBufferInitialXactsSetCatalogChanges() as well. > > > > Why do you think we can't remove > ReorderBufferInitialXactsSetCatalogChanges() from the back branch > patch? I think we don't need to change the existing function > ReorderBufferXidHasCatalogChanges() but instead can have a wrapper > like SnapBuildXidHasCatalogChanges() similar to master branch patch. IIUC we need to change SnapBuildCommitTxn() but it's exposed. Currently, we call like DecodeCommit() -> SnapBuildCommitTxn() -> ReorderBufferXidHasCatalogChanges(). If we have a wrapper function, we call like DecodeCommit() -> SnapBuildCommitTxn() -> SnapBuildXidHasCatalogChanges() -> ReorderBufferXidHasCatalogChanges(). In SnapBuildXidHasCatalogChanges(), we need to check if the transaction has XACT_XINFO_HAS_INVALS, which means DecodeCommit() needs to pass either parsed->xinfo or (parsed->xinfo & XACT_XINFO_HAS_INVALS != 0) down to SnapBuildXidHasCatalogChanges(). However, since SnapBuildCommitTxn(), between DecodeCommit() and SnapBuildXidHasCatalogChanges(), is exposed we cannot change it. Another idea would be to have functions, say SnapBuildCommitTxnWithXInfo() and SnapBuildCommitTxn_ext(). The latter does actual work of handling transaction commits and both SnapBuildCommitTxn() and SnapBuildCommit() call SnapBuildCommitTxnWithXInfo() with different arguments. Regards, -- Masahiko Sawada EDB: https://www.enterprisedb.com/
