From 1d6fb35ad62968c8678d0321887e2b9ca8fe1a84 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Wed, 13 Dec 2017 15:19:28 -0500 Subject: [PATCH] Revert "Fix accumulation of parallel worker instrumentation." This reverts commit 2c09a5c12a66087218c7f8cba269cd3de51b9b82. Per further discussion, that doesn't seem to be the best possible fix. Discussion: http://postgr.es/m/CAA4eK1LW2aFKzY3=vwvc=t-juzPPVWP2uT1bpx_MeyEqnM+p8g@mail.gmail.com --- src/backend/executor/execParallel.c | 51 +++++-------------- src/test/regress/expected/select_parallel.out | 21 -------- src/test/regress/sql/select_parallel.sql | 7 --- 3 files changed, 13 insertions(+), 66 deletions(-) diff --git a/src/backend/executor/execParallel.c b/src/backend/executor/execParallel.c index 558cb08b07..d57cdbd4e1 100644 --- a/src/backend/executor/execParallel.c +++ b/src/backend/executor/execParallel.c @@ -819,19 +819,6 @@ ExecParallelReinitialize(PlanState *planstate, /* Old workers must already be shut down */ Assert(pei->finished); - /* Clear the instrumentation space from the last round. */ - if (pei->instrumentation) - { - Instrumentation *instrument; - SharedExecutorInstrumentation *sh_instr; - int i; - - sh_instr = pei->instrumentation; - instrument = GetInstrumentationArray(sh_instr); - for (i = 0; i < sh_instr->num_workers * sh_instr->num_plan_nodes; ++i) - InstrInit(&instrument[i], pei->planstate->state->es_instrument); - } - /* Force parameters we're going to pass to workers to be evaluated. */ ExecEvalParamExecParams(sendParams, estate); @@ -953,33 +940,21 @@ ExecParallelRetrieveInstrumentation(PlanState *planstate, for (n = 0; n < instrumentation->num_workers; ++n) InstrAggNode(planstate->instrument, &instrument[n]); - if (!planstate->worker_instrument) - { - /* - * Allocate space for the per-worker detail. - * - * Worker instrumentation should be allocated in the same context as - * the regular instrumentation information, which is the per-query - * context. Switch into per-query memory context. - */ - oldcontext = MemoryContextSwitchTo(planstate->state->es_query_cxt); - ibytes = - mul_size(instrumentation->num_workers, sizeof(Instrumentation)); - planstate->worker_instrument = - palloc(ibytes + offsetof(WorkerInstrumentation, instrument)); - MemoryContextSwitchTo(oldcontext); - - for (n = 0; n < instrumentation->num_workers; ++n) - InstrInit(&planstate->worker_instrument->instrument[n], - planstate->state->es_instrument); - } + /* + * Also store the per-worker detail. + * + * Worker instrumentation should be allocated in the same context as the + * regular instrumentation information, which is the per-query context. + * Switch into per-query memory context. + */ + oldcontext = MemoryContextSwitchTo(planstate->state->es_query_cxt); + ibytes = mul_size(instrumentation->num_workers, sizeof(Instrumentation)); + planstate->worker_instrument = + palloc(ibytes + offsetof(WorkerInstrumentation, instrument)); + MemoryContextSwitchTo(oldcontext); planstate->worker_instrument->num_workers = instrumentation->num_workers; - - /* Accumulate the per-worker detail. */ - for (n = 0; n < instrumentation->num_workers; ++n) - InstrAggNode(&planstate->worker_instrument->instrument[n], - &instrument[n]); + memcpy(&planstate->worker_instrument->instrument, instrument, ibytes); /* Perform any node-type-specific work that needs to be done. */ switch (nodeTag(planstate)) diff --git a/src/test/regress/expected/select_parallel.out b/src/test/regress/expected/select_parallel.out index ff00d47f65..86a55922c8 100644 --- a/src/test/regress/expected/select_parallel.out +++ b/src/test/regress/expected/select_parallel.out @@ -465,28 +465,7 @@ select count(*) from bmscantest where a>1; 99999 (1 row) --- test accumulation of stats for parallel node reset enable_seqscan; -alter table tenk2 set (parallel_workers = 0); -explain (analyze, timing off, summary off, costs off) - select count(*) from tenk1, tenk2 where tenk1.hundred > 1 - and tenk2.thousand=0; - QUERY PLAN --------------------------------------------------------------------------- - Aggregate (actual rows=1 loops=1) - -> Nested Loop (actual rows=98000 loops=1) - -> Seq Scan on tenk2 (actual rows=10 loops=1) - Filter: (thousand = 0) - Rows Removed by Filter: 9990 - -> Gather (actual rows=9800 loops=10) - Workers Planned: 4 - Workers Launched: 4 - -> Parallel Seq Scan on tenk1 (actual rows=1960 loops=50) - Filter: (hundred > 1) - Rows Removed by Filter: 40 -(11 rows) - -alter table tenk2 reset (parallel_workers); reset enable_indexscan; reset enable_hashjoin; reset enable_mergejoin; diff --git a/src/test/regress/sql/select_parallel.sql b/src/test/regress/sql/select_parallel.sql index 1035d04d1a..fb35ca3376 100644 --- a/src/test/regress/sql/select_parallel.sql +++ b/src/test/regress/sql/select_parallel.sql @@ -179,14 +179,7 @@ insert into bmscantest select r, 'fooooooooooooooooooooooooooooooooooooooooooooo create index i_bmtest ON bmscantest(a); select count(*) from bmscantest where a>1; --- test accumulation of stats for parallel node reset enable_seqscan; -alter table tenk2 set (parallel_workers = 0); -explain (analyze, timing off, summary off, costs off) - select count(*) from tenk1, tenk2 where tenk1.hundred > 1 - and tenk2.thousand=0; -alter table tenk2 reset (parallel_workers); - reset enable_indexscan; reset enable_hashjoin; reset enable_mergejoin;