From: Robert Haas Date: Wed, 30 Sep 2015 22:35:40 +0000 (-0400) Subject: Introduce a new GUC force_parallel_mode for testing purposes. X-Git-Url: http://git.postgresql.org/gitweb/static/gitweb.js?a=commitdiff_plain;h=1b4d0cffd00248df5c1c57239284e985ba82cf86;p=users%2Frhaas%2Fpostgres.git Introduce a new GUC force_parallel_mode for testing purposes. When force_parallel_mode = true, we enable the parallel mode restrictions for all queries for which this is believed to be safe. For the subset of those queries believed to be safe to run entirely within a worker, we spin up a worker and run the query there instead of running it in the original process. XXX: This patch still needs some work yet as far as explain.c is concerned. Instead of what happens here at present, we should have a new Boolean inside the Gather node, maybe "bool invisible". explain.c should look through invisible Gather nodes but not others; force_parallel_mode should create an invisible Gather node rather than a normal one. No effect at execution time. XXX: I think force_parallel_mode should have three modes: on, off, regress. When off, we act normally. When on, we push regular, visible Gather nodes on top of plans that can support them, and display error context as usual. The third mode, regress, turns the Gather nodes invisible and suppresses the additional error context. --- diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 392eb700b0..81f9cbd4a7 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -3802,6 +3802,40 @@ SELECT * FROM parent WHERE key = 2400; + + force_parallel_mode (boolean) + + force_parallel_mode configuration parameter + + + + + This setting is intended for testing purposes only. It has + several different but related effects. First, it forces parallel + mode restrictions to be enforced for all queries except those which + the query planner believes would fail to execute properly with + those restrictions in place. If an error occurs when this option is + used, some functions used by the query may need to be marked + PARALLEL UNSAFE. + + + + Second, if the finished plan appears to be safe to run within a + parallel worker process, then the query planner will do so. This + provides a further test of the parallel query facilities and may + reveal additional problems with function markings. + + + + Third, if a parallel worker prints an error or other message while + this option is enabled, the error context indicating that the message + originated from a parallel worker will be suppressed. This makes + it possible to run the same set of regression tests with and without + this option enabled and expect to get the same results. + + + + diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c index bf2e691f57..3915750419 100644 --- a/src/backend/access/transam/parallel.c +++ b/src/backend/access/transam/parallel.c @@ -22,6 +22,7 @@ #include "libpq/pqformat.h" #include "libpq/pqmq.h" #include "miscadmin.h" +#include "optimizer/planmain.h" #include "storage/ipc.h" #include "storage/sinval.h" #include "storage/spin.h" @@ -1079,7 +1080,8 @@ ParallelExtensionTrampoline(dsm_segment *seg, shm_toc *toc) static void ParallelErrorContext(void *arg) { - errcontext("parallel worker, PID %d", *(int32 *) arg); + if (!force_parallel_mode) + errcontext("parallel worker, PID %d", *(int32 *) arg); } /* diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 25d8ca075d..3329d21ec0 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -572,6 +572,7 @@ void ExplainPrintPlan(ExplainState *es, QueryDesc *queryDesc) { Bitmapset *rels_used = NULL; + PlanState *ps; Assert(queryDesc->plannedstmt != NULL); es->pstmt = queryDesc->plannedstmt; @@ -580,7 +581,16 @@ ExplainPrintPlan(ExplainState *es, QueryDesc *queryDesc) es->rtable_names = select_rtable_names_for_explain(es->rtable, rels_used); es->deparse_cxt = deparse_context_for_plan_rtable(es->rtable, es->rtable_names); - ExplainNode(queryDesc->planstate, NIL, NULL, NULL, es); + /* + * XXX. Just for testing purposes, suppress the display of a toplevel + * gather node, so that we can run the regression tests with Gather + * nodes forcibly inserted without getting test failures due to different + * EXPLAIN output. + */ + ps = queryDesc->planstate; + if (IsA(ps, GatherState)) + ps = outerPlanState(ps); + ExplainNode(ps, NIL, NULL, NULL, es); } /* diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index b487c002a8..b0042a108b 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -1826,6 +1826,7 @@ _outPlannerGlobal(StringInfo str, const PlannerGlobal *node) WRITE_BOOL_FIELD(hasRowSecurity); WRITE_BOOL_FIELD(parallelModeOK); WRITE_BOOL_FIELD(parallelModeNeeded); + WRITE_BOOL_FIELD(wholePlanParallelSafe); WRITE_BOOL_FIELD(hasForeignJoin); } diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index bdac0b1860..ed0632984d 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -212,6 +212,10 @@ create_plan(PlannerInfo *root, Path *best_path) /* Recursively process the path tree */ plan = create_plan_recurse(root, best_path); + /* Update parallel safety information if needed. */ + if (!best_path->parallel_safe) + root->glob->wholePlanParallelSafe = false; + /* Check we successfully assigned all NestLoopParams to plan nodes */ if (root->curOuterParams != NIL) elog(ERROR, "failed to assign all NestLoopParams to plan nodes"); diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index a09b4b5b47..1204b42ce2 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -48,10 +48,12 @@ #include "storage/dsm_impl.h" #include "utils/rel.h" #include "utils/selfuncs.h" +#include "utils/syscache.h" -/* GUC parameter */ +/* GUC parameters */ double cursor_tuple_fraction = DEFAULT_CURSOR_TUPLE_FRACTION; +bool force_parallel_mode = false; /* Hook for plugins to get control in planner() */ planner_hook_type planner_hook = NULL; @@ -230,25 +232,36 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) !has_parallel_hazard((Node *) parse, true); /* - * glob->parallelModeOK should tell us whether it's necessary to impose - * the parallel mode restrictions, but we don't actually want to impose - * them unless we choose a parallel plan, so that people who mislabel - * their functions but don't use parallelism anyway aren't harmed. - * However, it's useful for testing purposes to be able to force the - * restrictions to be imposed whenever a parallel plan is actually chosen - * or not. + * glob->parallelModeNeeded should tell us whether it's necessary to + * impose the parallel mode restrictions, but we don't actually want to + * impose them unless we choose a parallel plan, so that people who + * mislabel their functions but don't use parallelism anyway aren't + * harmed. But when force_parallel_mode is set, we enable the restrictions + * whenever possible for testing purposes. * - * (It's been suggested that we should always impose these restrictions - * whenever glob->parallelModeOK is true, so that it's easier to notice - * incorrectly-labeled functions sooner. That might be the right thing to - * do, but for now I've taken this approach. We could also control this - * with a GUC.) + * glob->wholePlanParallelSafe should tell us whether it's OK to stick a + * Gather node on top of the entire plan. However, it only needs to be + * accurate when force_parallel_mode is set, so we don't bother doing the + * work otherwise. The value we set here is just a preliminary guess; it + * may get changed from true to false later, but not visca versa. */ -#ifdef FORCE_PARALLEL_MODE - glob->parallelModeNeeded = glob->parallelModeOK; -#else - glob->parallelModeNeeded = false; -#endif + if (!force_parallel_mode || !glob->parallelModeOK) + { + glob->parallelModeNeeded = false; + glob->wholePlanParallelSafe = false; /* either false or don't care */ + } + else + { + glob->parallelModeNeeded = true; + glob->wholePlanParallelSafe = + has_parallel_hazard((Node *) parse, false); + } + + /* + * We might flip this from true to false later on during planning, but + * never the other direction. + */ + glob->wholePlanParallelSafe = glob->parallelModeOK; /* Determine what fraction of the plan is likely to be scanned */ if (cursorOptions & CURSOR_OPT_FAST_PLAN) @@ -292,6 +305,33 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams) top_plan = materialize_finished_plan(top_plan); } + /* + * At present, we don't copy subplans to workers. The presence of a + * subplan in one part of the plan doesn't preclude the use of parallelism + * in some other part of the plan, but it does preclude the possibility of + * regarding the entire plan parallel-safe. + */ + if (glob->subplans != NULL) + glob->wholePlanParallelSafe = false; + + /* + * Optionally add a Gather node for testing purposes, provided this is + * actually a safe thing to do. + */ + if (glob->wholePlanParallelSafe && force_parallel_mode) + { + Gather *gather = makeNode(Gather); + + gather->plan.targetlist = top_plan->targetlist; + gather->plan.qual = NIL; + gather->plan.lefttree = top_plan; + gather->plan.righttree = NULL; + gather->num_workers = 1; + gather->single_copy = true; + root->glob->parallelModeNeeded = true; + top_plan = &gather->plan; + } + /* * If any Params were generated, run through the plan tree and compute * each plan node's extParam/allParam sets. Ideally we'd merge this into diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index d949c19ca2..c25656ad2e 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -863,6 +863,17 @@ static struct config_bool ConfigureNamesBool[] = true, NULL, NULL, NULL }, + + { + {"force_parallel_mode", PGC_USERSET, QUERY_TUNING_OTHER, + gettext_noop("Forces use of parallel query facilities."), + gettext_noop("If possible, run query using a parallel worker and with parallel restrictions.") + }, + &force_parallel_mode, + false, + NULL, NULL, NULL + }, + { {"geqo", PGC_USERSET, QUERY_TUNING_GEQO, gettext_noop("Enables genetic query optimization."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 930628c33b..06bc3fa51c 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -313,6 +313,7 @@ #from_collapse_limit = 8 #join_collapse_limit = 8 # 1 disables collapsing of explicit # JOIN clauses +#force_parallel_mode = off #------------------------------------------------------------------------------ diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index 94925984bf..5c22679afa 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -108,6 +108,9 @@ typedef struct PlannerGlobal bool parallelModeOK; /* parallel mode potentially OK? */ bool parallelModeNeeded; /* parallel mode actually required? */ + + bool wholePlanParallelSafe; /* is the entire plan parallel safe? */ + bool hasForeignJoin; /* does have a pushed down foreign join */ } PlannerGlobal; diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h index 7ae73676e8..f38f017933 100644 --- a/src/include/optimizer/planmain.h +++ b/src/include/optimizer/planmain.h @@ -20,6 +20,7 @@ /* GUC parameters */ #define DEFAULT_CURSOR_TUPLE_FRACTION 0.1 extern double cursor_tuple_fraction; +extern bool force_parallel_mode; /* query_planner callback to compute query_pathkeys */ typedef void (*query_pathkeys_callback) (PlannerInfo *root, void *extra);