From 6de3a92e7933077006b614ee74095ea1a468fc18 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 27 Sep 2016 17:19:33 -0700 Subject: [PATCH 1/3] Lower "waiting for activity on tasks took longer than" log level. It's perfectly normal to wait longer in several circumstances, and the output can lead to spurious regression output changes. --- src/backend/distributed/executor/multi_client_executor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/distributed/executor/multi_client_executor.c b/src/backend/distributed/executor/multi_client_executor.c index f5f0df1cc..da852d5c1 100644 --- a/src/backend/distributed/executor/multi_client_executor.c +++ b/src/backend/distributed/executor/multi_client_executor.c @@ -883,7 +883,7 @@ MultiClientWait(WaitInfo *waitInfo) } else if (rc == 0) { - ereport(DEBUG2, + ereport(DEBUG5, (errmsg("waiting for activity on tasks took longer than %ld ms", (long) RemoteTaskCheckInterval * 10))); } From 9ebc46d15cfd84e9f21c627803354b351be46089 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 27 Sep 2016 17:20:37 -0700 Subject: [PATCH 2/3] Initialize count_agg_clauses argument to 0. count_agg_clause *adds* the cost of the aggregates to the state variable, it doesn't reinitialize it. That is intentional, as it is used to incrementally add costs in some places. --- src/backend/distributed/planner/multi_master_planner.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/backend/distributed/planner/multi_master_planner.c b/src/backend/distributed/planner/multi_master_planner.c index 05a522e6e..5efd68fc5 100644 --- a/src/backend/distributed/planner/multi_master_planner.c +++ b/src/backend/distributed/planner/multi_master_planner.c @@ -134,6 +134,7 @@ BuildAggregatePlan(Query *masterQuery, Plan *subPlan) Assert(masterQuery->hasAggs || masterQuery->groupClause); aggregateTargetList = masterQuery->targetList; + memset(&aggregateCosts, 0, sizeof(AggClauseCosts)); count_agg_clauses(NULL, (Node *) aggregateTargetList, &aggregateCosts); /* From 3f8fb191ccfb792796aa438d48ce81c20006d60d Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Mon, 3 Oct 2016 13:05:05 -0700 Subject: [PATCH 3/3] Don't create hash-table of zero size in TaskHashCreate(). hash_create(), called by TaskHashCreate(), doesn't work correctly for a zero sized hash table. This triggers valgrind errors, and could potentially cause crashes even without valgring. This currently happens for Jobs with 0 tasks. These probably should be optimized away before reaching TaskHashCreate(), but that's a bigger change. --- .../distributed/executor/multi_task_tracker_executor.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/backend/distributed/executor/multi_task_tracker_executor.c b/src/backend/distributed/executor/multi_task_tracker_executor.c index e08657e98..8bdcdacd3 100644 --- a/src/backend/distributed/executor/multi_task_tracker_executor.c +++ b/src/backend/distributed/executor/multi_task_tracker_executor.c @@ -519,6 +519,15 @@ TaskHashCreate(uint32 taskHashSize) int hashFlags = 0; HTAB *taskHash = NULL; + /* + * Can't create a hashtable of size 0. Normally that shouldn't happen, but + * shard pruning currently can lead to this (Job with 0 Tasks). See #833. + */ + if (taskHashSize == 0) + { + taskHashSize = 2; + } + memset(&info, 0, sizeof(info)); info.keysize = sizeof(TaskMapKey); info.entrysize = sizeof(TaskMapEntry);