On Fri, Nov 25, 2022 at 5:30 PM Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > > Hi Hou, > Thanks for the patch. With a simple condition, we can eliminate the > need to queueing snapshot change in the current transaction and then > applying it. Saves some memory and computation. This looks useful. > > When the queue snapshot change is processed in > ReorderBufferProcessTXN(), we call SetupHistoricSnapshot(). But I > didn't find a path through which SetupHistoricSnapshot() is called > from SnapBuildCommitTxn(). >
It will be called after SnapBuildCommitTxn() via ReorderBufferCommit()->ReorderBufferReplay()->ReorderBufferProcessTXN(). But, I think what I don't see is how the snapshot we build in SnapBuildCommitTxn() will be assigned as a historic snapshot. We assign base_snapshot as a historic snapshot and the new snapshot we build in SnapBuildCommitTxn() is assigned as base_snapshot only if the same is not set previously. I might be missing something but if that is true then I don't think the patch is correct, OTOH I would expect some existing tests to fail if this change is incorrect. -- With Regards, Amit Kapila.
