From: | Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | James Sewell <james(dot)sewell(at)lisasoft(dot)com>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Paul Ramsey <pramsey(at)cleverelephant(dot)ca>, Andreas Ulbrich <andreas(dot)ulbrich(at)matheversum(dot)de> |
Subject: | Re: Choosing parallel_degree |
Date: | 2016-04-05 18:25:45 |
Message-ID: | 57040329.6020902@dalibo.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2016-04-05 18:33:50 | Re: Combining Aggregates |
Previous Message | Magnus Hagander | 2016-04-05 18:15:16 | Re: Updated backup APIs for non-exclusive backups |