RE: Parallel INSERT (INTO ... SELECT ...) - Mailing list pgsql-hackers

> > 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.

Yes, these changes look good to me.

Best regards,
houzj