RE: Parallel INSERT (INTO ... SELECT ...)

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