assessing parallel-safety
Amit's parallel sequential scan assumes that we can enter parallel
mode when the parallel sequential scan is initialized and exit
parallel mode when the scan ends and all the code that runs in between
will be happy with that. Unfortunately, that's not necessarily the
case. There are two ways it can fail:
1. Some other part of the query can contain functions that are not
safe to run in parallel-mode; e.g. a PL/pgsql function that writes
data or uses subtransactions.
2. The user can run partially execute the query and then, while
execution is suspended, go do something not parallel-safe with the
results before resuming query execution.
To properly assess whether a query is parallel-safe, we need to
inspect the entire query for non-parallel-safe functions. We also
need the code that's going to execute the plan to tell us whether or
not they might want to do not-parallel-safe things between the time we
start running the query and the time we finish running it. So I tried
writing some code to address this; a first cut is attached. Here's
what it does:
1. As we parse each query, it sets a flag in the parse-state if we see
a non-immutable function. For the time being, I'm assuming immutable
== parallel-safe, although that's probably not correct in detail. It
also sets the flag if it sees a data-modifying operation, meaning an
insert, update, delete, or locking clause. The point of this is to
avoid making an extra pass over the query just to assess
parallel-safety; we want to accumulate that information as we go
along.
2. When parsing is complete, the parse-state flag is copied into the
Query, similar to what we already do for flags like hasModifyingCTE.
3. When the query is planned, planner() sets a flag in the
PlannerGlobal called parallelModeOK if the Query is not marked as
parallel-mode unsafe. There's also a new cursor option,
CURSOR_OPT_NO_PARALLEL, with forces parallelModeOK to false regardless
of what the Query says. It initializes another flag
parallelModeNeeded to false as well. The idea here is that before
generating a parallel path, the planner should examine parallelModeOK
and skip it if that's false. If we end up creating a plan from a
parallel path, then the plan-generation function should set
parallelModeNeeded.
4. At the conclusion of planning, the parallelModeNeeded flag is
copied from the PlannerGlobal to the PlannedStmt.
5. ExecutorStart() calls EnterParallelMode() if parallelModeNeeded is
set and we're not already in parallel mode. ExecutorEnd() calls
ExitParallelMode() if EnterParallelMode() was called in
ExecutorStart().
There are a few problems with this design that I don't immediately
know how to solve:
1. I'm concerned that the query-rewrite step could substitute a query
that is not parallel-safe for one that is. The upper Query might
still be flagged as safe, and that's all that planner() looks at.
2. Interleaving the execution of two parallel queries by firing up two
copies of the executor simultaneously can result in leaving parallel
mode at the wrong time.
3. Any code using SPI has to think hard about whether to pass
OPT_CURSOR_NO_PARALLEL. For example, PL/pgsql doesn't need to pass
this flag when caching a plan for a query that will be run to
completion each time it's executed. But it DOES need to pass the flag
for a FOR loop over an SQL statement, because the code inside the FOR
loop might do parallel-unsafe things while the query is suspended.
Thoughts, either on the general approach or on what to do about the problems?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
assess-parallel-safety-v1.patchapplication/x-patch; name=assess-parallel-safety-v1.patchDownload+162-42
On Sat, Feb 07, 2015 at 08:18:55PM -0500, Robert Haas wrote:
There are a few problems with this design that I don't immediately
know how to solve:1. I'm concerned that the query-rewrite step could substitute a query
that is not parallel-safe for one that is. The upper Query might
still be flagged as safe, and that's all that planner() looks at.
I would look at determining the query's parallel safety early in the planner
instead; simplify_function() might be a cheap place to check. Besides
avoiding rewriter trouble, this allows one to alter parallel safety of a
function without invalidating Query nodes serialized in the system catalogs.
2. Interleaving the execution of two parallel queries by firing up two
copies of the executor simultaneously can result in leaving parallel
mode at the wrong time.
Perhaps the parallel mode state should be a reference count, not a boolean.
Alternately, as a first cut, just don't attempt parallelism when we're already
in parallel mode.
3. Any code using SPI has to think hard about whether to pass
OPT_CURSOR_NO_PARALLEL. For example, PL/pgsql doesn't need to pass
this flag when caching a plan for a query that will be run to
completion each time it's executed. But it DOES need to pass the flag
for a FOR loop over an SQL statement, because the code inside the FOR
loop might do parallel-unsafe things while the query is suspended.
That makes sense; the code entering SPI knows best which restrictions it can
tolerate for the life of a given cursor. (One can imagine finer-grained rules
in the future. If the current function is itself marked parallel-safe, it's
safe in principle for a FOR-loop SQL statement to use parallelism.) I do
recommend inverting the sense of the flag, so unmodified non-core PLs will
continue to behave as they do today.
Thanks,
nm
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Feb 8, 2015 at 11:31 AM, Noah Misch <noah@leadboat.com> wrote:
On Sat, Feb 07, 2015 at 08:18:55PM -0500, Robert Haas wrote:
There are a few problems with this design that I don't immediately
know how to solve:1. I'm concerned that the query-rewrite step could substitute a query
that is not parallel-safe for one that is. The upper Query might
still be flagged as safe, and that's all that planner() looks at.I would look at determining the query's parallel safety early in the planner
instead; simplify_function() might be a cheap place to check. Besides
avoiding rewriter trouble, this allows one to alter parallel safety of a
function without invalidating Query nodes serialized in the system catalogs.
Thanks, I'll investigate that approach.
2. Interleaving the execution of two parallel queries by firing up two
copies of the executor simultaneously can result in leaving parallel
mode at the wrong time.Perhaps the parallel mode state should be a reference count, not a boolean.
Alternately, as a first cut, just don't attempt parallelism when we're already
in parallel mode.
I think changing it to a reference count makes sense. I'll work on that.
3. Any code using SPI has to think hard about whether to pass
OPT_CURSOR_NO_PARALLEL. For example, PL/pgsql doesn't need to pass
this flag when caching a plan for a query that will be run to
completion each time it's executed. But it DOES need to pass the flag
for a FOR loop over an SQL statement, because the code inside the FOR
loop might do parallel-unsafe things while the query is suspended.That makes sense; the code entering SPI knows best which restrictions it can
tolerate for the life of a given cursor. (One can imagine finer-grained rules
in the future. If the current function is itself marked parallel-safe, it's
safe in principle for a FOR-loop SQL statement to use parallelism.) I do
recommend inverting the sense of the flag, so unmodified non-core PLs will
continue to behave as they do today.
Yeah, that's probably a good idea. Sort of annoying, but playing with
the patch in the OP made it pretty clear that we cannot possibly just
assume parallelism is OK by default. In the core code, parallelism is
OK in more places than not, but in the PLs it seems to be the reverse.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Feb 8, 2015 at 12:28 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Sun, Feb 8, 2015 at 11:31 AM, Noah Misch <noah@leadboat.com> wrote:
On Sat, Feb 07, 2015 at 08:18:55PM -0500, Robert Haas wrote:
There are a few problems with this design that I don't immediately
know how to solve:1. I'm concerned that the query-rewrite step could substitute a query
that is not parallel-safe for one that is. The upper Query might
still be flagged as safe, and that's all that planner() looks at.I would look at determining the query's parallel safety early in the planner
instead; simplify_function() might be a cheap place to check. Besides
avoiding rewriter trouble, this allows one to alter parallel safety of a
function without invalidating Query nodes serialized in the system catalogs.Thanks, I'll investigate that approach.
This does not seem to work out nicely. The problem here is that
simplify_function() gets called from eval_const_expressions() which
gets called from a variety of places, but the principal one seems to
be subquery_planner(). So if you have a query with two subqueries,
and the second one contains something parallel-unsafe, you might by
that time have already generated a parallel plan for the first one,
which won't do. Unless we want to rejigger this so that we do a
complete eval_const_expressions() pass over the entire query tree
(including all subqueries) FIRST, and then only after that go back and
plan all of those subqueries, I don't see how to make this work; and
I'm guessing that there are good reasons not to do that.
Ideas?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Feb 11, 2015 at 9:39 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Sun, Feb 8, 2015 at 12:28 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Sun, Feb 8, 2015 at 11:31 AM, Noah Misch <noah@leadboat.com> wrote:
On Sat, Feb 07, 2015 at 08:18:55PM -0500, Robert Haas wrote:
There are a few problems with this design that I don't immediately
know how to solve:1. I'm concerned that the query-rewrite step could substitute a query
that is not parallel-safe for one that is. The upper Query might
still be flagged as safe, and that's all that planner() looks at.I would look at determining the query's parallel safety early in the planner
instead; simplify_function() might be a cheap place to check. Besides
avoiding rewriter trouble, this allows one to alter parallel safety of a
function without invalidating Query nodes serialized in the system catalogs.Thanks, I'll investigate that approach.
This does not seem to work out nicely. The problem here is that
simplify_function() gets called from eval_const_expressions() which
gets called from a variety of places, but the principal one seems to
be subquery_planner(). So if you have a query with two subqueries,
and the second one contains something parallel-unsafe, you might by
that time have already generated a parallel plan for the first one,
which won't do. Unless we want to rejigger this so that we do a
complete eval_const_expressions() pass over the entire query tree
(including all subqueries) FIRST, and then only after that go back and
plan all of those subqueries, I don't see how to make this work; and
I'm guessing that there are good reasons not to do that.
And ... while I was just talking with Jan about this, I realized
something that should have been blindingly obvious to me from the
beginning, which is that anything we do at parse time is doomed to
failure, because the properties of a function that we use to determine
parallel-safety can be modified later:
rhaas=# alter function random() immutable; -- no it isn't
ALTER FUNCTION
I think we may want a dedicated parallel-safe property for functions
rather than piggybacking on provolatile, but that will probably also
be changeable via ALTER FUNCTION, and stored rules won't get
miraculously updated. So this definitely can't be something we figure
out at parse-time ... it's got to be determined later. But at the
moment I see no way to do that without an extra pass over the whole
rewritten query tree. :-(
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Feb 11, 2015 at 03:21:12PM -0500, Robert Haas wrote:
On Wed, Feb 11, 2015 at 9:39 AM, Robert Haas <robertmhaas@gmail.com> wrote:
This does not seem to work out nicely. The problem here is that
simplify_function() gets called from eval_const_expressions() which
gets called from a variety of places, but the principal one seems to
be subquery_planner(). So if you have a query with two subqueries,
and the second one contains something parallel-unsafe, you might by
that time have already generated a parallel plan for the first one,
which won't do.
That is a major mark against putting the check in simplify_function(), agreed.
Unless we want to rejigger this so that we do a
complete eval_const_expressions() pass over the entire query tree
(including all subqueries) FIRST, and then only after that go back and
plan all of those subqueries, I don't see how to make this work; and
I'm guessing that there are good reasons not to do that.
I expect that would work fine, but I do think it premature to venture that far
out of your way to optimize this new tree examination. The cost may just not
matter. Other parts of the planner use code like contain_volatile_functions()
and contain_nonstrict_functions(), which have the same kind of inefficiency
you're looking to avoid here.
I think we may want a dedicated parallel-safe property for functions
rather than piggybacking on provolatile, but that will probably also
be changeable via ALTER FUNCTION, and stored rules won't get
miraculously updated. So this definitely can't be something we figure
out at parse-time ... it's got to be determined later.
Pretty much.
But at the
moment I see no way to do that without an extra pass over the whole
rewritten query tree. :-(
+1 for doing that.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Feb 12, 2015 at 1:51 AM, Robert Haas <robertmhaas@gmail.com> wrote:
I think we may want a dedicated parallel-safe property for functions
rather than piggybacking on provolatile, but that will probably also
be changeable via ALTER FUNCTION, and stored rules won't get
miraculously updated. So this definitely can't be something we figure
out at parse-time ... it's got to be determined later. But at the
moment I see no way to do that without an extra pass over the whole
rewritten query tree. :-(
If we have to go this way, then isn't it better to evaluate the same
when we are trying to create parallel path (something like in the
parallel_seq scan patch - create_parallelscan_paths())?
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Thu, Feb 12, 2015 at 12:16 AM, Noah Misch <noah@leadboat.com> wrote:
That is a major mark against putting the check in simplify_function(), agreed.
I do see one way to rescue that idea, which is this: put two flags,
parallelModeOK and parallelModeRequired, into PlannerGlobal. At the
beginning of planning, set parallelModeOK based on our best knowledge
at that time; as we preprocess expressions, update it to false if we
see something that's not parallel-safe. Emit paths for parallel plans
only if the flag is currently true. At the end of planning, when we
convert paths to plans, set parallelModeRequired if any parallel plan
elements are generated. If we started with parallelModeOK = true or
ended up with parallelModeRequired = false, then life is good. In the
unfortunate event that we end up with parallelModeOK = false and
parallelModeRequired = true, replan, this time with parallelModeOK =
false from the beginning.
If there are no sub-queries involved, this will work out fine -
parallelModeOK will always be definitively set to the right value
before we rely on it for anything. This includes cases where
subqueries are eliminated by pulling them up. If there *are*
subqueries, we'll still be OK if we happen to hit the parallel-unsafe
construct before we hit the part - if any - that can benefit from
parallelism.
So this would mostly be pretty cheap, but if you do hit the case where
a re-plan is required it would be pretty painful.
Unless we want to rejigger this so that we do a
complete eval_const_expressions() pass over the entire query tree
(including all subqueries) FIRST, and then only after that go back and
plan all of those subqueries, I don't see how to make this work; and
I'm guessing that there are good reasons not to do that.I expect that would work fine, but I do think it premature to venture that far
out of your way to optimize this new tree examination. The cost may just not
matter. Other parts of the planner use code like contain_volatile_functions()
and contain_nonstrict_functions(), which have the same kind of inefficiency
you're looking to avoid here.
They do, but those are run on much smaller pieces of the query tree,
never on the whole thing. I really wish Tom Lane would weigh in here,
as an opinion from him could save me from spending time on a doomed
approach. I expect criticism of this type:
/messages/by-id/22266.1269641438@sss.pgh.pa.us
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Feb 12, 2015 at 6:40 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
If we have to go this way, then isn't it better to evaluate the same
when we are trying to create parallel path (something like in the
parallel_seq scan patch - create_parallelscan_paths())?
Probably not, because many queries will scan multiple relations, and
we want to do all of this work just once per query. Also, when
somebody adds another parallel node (e.g. parallel hash join) that
will need this same information.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Feb 11, 2015 at 3:21 PM, Robert Haas <robertmhaas@gmail.com> wrote:
I think we may want a dedicated parallel-safe property for functions
rather than piggybacking on provolatile ...
I went through the current contents of pg_proc and tried to assess how
much parallel-unsafe stuff we've got. I think there are actually
three categories of things: (1) functions that can be called in
parallel mode either in the worker or in the leader ("parallel safe"),
(2) functions that can be called in parallel mode in the worker, but
not in the leader ("parallel restricted"), and (3) functions that
cannot be called in parallel mode at all ("parallel unsafe"). On a
first read-through, the number of things that looked not to be
anything other than parallel-safe looked to be fairly small; many of
these could be made parallel-safe with more work, but it's unlikely to
be worth the effort.
current_query() - Restricted because debug_query_string is not copied.
lo_open(), lo_close(), loread(), lowrite(), and other large object
functions - Restricted because large object state is not shared.
age(xid) - Restricted because it uses a transaction-lifespan cache
which is not shared.
now() - Restricted because transaction start timestamp is not copied.
statement_timestamp() - Restricted because statement start timestamp
is not copied.
pg_conf_load_time() - Restricted because PgReloadTime is not copied.
nextval(), currval() - Restricted because sequence-related state is not shared.
setval() - Unsafe because no data can be written in parallel mode.
random(), setseed() - Restricted because random seed state is not
shared. (We could alternatively treat setseed() as unsafe and random()
to be restricted only in sessions where setseed() has never been
called, and otherwise safe.)
pg_stat_get_* - Restricted because there's no guarantee the value
would be the same in the parallel worker as in the leader.
pg_backend_pid() - Restricted because the worker has a different PID.
set_config() - Unsafe because GUC state must remain synchronized.
pg_my_temp_schema() - Restricted because temporary namespaces aren't
shared with parallel workers.
pg_export_snapshot() - Restricted because the worker will go away quickly.
pg_prepared_statement(), pg_cursor() - Restricted because the prepared
statements and cursors are not synchronized with the worker.
pg_listening_channels() - Restricted because listening channels are
not synchronized with the worker.
pg*advisory*lock*() - Restricted because advisory lock state is not
shared with workers - and even if it were, the semantics would be hard
to reason about.
txid_current() - Unsafe because it might attempt XID assignment.
pg_logical_slot*() - Unsafe because they do all kinds of crazy stuff.
That's not a lot, and very little of it is anything you'd care about
parallelizing anyway. I expect that the incidence of user-written
parallel-unsafe functions will be considerably higher. I'm not sure
if this impacts the decision about how to design the facility for
assessing parallel-safety or not, but I thought it was worth sharing.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Feb 12, 2015 at 10:07 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Feb 12, 2015 at 6:40 AM, Amit Kapila <amit.kapila16@gmail.com>
wrote:
If we have to go this way, then isn't it better to evaluate the same
when we are trying to create parallel path (something like in the
parallel_seq scan patch - create_parallelscan_paths())?Probably not, because many queries will scan multiple relations, and
we want to do all of this work just once per query.
By this, do you mean to say that if there is any parallel-unsafe
expression (function call) in query, then we won't parallelize any
part of query, if so why is that mandatory?
Can't we parallelize scan on a particular relation if all the expressions
in which that relation is involved are parallel-safe
Also, when
somebody adds another parallel node (e.g. parallel hash join) that
will need this same information.
I think we should be able to handle this even if we have per relation
information (something like don't parallelize a node/path if any lower
node is not parallel)
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Thu, Feb 12, 2015 at 3:52 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
Probably not, because many queries will scan multiple relations, and
we want to do all of this work just once per query.By this, do you mean to say that if there is any parallel-unsafe
expression (function call) in query, then we won't parallelize any
part of query, if so why is that mandatory?
Because of stuff like set_config() and txid_current(), which will fail
outright in parallel mode. Also because the user may have defined a
function that updates some other table in the database, which will
also fail outright if called in parallel mode. Instead of failing, we
want those kinds of things to fall back to a non-parallel plan.
Can't we parallelize scan on a particular relation if all the expressions
in which that relation is involved are parallel-safe
No, because the execution of that node can be interleaved in arbitrary
fashion with all the other nodes in the plan tree. Once we've got
parallel mode active, all the related prohibitions apply to
*everything* we do thereafter, not just that one node.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Feb 12, 2015 at 07:40:12AM -0500, Robert Haas wrote:
On Thu, Feb 12, 2015 at 12:16 AM, Noah Misch <noah@leadboat.com> wrote:
That is a major mark against putting the check in simplify_function(), agreed.
I do see one way to rescue that idea, which is this: put two flags,
parallelModeOK and parallelModeRequired, into PlannerGlobal. At the
beginning of planning, set parallelModeOK based on our best knowledge
at that time; as we preprocess expressions, update it to false if we
see something that's not parallel-safe. Emit paths for parallel plans
only if the flag is currently true. At the end of planning, when we
convert paths to plans, set parallelModeRequired if any parallel plan
elements are generated. If we started with parallelModeOK = true or
ended up with parallelModeRequired = false, then life is good. In the
unfortunate event that we end up with parallelModeOK = false and
parallelModeRequired = true, replan, this time with parallelModeOK =
false from the beginning.
So this would mostly be pretty cheap, but if you do hit the case where
a re-plan is required it would be pretty painful.
Unless we want to rejigger this so that we do a
complete eval_const_expressions() pass over the entire query tree
(including all subqueries) FIRST, and then only after that go back and
plan all of those subqueries, I don't see how to make this work; and
I'm guessing that there are good reasons not to do that.I expect that would work fine, but I do think it premature to venture that far
out of your way to optimize this new tree examination.
Given your wish to optimize, I recommend first investigating the earlier
thought to issue eval_const_expressions() once per planner() instead of once
per subquery_planner(). Compared to the parallelModeRequired/parallelModeOK
idea, it would leave us with a more maintainable src/backend/optimizer. I
won't object to either design, though.
Your survey of parallel safety among built-in functions was on-target.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Feb 13, 2015 at 2:40 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Feb 12, 2015 at 3:52 PM, Amit Kapila <amit.kapila16@gmail.com>
wrote:
Probably not, because many queries will scan multiple relations, and
we want to do all of this work just once per query.By this, do you mean to say that if there is any parallel-unsafe
expression (function call) in query, then we won't parallelize any
part of query, if so why is that mandatory?Because of stuff like set_config() and txid_current(), which will fail
outright in parallel mode. Also because the user may have defined a
function that updates some other table in the database, which will
also fail outright if called in parallel mode. Instead of failing, we
want those kinds of things to fall back to a non-parallel plan.Can't we parallelize scan on a particular relation if all the
expressions
in which that relation is involved are parallel-safe
No, because the execution of that node can be interleaved in arbitrary
fashion with all the other nodes in the plan tree. Once we've got
parallel mode active, all the related prohibitions apply to
*everything* we do thereafter, not just that one node.
Okay, got the point.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
On Fri, Feb 13, 2015 at 12:10 AM, Noah Misch <noah@leadboat.com> wrote:
Given your wish to optimize, I recommend first investigating the earlier
thought to issue eval_const_expressions() once per planner() instead of once
per subquery_planner(). Compared to the parallelModeRequired/parallelModeOK
idea, it would leave us with a more maintainable src/backend/optimizer. I
won't object to either design, though.
In off-list discussions with Tom Lane, he pressed hard on the question
of whether we can zero out the number of functions that are
parallel-unsafe (i.e. can't be run while parallel even in the master)
vs. parallel-restricted (must be run in the master rather than
elsewhere). The latter category can be handled by strictly local
decision-making, without needing to walk the entire plan tree; e.g.
parallel seq scan can look like this:
Parallel Seq Scan on foo
Filter: a = pg_backend_pid()
Parallel Filter: b = 1
And, indeed, I was pleasantly surprised when surveying the catalogs by
how few functions were truly unsafe, vs. merely needing to be
restricted to the master. But I can't convince myself that there's
any way sane of allowing database writes even in the master; creating
new combo CIDs there seems disastrous, and users will be sad if a
parallel plan is chosen for some_plpgsql_function_that_does_updates()
and this then errors out because of parallel mode.
Tom also argued that (1) trying to assess parallel-safety before
preprocess_expressions() was doomed to fail, because
preprocess_expressions() can additional function calls via, at least,
inlining and default argument insertion and (2)
preprocess_expressions() can't be moved earlier than without changing
the semantics. I'm not sure if he's right, but those are sobering
conclusions. Andres pointed out to me via IM that inlining is
dismissable here; if inlining introduces a parallel-unsafe construct,
the inlined function was mislabeled to begin with, and the user has
earned the error message they get. Default argument insertion might
not be dismissable although the practical risks seem low.
It's pretty awful that this is such a thorny issue, and the patch may
be rather cringe-worthy, given the (to me) essentially reasonable
nature of wanting to know whether a query has any references to a
function with a certain property in it.
Your survey of parallel safety among built-in functions was on-target.
Thanks for having a look. A healthy amount of the way the parallel
mode stuff works came from your brain, so your opinion, while always
valuable, is especially appreciated here. I have a lot of confidence
in your judgement about this stuff.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Feb 13, 2015 at 05:13:06PM -0500, Robert Haas wrote:
On Fri, Feb 13, 2015 at 12:10 AM, Noah Misch <noah@leadboat.com> wrote:
Given your wish to optimize, I recommend first investigating the earlier
thought to issue eval_const_expressions() once per planner() instead of once
per subquery_planner(). Compared to the parallelModeRequired/parallelModeOK
idea, it would leave us with a more maintainable src/backend/optimizer. I
won't object to either design, though.In off-list discussions with Tom Lane, he pressed hard on the question
of whether we can zero out the number of functions that are
parallel-unsafe (i.e. can't be run while parallel even in the master)
vs. parallel-restricted (must be run in the master rather than
elsewhere). The latter category can be handled by strictly local
decision-making, without needing to walk the entire plan tree; e.g.
parallel seq scan can look like this:Parallel Seq Scan on foo
Filter: a = pg_backend_pid()
Parallel Filter: b = 1And, indeed, I was pleasantly surprised when surveying the catalogs by
how few functions were truly unsafe, vs. merely needing to be
restricted to the master. But I can't convince myself that there's
any way sane of allowing database writes even in the master; creating
new combo CIDs there seems disastrous, and users will be sad if a
parallel plan is chosen for some_plpgsql_function_that_does_updates()
and this then errors out because of parallel mode.
Yep. The scarcity of parallel-unsafe, built-in functions reflects the
dominant subject matter of built-in functions. User-defined functions are
more diverse. It would take quite a big hammer to beat the parallel-unsafe
category into irrelevancy.
Tom also argued that (1) trying to assess parallel-safety before
preprocess_expressions() was doomed to fail, because
preprocess_expressions() can additional function calls via, at least,
inlining and default argument insertion and (2)
preprocess_expressions() can't be moved earlier than without changing
the semantics. I'm not sure if he's right, but those are sobering
conclusions. Andres pointed out to me via IM that inlining is
dismissable here; if inlining introduces a parallel-unsafe construct,
the inlined function was mislabeled to begin with, and the user has
earned the error message they get. Default argument insertion might
not be dismissable although the practical risks seem low.
All implementation difficulties being equal, I would opt to check for parallel
safety after inserting default arguments and before inlining. Checking before
inlining reveals the mislabeling every time instead of revealing it only when
inline_function() gives up. Timing of the parallel safety check relative to
default argument insertion matters less. Remember, the risk is merely that a
user will find cause to remove a parallel-safe marking where he/she expected
the system to deduce parallel unsafety. If implementation difficulties lead
to some other timings, that won't bother me.
Thanks,
nm
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Feb 14, 2015 at 12:09 AM, Noah Misch <noah@leadboat.com> wrote:
Tom also argued that (1) trying to assess parallel-safety before
preprocess_expressions() was doomed to fail, because
preprocess_expressions() can additional function calls via, at least,
inlining and default argument insertion and (2)
preprocess_expressions() can't be moved earlier than without changing
the semantics. I'm not sure if he's right, but those are sobering
conclusions. Andres pointed out to me via IM that inlining is
dismissable here; if inlining introduces a parallel-unsafe construct,
the inlined function was mislabeled to begin with, and the user has
earned the error message they get. Default argument insertion might
not be dismissable although the practical risks seem low.All implementation difficulties being equal, I would opt to check for parallel
safety after inserting default arguments and before inlining. Checking before
inlining reveals the mislabeling every time instead of revealing it only when
inline_function() gives up. Timing of the parallel safety check relative to
default argument insertion matters less. Remember, the risk is merely that a
user will find cause to remove a parallel-safe marking where he/she expected
the system to deduce parallel unsafety. If implementation difficulties lead
to some other timings, that won't bother me.
Fair. For a first cut at this, I've implemented the approach you
suggested before: just do an extra pass over the whole query tree,
looking for parallel-unsafe functions. The attached patch implements
that, via a new proparallel system catalog column. A few notes:
- For testing purposes, I set this up so that the executor activates
parallel mode when running any query that seems to be parallel-safe.
It seems nearly certain we'd only want to do this when a parallel plan
was actually selected, but this behavior is awfully useful for
testing, and revealed a number of bugs in the parallel-safety
markings. parallel-mode-v6.patch + this patch passes make
check-world.
- This patch doesn't include syntax to let the user set the
proparallel flag on new objects. Presumably we could add PARALLEL {
SAFE | RESTRICTED | UNSAFE } to CREATE/ALTER FUNCTION, but I haven't
done that yet. This is enough to assess whether the approach is
fundamentally workable, and to check what the performance
ramifications may be.
- The patch includes a script, fixpgproc.pl, which I used to insert
and update the parallel markings on the system catalogs. That is, of
course, not intended for commit, but it's mighty useful for
experimentation.
- parallel-mode-v6.patch adds a new restriction for a function to be
considered parallel-safe: it must not acquire any heavyweight locks
that it does not also release. This allows scans of system catalogs
and the scan relation, but generally rules out scanning other things.
To compensate, this patch marks more stuff parallel-unsafe than the
list I sent previously. I think that's OK for now, but this is a
pretty onerous restriction which I think we will want to try to relax
at least to the extent of allowing a function to be parallel-safe if
it acquires AccessShareLock on additional relations. See the README
in the other patch for why I imposed this restriction.
Thoughts?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
assess-parallel-safety-v2.patchapplication/x-patch; name=assess-parallel-safety-v2.patchDownload+2968-2676
It's important for parallelism to work under the extended query protocol, and
that's nontrivial. exec_parse_message() sets cursorOptions.
exec_bind_message() runs the planner. We don't know if a parallel plan is
okay before seeing max_rows in exec_execute_message().
On Sun, Feb 15, 2015 at 01:29:06AM -0500, Robert Haas wrote:
- For testing purposes, I set this up so that the executor activates
parallel mode when running any query that seems to be parallel-safe.
It seems nearly certain we'd only want to do this when a parallel plan
was actually selected, but this behavior is awfully useful for
testing, and revealed a number of bugs in the parallel-safety
markings.
Why wouldn't that be a good permanent behavior? The testing benefit extends
to users. If I wrongly mark a function something other than
PROPARALLEL_UNSAFE, I'd rather find out ASAP.
- The patch includes a script, fixpgproc.pl, which I used to insert
and update the parallel markings on the system catalogs. That is, of
course, not intended for commit, but it's mighty useful for
experimentation.
The CREATE OR REPLACE FUNCTION calls in src/backend/catalog/system_views.sql
reset proparallel=u for some built-in functions, such as make_interval.
Some categories of PROPARALLEL_SAFE functions deserve more attention:
- database_to_xml* need to match proparallel of schema_to_xml*, which are
PROPARALLEL_UNSAFE due to the heavyweight lock rule. cursor_to_xml*
probably need to do likewise.
- Several functions, such as array_in and anytextcat, can call arbitrary I/O
functions. This implies a policy that I/O functions must be
PROPARALLEL_SAFE. I agree with that decision, but it should be documented.
Several json-related functions can invoke X->json cast functions; the
marking implies that all such cast functions must be PROPARALLEL_SAFE.
That's probably okay, too.
- All selectivity estimator functions are PROPARALLEL_SAFE. That implies, for
example, that any operator using eqjoinsel ought to be PROPARALLEL_SAFE or
define its own restriction selectivity estimator function. On the other
hand, the planner need not check the parallel safety of selectivity
estimator functions. If we're already in parallel mode at the moment we
enter the planner, we must be planning a query called within a
PROPARALLEL_SAFE function. If the query uses an unsafe operator, the
containing function was mismarked.
- inet_{client,server}_{addr,port} are PROPARALLEL_SAFE; will the information
they need indeed be available in workers?
- Assuming you don't want to propagate XactLastRecEnd from the slave back to
the master, restrict XLogInsert() during parallel mode. Restrict functions
that call it, including pg_create_restore_point, pg_switch_xlog and
pg_stop_backup.
- pg_extension_config_dump modifies the database, so it shall be
PROPARALLEL_UNSAFE. Assuming pg_stat_clear_snapshot won't reach back to
the master to clear the snapshot, it shall be PROPARALLEL_RESTRICTED.
If a mismarked function elicits "ERROR: cannot retain locks acquired while in
parallel mode", innocent queries later in the transaction get the same error:
begin;
select 1; -- ok
savepoint q; select database_to_xml(true, true, 'x'); rollback to q; -- ERROR
select 1; -- ERROR
rollback;
--- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -1610,6 +1610,7 @@ makeRangeConstructors(const char *name, Oid namespace, false, /* leakproof */ false, /* isStrict */ PROVOLATILE_IMMUTABLE, /* volatility */ + PROPARALLEL_UNSAFE, /* parallel safety */
Range constructors are PROPARALLEL_SAFE.
--- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -496,7 +496,9 @@ init_execution_state(List *queryTree_list, if (queryTree->commandType == CMD_UTILITY) stmt = queryTree->utilityStmt; else - stmt = (Node *) pg_plan_query(queryTree, 0, NULL); + stmt = (Node *) pg_plan_query(queryTree, + fcache->readonly_func ? CURSOR_OPT_PARALLEL_OK : 0, + NULL);
(This code is deciding whether to allow parallelism in one statement of an
SQL-language function.) Why does it care if the function is read-only?
I see that PL/pgSQL never allows parallelism in queries it launches.
--- a/src/include/utils/lsyscache.h +++ b/src/include/utils/lsyscache.h @@ -79,6 +79,7 @@ extern bool op_mergejoinable(Oid opno, Oid inputtype); extern bool op_hashjoinable(Oid opno, Oid inputtype); extern bool op_strict(Oid opno); extern char op_volatile(Oid opno); +extern char op_parallel(Oid opno);
No such function defined.
In passing, I noticed a few cosmetic problems in the prerequisite
parallel-mode-v6.patch. Typos: initating Musn't paralell paralellism
paralllelism processe. Capitalization of "transactionIdPrecedes". When
applying the patch, this:
$ git apply ../rev/parallel-mode-v6.patch
../rev/parallel-mode-v6.patch:2897: indent with spaces.
(nLocksAcquiredInParallelMode - n));
warning: 1 line adds whitespace errors.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Feb 15, 2015 at 6:24 PM, Noah Misch <noah@leadboat.com> wrote:
It's important for parallelism to work under the extended query protocol, and
that's nontrivial. exec_parse_message() sets cursorOptions.
exec_bind_message() runs the planner. We don't know if a parallel plan is
okay before seeing max_rows in exec_execute_message().
Yes, that's a problem. One could require users of the extended query
protocol to indicate their willingness to accept a parallel query plan
when sending the bind message by setting the appropriate cursor
option; and one could, when that option is specified, refuse execute
messages with max_rows != 0. However, that has the disadvantage of
forcing all clients to be updated for the new world order. Or, one
could assume a parallel plan is OK and re-plan if we choose one and
then a non-zero max_rows shows up. In the worst case, though, that
could require every query to be planned twice.
On Sun, Feb 15, 2015 at 01:29:06AM -0500, Robert Haas wrote:
- For testing purposes, I set this up so that the executor activates
parallel mode when running any query that seems to be parallel-safe.
It seems nearly certain we'd only want to do this when a parallel plan
was actually selected, but this behavior is awfully useful for
testing, and revealed a number of bugs in the parallel-safety
markings.Why wouldn't that be a good permanent behavior? The testing benefit extends
to users. If I wrongly mark a function something other than
PROPARALLEL_UNSAFE, I'd rather find out ASAP.
It might be a good permanent behavior, or it might just annoy users
unnecessarily. I am unable to guess what is best.
- The patch includes a script, fixpgproc.pl, which I used to insert
and update the parallel markings on the system catalogs. That is, of
course, not intended for commit, but it's mighty useful for
experimentation.The CREATE OR REPLACE FUNCTION calls in src/backend/catalog/system_views.sql
reset proparallel=u for some built-in functions, such as make_interval.
I can fix that if I add a PARALLEL { SAFE | RESTRICTED | UNSAFE }
clause to CREATE FUNCTION. Or are you saying that CORF shouldn't
change the existing value of the flag? I'm not sure exactly what our
policy is here, but I would expect that the CORF is doing the right
thing and we need to add the necessary syntax to CREATE FUNCTION and
then add that clause to those calls.
Some categories of PROPARALLEL_SAFE functions deserve more attention:
- database_to_xml* need to match proparallel of schema_to_xml*, which are
PROPARALLEL_UNSAFE due to the heavyweight lock rule. cursor_to_xml*
probably need to do likewise.
Thanks, fixed.
- Several functions, such as array_in and anytextcat, can call arbitrary I/O
functions. This implies a policy that I/O functions must be
PROPARALLEL_SAFE. I agree with that decision, but it should be documented.
Where?
Several json-related functions can invoke X->json cast functions; the
marking implies that all such cast functions must be PROPARALLEL_SAFE.
That's probably okay, too.- All selectivity estimator functions are PROPARALLEL_SAFE. That implies, for
example, that any operator using eqjoinsel ought to be PROPARALLEL_SAFE or
define its own restriction selectivity estimator function. On the other
hand, the planner need not check the parallel safety of selectivity
estimator functions. If we're already in parallel mode at the moment we
enter the planner, we must be planning a query called within a
PROPARALLEL_SAFE function. If the query uses an unsafe operator, the
containing function was mismarked.
This seems backwards to me. If some hypothetical selectivity
estimator were PROPARALLEL_UNSAFE, then any operator that uses that
function would also need to be PROPARALLEL_UNSAFE. The reverse is not
true. The operator can be as unsafe as it likes and still use a safe
estimator.
- inet_{client,server}_{addr,port} are PROPARALLEL_SAFE; will the information
they need indeed be available in workers?
Nope, marked those PROPARALLEL_RESTRICTED. Thanks.
- Assuming you don't want to propagate XactLastRecEnd from the slave back to
the master, restrict XLogInsert() during parallel mode. Restrict functions
that call it, including pg_create_restore_point, pg_switch_xlog and
pg_stop_backup.
Hmm. Altogether prohibiting XLogInsert() in parallel mode seems
unwise, because it would foreclose heap_page_prune_opt() in workers.
I realize there's separate conversation about whether pruning during
SELECT queries is good policy, but in the interested of separating
mechanism from policy, and in the sure knowledge that allowing at
least some writes in parallel mode is certainly going to be something
people will want, it seems better to look into propagating
XactLastRecEnd.
- pg_extension_config_dump modifies the database, so it shall be
PROPARALLEL_UNSAFE. Assuming pg_stat_clear_snapshot won't reach back to
the master to clear the snapshot, it shall be PROPARALLEL_RESTRICTED.
Right, makes sense.
If a mismarked function elicits "ERROR: cannot retain locks acquired while in
parallel mode", innocent queries later in the transaction get the same error:begin;
select 1; -- ok
savepoint q; select database_to_xml(true, true, 'x'); rollback to q; -- ERROR
select 1; -- ERROR
rollback;
Hmm, I guess that's a bug in the parallel-mode patch. Will fix.
--- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -1610,6 +1610,7 @@ makeRangeConstructors(const char *name, Oid namespace, false, /* leakproof */ false, /* isStrict */ PROVOLATILE_IMMUTABLE, /* volatility */ + PROPARALLEL_UNSAFE, /* parallel safety */Range constructors are PROPARALLEL_SAFE.
Changed.
--- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -496,7 +496,9 @@ init_execution_state(List *queryTree_list, if (queryTree->commandType == CMD_UTILITY) stmt = queryTree->utilityStmt; else - stmt = (Node *) pg_plan_query(queryTree, 0, NULL); + stmt = (Node *) pg_plan_query(queryTree, + fcache->readonly_func ? CURSOR_OPT_PARALLEL_OK : 0, + NULL);(This code is deciding whether to allow parallelism in one statement of an
SQL-language function.) Why does it care if the function is read-only?
I don't know what I was thinking there. Interestingly, changing that
causes several tests to fail with complaints about lock retention:
SELECT p.name, name(p.hobbies), name(equipment(p.hobbies)) FROM ONLY person p;
SELECT p.name, name(p.hobbies), name(equipment(p.hobbies)) FROM person* p;
SELECT name(equipment(p.hobbies)), p.name, name(p.hobbies) FROM ONLY person p;
SELECT (p.hobbies).equipment.name, p.name, name(p.hobbies) FROM person* p;
SELECT (p.hobbies).equipment.name, name(p.hobbies), p.name FROM ONLY person p;
SELECT name(equipment(p.hobbies)), name(p.hobbies), p.name FROM person* p;
I don't immediately know why.
I see that PL/pgSQL never allows parallelism in queries it launches.
Yeah, I mentioned to mention that limitation in my initial email. Fixed.
--- a/src/include/utils/lsyscache.h +++ b/src/include/utils/lsyscache.h @@ -79,6 +79,7 @@ extern bool op_mergejoinable(Oid opno, Oid inputtype); extern bool op_hashjoinable(Oid opno, Oid inputtype); extern bool op_strict(Oid opno); extern char op_volatile(Oid opno); +extern char op_parallel(Oid opno);No such function defined.
Removed.
In passing, I noticed a few cosmetic problems in the prerequisite
parallel-mode-v6.patch. Typos: initating Musn't paralell paralellism
paralllelism processe. Capitalization of "transactionIdPrecedes". When
applying the patch, this:$ git apply ../rev/parallel-mode-v6.patch
../rev/parallel-mode-v6.patch:2897: indent with spaces.
(nLocksAcquiredInParallelMode - n));
warning: 1 line adds whitespace errors.
OK, will fix those. Thanks.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
assess-parallel-safety-v3.patchapplication/x-patch; name=assess-parallel-safety-v3.patchDownload+2982-2683
On Wed, Feb 18, 2015 at 12:23:05PM -0500, Robert Haas wrote:
On Sun, Feb 15, 2015 at 6:24 PM, Noah Misch <noah@leadboat.com> wrote:
It's important for parallelism to work under the extended query protocol, and
that's nontrivial. exec_parse_message() sets cursorOptions.
exec_bind_message() runs the planner. We don't know if a parallel plan is
okay before seeing max_rows in exec_execute_message().Yes, that's a problem. One could require users of the extended query
protocol to indicate their willingness to accept a parallel query plan
when sending the bind message by setting the appropriate cursor
option; and one could, when that option is specified, refuse execute
messages with max_rows != 0. However, that has the disadvantage of
forcing all clients to be updated for the new world order.
Right; that would imply a protocol version bump.
Or, one
could assume a parallel plan is OK and re-plan if we choose one and
then a non-zero max_rows shows up. In the worst case, though, that
could require every query to be planned twice.
That sounds reasonable as a first cut. It is perfect for libpq clients, which
always use zero max_rows. The policy deserves wider scrutiny before release,
but it's the sort of thing we can tweak without wide-ranging effects on the
rest of the parallelism work. One might use the Bind message portal name as
an additional heuristic. libpq usually (always?) uses the empty portal name.
A caller specifying a non-empty portal name is far more likely to have a
non-zero max_rows on its way. More radical still: upon receiving the Bind
message, peek ahead to see if an Execute message is waiting. If so, let the
max_rows of the Execute inform Bind-associated planning.
The CREATE OR REPLACE FUNCTION calls in src/backend/catalog/system_views.sql
reset proparallel=u for some built-in functions, such as make_interval.I can fix that if I add a PARALLEL { SAFE | RESTRICTED | UNSAFE }
clause to CREATE FUNCTION.
Eventually that, but adding "UPDATE pg_proc" after the CORF is a fine stopgap.
Or are you saying that CORF shouldn't
change the existing value of the flag? I'm not sure exactly what our
policy is here, but I would expect that the CORF is doing the right
thing and we need to add the necessary syntax to CREATE FUNCTION and
then add that clause to those calls.
I agree that CREATE OR REPLACE FUNCTION is doing the right thing.
- Several functions, such as array_in and anytextcat, can call arbitrary I/O
functions. This implies a policy that I/O functions must be
PROPARALLEL_SAFE. I agree with that decision, but it should be documented.Where?
On further thought, it can wait for the CREATE FUNCTION syntax additions. At
that time, xtypes.sgml is the place. Perhaps just adding PARALLEL SAFE to the
example CREATE FUNCTION calls will suffice.
- All selectivity estimator functions are PROPARALLEL_SAFE. That implies, for
example, that any operator using eqjoinsel ought to be PROPARALLEL_SAFE or
(I meant to write "eqsel", not "eqjoinsel". eqjoinsel is not a restriction
selectivity estimator function, but the rest of the argument applies to both
functions.) That argument does not apply to all possible selectivity
estimator functions. It is common for an estimator to call the opercode on
MCV and/or histogram values, which makes the estimator function no more
parallel safe than the weakest associated opercode. eqsel, eqjoinsel and
scalarltsel all do that.
define its own restriction selectivity estimator function. On the other
hand, the planner need not check the parallel safety of selectivity
estimator functions. If we're already in parallel mode at the moment we
enter the planner, we must be planning a query called within a
PROPARALLEL_SAFE function. If the query uses an unsafe operator, the
containing function was mismarked.This seems backwards to me. If some hypothetical selectivity
estimator were PROPARALLEL_UNSAFE, then any operator that uses that
function would also need to be PROPARALLEL_UNSAFE.
It's fair to have a PROPARALLEL_UNSAFE estimator for a PROPARALLEL_SAFE
function, because the planning of a parallel query is often not itself done in
parallel mode. In that case, "SELECT * FROM tablename WHERE colname OP 0"
might use a parallel seqscan but fail completely if called from inside a
function running in parallel mode. That is to say, an affected query can
itself use parallelism, but placing the query in a function makes the function
PROPARALLEL_UNSAFE. Surprising, but not wrong.
Rereading my previous message, I failed to make the bottom line clear: I
recommend marking eqsel etc. PROPARALLEL_UNSAFE but *not* checking an
estimator's proparallel before calling it in the planner.
The reverse is not
true. The operator can be as unsafe as it likes and still use a safe
estimator.
Agreed. "contsel" is PROPARALLEL_SAFE no matter what operator uses it.
- Assuming you don't want to propagate XactLastRecEnd from the slave back to
the master, restrict XLogInsert() during parallel mode. Restrict functions
that call it, including pg_create_restore_point, pg_switch_xlog and
pg_stop_backup.Hmm. Altogether prohibiting XLogInsert() in parallel mode seems
unwise, because it would foreclose heap_page_prune_opt() in workers.
I realize there's separate conversation about whether pruning during
SELECT queries is good policy, but in the interested of separating
mechanism from policy, and in the sure knowledge that allowing at
least some writes in parallel mode is certainly going to be something
people will want, it seems better to look into propagating
XactLastRecEnd.
Good points; that works for me.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
