On Fri, Nov 25, 2022 at 5:30 PM Ashutosh Bapat
<ashutosh.bapat.oss@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.
