On Mon, Mar 22, 2021 at 2:30 PM houzj.f...@fujitsu.com
<houzj.f...@fujitsu.com> wrote:
>
> I noticed that some comments may need updated since we introduced parallel 
> insert in this patch.
>
> 1) src/backend/executor/execMain.c
>          * Don't allow writes in parallel mode.  Supporting UPDATE and DELETE
>          * would require (a) storing the combocid hash in shared memory, 
> rather
>          * than synchronizing it just once at the start of parallelism, and 
> (b) an
>          * alternative to heap_update()'s reliance on xmax for mutual 
> exclusion.
>          * INSERT may have no such troubles, but we forbid it to simplify the
>          * checks.
>
> As we will allow INSERT in parallel mode, we'd better change the comment here.
>
Thanks, it does need to be updated for parallel INSERT.
I was thinking of the following change:

-     * Don't allow writes in parallel mode.  Supporting UPDATE and DELETE
-     * would require (a) storing the combocid hash in shared memory, rather
-     * than synchronizing it just once at the start of parallelism, and (b) an
-     * alternative to heap_update()'s reliance on xmax for mutual exclusion.
-     * INSERT may have no such troubles, but we forbid it to simplify the
-     * checks.
+     * Except for INSERT, don't allow writes in parallel mode.  Supporting
+     * UPDATE and DELETE would require (a) storing the combocid hash in shared
+     * memory, rather than synchronizing it just once at the start of
+     * parallelism, and (b) an alternative to heap_update()'s reliance on xmax
+     * for mutual exclusion.



> 2) src/backend/storage/lmgr/README
> dangers are modest.  The leader and worker share the same transaction,
> snapshot, and combo CID hash, and neither can perform any DDL or, indeed,
> write any data at all.  Thus, for either to read a table locked exclusively by
>
> The same as 1), parallel insert is the exception.
>

I agree, it needs to be updated too, to account for parallel INSERT
now being supported.

-write any data at all.  ...
+write any data at all (with the exception of parallel insert).  ...


> 3) src/backend/storage/lmgr/README
> mutual exclusion method for such cases.  Currently, the parallel mode is
> strictly read-only, but now we have the infrastructure to allow parallel
> inserts and parallel copy.
>
> May be we can say:
> +mutual exclusion method for such cases.  Currently, we only allowed parallel
> +inserts, but we already have the infrastructure to allow parallel copy.
>

Yes, agree, something like:

-mutual exclusion method for such cases.  Currently, the parallel mode is
-strictly read-only, but now we have the infrastructure to allow parallel
-inserts and parallel copy.
+mutual exclusion method for such cases.  Currently, only parallel insert is
+allowed, but we have the infrastructure to allow parallel copy.


Let me know if these changes seem OK to you.

Regards,
Greg Nancarrow
Fujitsu Australia


Reply via email to