On Mon, Jan 25, 2021 at 4:37 PM tsunakawa.takay@fujitsu.com
<tsunakawa.takay@fujitsu.com> wrote:
>
>
> > > (4)
> > You could, but I'm not sure it would make the code easier to read,
> > especially for those who don't know !isParallelWorker() means it's a
> > parallel leader.
> ...
> > I really can't see a problem. PrepareParallelMode() is only needed
> > prior to execution of a parallel plan, so it's not needed for "other
> > call sites" using EnterParallelMode().
> My frank first impressions were (and are):
>
> * Why do we have to call a separate function for preparation despite the actual entering follows immediately? We can
donecessary preparation in the entering function.
>
> * Those who read the parallel index build and parallel VACUUM code for the first time might be startled at the
missingPrepareParallelMode() call: "Oh, EnterParallelMode() is called without preparation unlike the other site I saw
theother day. Isn't this a but?"
>
>
> > Perhaps the name can be changed to disassociate it from generic
> > EnterParallelMode() usage. So far, I've only thought of long names
> > like: PrepareParallelModePlanExec().
> > Ideas?
>
> What PrepareParallelMode() handles is the XID and command ID, which are managed by access/transam/ module and are not
executor-specific. It's natural (or at least not unnatural) that EnterParallelMode() prepares them, because
EnterParallelMode()is part of access/transam/.
>
>
EnterParallelMode() is part of a generic interface for execution of a
parallel operation, and EnterParallelMode() is called in several
different places to enter parallel mode prior to execution of
different parallel operations. At the moment it is assumed that
EnterParallelMode() just essentially sets a flag to prohibit certain
unsafe operations when doing the parallel operation. If I move
PrepareParallelMode() into EnterParallelMode() then I need to pass in
contextual information to distinguish who the caller is, and possibly
extra information needed by that caller - and change the function call
for each caller, and probably update the comments for each, and in
other places, etc. etc.
I think that it just complicates things doing this. The other callers
of EnterParallelMode() are obviously currently doing their own "pre"
parallel-mode code themselves, specific to whatever parallel operation
they are doing - but nobody has thought it necessary to have to hook
this code into EnterParallelMode().
I think the "PrepareParallelMode()" name can just be changed to
something specific to plan execution, so nobody gets confused with a
name like "PrepareParallelMode()", which as you point out sounds
generic to all callers of EnterParallelMode().
Regards,
Greg Nancarrow
Fujitsu Australia
