Re: Choosing parallel_degree

On Sun, Apr 3, 2016 at 4:37 PM, Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com>
wrote:
>
> On 22/03/2016 07:58, Julien Rouhaud wrote:
> > On 21/03/2016 20:38, Julien Rouhaud wrote:
> >> On 21/03/2016 05:18, James Sewell wrote:
> >>> OK cool, thanks.
> >>>
> >>> Can we remove the minimum size limit when the per table degree setting
> >>> is applied?
> >>>
> >>> This would help for tables with 2 - 1000 pages combined with a high
CPU
> >>> cost aggregate.
> >>>
> >>
> >> Attached v4 implements that. It also makes sure that the chosen
> >> parallel_degree won't be more than the relation's estimated number of
pages.
> >>
> >
> > And I just realize that it'd prevent from forcing parallelism on
> > partitionned table, v5 attached removes the check on the estimated
> > number of pages.
> >
> >

Few comments:
1.
+ limited according to the <xref linkend="gux-max-parallel-degree">

A. typo.
/gux-max-parallel-degree/guc-max-parallel-degree
/worker/workers
B. + <para>
+ Number of workers wanted for this table. The number of worker will be
+ limited according to
the <xref linkend="gux-max-parallel-degree">
+ parameter.
+ </para>

How about writing the above as:
Sets the degree of parallelism for an individual relation. The requested
number of workers will be limited by <xref
linkend="guc-max-parallel-degree">

2.
+ {
+ {
+ "parallel_degree",
+ "Number of parallel processes
per executor node wanted for this relation.",
+ RELOPT_KIND_HEAP,
+
AccessExclusiveLock
+ },
+ -1, 1, INT_MAX
+ },

I think here min and max values should be same as for max_parallel_degree
(I have verified that for some of the other reloption parameters, min and
max are same as their guc values); Is there a reason to keep them different?

3.
@@ -1291,7 +1300,9 @@ default_reloptions(Datum reloptions, bool validate,
relopt_kind kind)

Comment on top of this function says:
/*
* Option parser for anything that uses StdRdOptions (i.e. fillfactor and
* autovacuum)
*/

I think it is better to include parallel_degree in above comment along with
fillfactor and autovacuum.

4.
/*
+ * RelationGetMaxParallelDegree
+ * Returns the relation's parallel_degree. Note multiple eval of
argument!
+ */
+#define RelationGetParallelDegree(relation, defaultmpd) \
+ ((relation)->rd_options ? \
+
((StdRdOptions *) (relation)->rd_options)->parallel_degree : (defaultmpd))
+

There are minor in-consistencies in the above macro definition.

a. RelationGetMaxParallelDegree - This should be RelationGetParallelDegree.
b. defaultmpd - it is better to name it as defaultpd

>
>
> The feature freeze is now very close. If this GUC is still wanted,
> should I add this patch to the next commitfest?
>

I am hoping that this will be committed to 9.6, but I think it is good to
register it in next CF.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com