Re: Rename max_parallel_degree?

Julien Rouhaud <julien.rouhaud@dalibo.com>

From: Julien Rouhaud <julien.rouhaud@dalibo.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Josh Berkus <josh@agliodbs.com>, "David G. Johnston" <david.g.johnston@gmail.com>, Peter Geoghegan <pg@heroku.com>, Noah Misch <noah@leadboat.com>, David Rowley <david.rowley@2ndquadrant.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, Magnus Hagander <magnus@hagander.net>, Bruce Momjian <bruce@momjian.us>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2016-06-29T06:24:23Z
Lists: pgsql-hackers

Attachments

On 29/06/2016 06:29, Amit Kapila wrote:
> On Wed, Jun 29, 2016 at 2:57 AM, Julien Rouhaud
> <julien.rouhaud@dalibo.com> wrote:
>>
>> Thanks a lot for the help!
>>
>> PFA v6 which should fix all the issues mentioned.
> 
> Couple of minor suggestions.
> 
> -         <xref linkend="guc-max-worker-processes">.  Note that the requested
> +         <xref linkend="guc-max-worker-processes">, limited by
> +         <xref linked="guc-max-parallel-workers">.  Note that the requested
> 
> Typo.
> /linked/linkend
> 

Oops, fixed.

> You can always find such mistakes by doing make check in doc/src/sgml/
> 

I wasn't aware of that, it's really a nice thing to know, thanks!

> + /*
> + * We need a memory barrier here to make sure the above test doesn't get
> + * reordered
> + */
> + pg_read_barrier();
> 
> /memory barrier/read barrier
> 

fixed

> + if (max_parallel_workers == 0)
> + {
> + ereport(elevel,
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("background worker \"%s\": cannot request parallel worker if
> no parallel worker allowed",
> 
> " ..no parallel worker is allowed".  'is' seems to be missing.
> 

fixed

> 
>>  Also, after second
>> thought I didn't add the extra hint about max_worker_processes in the
>> max_parallel_worker paragraph, since this line was a duplicate of the
>> precedent paragraph, it seemed better to leave the text as is.
>>
> 
> not a big problem, we can leave it for committer to decide on same.
> However just by reading the description of max_parallel_worker, user
> can set its value more than max_wroker_processes which we don't want.
> 

Right.  On the other hand I'm not sure that's really an issue, because
such a case is handled in the code, and setting max_parallel_workers way
above max_worker_processes could be a way to configure it as unlimited.
Or should we allow setting it to -1 for instance to disable the limit?

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