Re: Choosing parallel_degree - Mailing list pgsql-hackers

On 05/04/2016 06:19, Amit Kapila wrote:
>
> Few more comments:
> 
> 1.
> @@ -909,6 +909,17 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } |
> UNLOGGED ] TABLE [ IF NOT EXI
>     
> </varlistentry>
>  
>     <varlistentry>
> +    <term><literal>parallel_degree</> (<type>integer</>)</term>
> +    <listitem>
> +     
> <para>
> +     Sets the degree of parallelism for an individual relation.  The
> requested
> +     number of workers will be 
> limited by <xref
> +     linkend="guc-max-parallel-degree">.
> +     </para>
> +    </listitem>
> +   </varlistentry>
> 
> All other parameters in this category are supportted by Alter table
> command as well, so I think this parameter should also be supported by
> Alter Table command (for both SET and RESET variants).
>

I don't quite understand.  With the patch you can use parallel_degree in
either CREATE or ALTER table (SET and RESET) statements. Considering
documentation, the list of storage parameters only appears in
create_table.sgml, alter_table.sgml pointing to it.

In alter_table.sgml, I didn't comment the lock level needed to modify
parallel_degree since it requires an access exclusive lock for now.
While thinking about it, I think it's safe to use a share update
exclusive lock but I may be wrong.  What do you think?

> 2.
> +"Number of parallel processes per executor node wanted for this relation.",
> 
> How about
> Number of parallel processes that can be used for this relation per
> executor node.
> 

I just rephrased what was used for the max_parallel_degree GUC, which is:

"Sets the maximum number of parallel processes per executor node."

I find your version better once again, but should we keep some
consistency between them or it's not important?

> 3.
> -if (rel->pages < parallel_threshold && rel->reloptkind == RELOPT_BASEREL)
> +if (rel->pages < 
> parallel_threshold && rel->rel_parallel_degree == -1 &&
> +rel->reloptkind == RELOPT_BASEREL)
> 
> A. Second line should be indented with the begin of first line after
> bracket '(' which means with rel->pages.  Refer multiline condition in
> near by code.  Or you can run pgindent.

I ran pgindent, fixed.

> B. The comment above this condition needs slight adjustment as per new
> condition.
> 

Also fixed.

> 4.
> +intparallel_degree; /* max number of parallel worker */
>  } StdRdOptions;
> 
> Typo in comments
> /worker/workers
>

fixed.

I'll send an updated patch when I'll know what to do about the first two
points.


-- 
Julien Rouhaud
http://dalibo.com - http://dalibo.org