On Wed, Mar 13, 2024 at 11:39 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > Andres already commented on the snapshot stuff on an earlier patch
> > version, and that's much nicer with this version. However, I don't
> > understand why a parallel bitmap heap scan needs to do anything at all
> > with the snapshot, even before these patches. The parallel worker
> > infrastructure already passes the active snapshot from the leader to the
> > parallel worker. Why does bitmap heap scan code need to do that too?
>
> Yeah thinking on this now it seems you are right that the parallel
> infrastructure is already passing the active snapshot so why do we
> need it again. Then I checked other low scan nodes like indexscan and
> seqscan and it seems we are doing the same things there as well.
> Check for SerializeSnapshot() in table_parallelscan_initialize() and
> index_parallelscan_initialize() which are being called from
> ExecSeqScanInitializeDSM() and ExecIndexScanInitializeDSM()
> respectively.
I remember thinking about this when I was writing very early parallel
query code. It seemed to me that there must be some reason why the
EState has a snapshot, as opposed to just using the active snapshot,
and so I took care to propagate that snapshot, which is used for the
leader's scans, to the worker scans also. Now, if the EState doesn't
need to contain a snapshot, then all of that mechanism is unnecessary,
but I don't see how it can be right for the leader to do
table_beginscan() using estate->es_snapshot and the worker to use the
active snapshot.
--
Robert Haas
EDB: http://www.enterprisedb.com