On Wed, Jun 29, 2016 at 10:46 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> Your patch looks good to me and is ready for a committer's look.
>
> Notes for committer -
> a. Verify if description of newly added Guc max_parallel_workers looks
> okay to you, me and Julien are not in 100% agreement on that.
> b. Comments might need some improvement.
This patch needs to be rebased. I hope somebody can volunteer to do
that, because I'd like to commit it once we've hashed out the details.
Would it bother anybody very much if we bumped up these values, say by
increasing max_worker_processes from 8 to 16 and max_parallel_workers
from 4 (as it is in the current patch version) to 8? I feel like 4 is
a bit more conservative than I'd like to be by default, and I'd like
to make sure that we leave room for other sorts of background workers
between the two limits.
I'd suggest renaming the "parallel" flag to BackgroundWorkerSlot to
"is_parallel_worker". Or, actually, what I think would be better is
to give it a name like worker_class, and then we can have
BGWORKER_CLASS_PARALLEL and perhaps eventually
BGWORKER_CLASS_REPLICATION, etc.
+ * terminated ones. These counters can of course overlaps, but it's not
+ * important here since the substraction will still give the right number.
overlaps -> overflow. substraction -> subtraction.
+ /*
+ * We need a write barrier to make sure the update of
+ * parallel_terminate_count is done before the store to in_use
+ */
Does the order actually matter here?
+ {"max_parallel_workers", PGC_USERSET, RESOURCES_ASYNCHRONOUS,
+ gettext_noop("Sets the maximum number of
parallel processes for the cluster."),
I suggest: sets the maximum number of parallel workers that can be
active at one time.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company