Retire GUC parallel_hash_enable_motion_broadcast

It's added by history reasons as a workaround. We don't have exactly the right parallel feature at the time. It's neither reasonable to keep it, nor we have test cases for this wired GUC. Retire it and keep codes same with UPSTREAM. Authored-by: Zhang Mingli avamingli@gmail.com
apache · Aug 8, 2023 · 2b3ae2f · 2b3ae2f
1 parent 4a9c401
commit 2b3ae2f
Show file tree

Hide file tree

Showing 8 changed files with 12 additions and 67 deletions.
diff --git a/src/backend/cdb/cdbpath.c b/src/backend/cdb/cdbpath.c
@@ -2871,8 +2871,7 @@ cdbpath_motion_for_parallel_join(PlannerInfo *root,
 						List *inner_pathkeys,
 						bool outer_require_existing_order,
 						bool inner_require_existing_order,
-						bool parallel_aware,
-						bool uninterested_broadcast)
+						bool parallel_aware)
 {
 	CdbpathMfjRel outer;
 	CdbpathMfjRel inner;
@@ -3699,7 +3698,7 @@ cdbpath_motion_for_parallel_join(PlannerInfo *root,
 		else if (!small_rel->require_existing_order &&
 				 small_rel->ok_to_replicate &&
 				 ((!parallel_aware && (small_rel->bytes * CdbPathLocus_NumSegmentsPlusParallelWorkers(large_rel->locus) < large_rel->bytes)) ||
-				  (parallel_aware && !uninterested_broadcast && (small_rel->bytes * CdbPathLocus_NumSegments(large_rel->locus) < large_rel->bytes))))
+				  (parallel_aware && (small_rel->bytes * CdbPathLocus_NumSegments(large_rel->locus) < large_rel->bytes))))
 				{
 					if (!parallel_aware)
 						CdbPathLocus_MakeReplicated(&small_rel->move_to,
@@ -3718,7 +3717,7 @@ cdbpath_motion_for_parallel_join(PlannerInfo *root,
 		else if (!large_rel->require_existing_order &&
 				 large_rel->ok_to_replicate &&
 				 ((!parallel_aware && (large_rel->bytes * CdbPathLocus_NumSegmentsPlusParallelWorkers(small_rel->locus) < small_rel->bytes)) ||
-				  (parallel_aware && !uninterested_broadcast && (large_rel->bytes * CdbPathLocus_NumSegments(small_rel->locus) < small_rel->bytes))))
+				  (parallel_aware && (large_rel->bytes * CdbPathLocus_NumSegments(small_rel->locus) < small_rel->bytes))))
 				{
 					if (!parallel_aware)
 						CdbPathLocus_MakeReplicated(&large_rel->move_to,
@@ -3749,7 +3748,7 @@ cdbpath_motion_for_parallel_join(PlannerInfo *root,
 		else if (!small_rel->require_existing_order &&
 				 small_rel->ok_to_replicate &&
 				 ((!parallel_aware && (small_rel->bytes * CdbPathLocus_NumSegmentsPlusParallelWorkers(large_rel->locus) < small_rel->bytes + large_rel->bytes)) ||
-					(parallel_aware && !uninterested_broadcast && (small_rel->bytes * CdbPathLocus_NumSegments(large_rel->locus) < small_rel->bytes + large_rel->bytes))))
+					(parallel_aware && (small_rel->bytes * CdbPathLocus_NumSegments(large_rel->locus) < small_rel->bytes + large_rel->bytes))))
 				{
 					if (!parallel_aware)
 						CdbPathLocus_MakeReplicated(&small_rel->move_to,
@@ -3765,7 +3764,7 @@ cdbpath_motion_for_parallel_join(PlannerInfo *root,
 				else if (!large_rel->require_existing_order &&
 						 large_rel->ok_to_replicate &&
 						 ((!parallel_aware && (large_rel->bytes * CdbPathLocus_NumSegmentsPlusParallelWorkers(small_rel->locus) < small_rel->bytes + large_rel->bytes)) ||
-						  (parallel_aware && !uninterested_broadcast && (large_rel->bytes * CdbPathLocus_NumSegments(small_rel->locus) < small_rel->bytes + large_rel->bytes))))
+						  (parallel_aware && (large_rel->bytes * CdbPathLocus_NumSegments(small_rel->locus) < small_rel->bytes + large_rel->bytes))))
 				{
 					if (!parallel_aware)
 						CdbPathLocus_MakeReplicated(&large_rel->move_to,

diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
@@ -1089,8 +1089,7 @@ try_hashjoin_path(PlannerInfo *root,
 									  extra->restrictlist,
 									  required_outer,
 									  extra->redistribution_clauses,
-									  hashclauses, 
-									  false),
+									  hashclauses),
 				 root);
 	}
 	else
@@ -1147,40 +1146,6 @@ try_partial_hashjoin_path(PlannerInfo *root,
 	if (!add_partial_path_precheck(joinrel, workspace.total_cost, NIL))
 		return;
 
-	/*
-	 * CBDB_PARALLEL_FIXME
-	 * Customers encounter an issue that when parallel hash, broadcast motion
-	 * a smaller table may be worser than redistribute a big table.
-	 * We add a path whic doesn't try broadcast if possible.
-	 * And let the path cost decide which is better. 
-	 */
-	if (parallel_hash)
-	{
-		hashpath = create_hashjoin_path(root,
-										joinrel,
-										jointype,
-										orig_jointype,
-										&workspace,
-										extra,
-										outer_path,
-										inner_path,
-										true,
-										extra->restrictlist,
-										NULL,
-										extra->redistribution_clauses,
-										hashclauses,
-										true); /* not use broadcast */
-		if (hashpath && hashpath->parallel_safe)
-			add_partial_path(joinrel, hashpath);
-	}
-
-	/* 
-	 * CBDB_PARALLEL_FIXME:
-	 * We only want non-broadcast in parallel hash if the guc is set.
-	 */
-	if (parallel_hash && !parallel_hash_enable_motion_broadcast)
-		return;
-
 	hashpath = create_hashjoin_path(root,
 									joinrel,
 									jointype,
@@ -1193,8 +1158,8 @@ try_partial_hashjoin_path(PlannerInfo *root,
 									extra->restrictlist,
 									NULL,
 									extra->redistribution_clauses,
-									hashclauses,
-									false);
+									hashclauses);
+
 	/* Might be good enough to be worth trying and no motion, so let's try it. */
 	if (hashpath && hashpath->parallel_safe)
 		add_partial_path(joinrel, hashpath);

diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
@@ -3911,7 +3911,6 @@ create_nestloop_path(PlannerInfo *root,
 											 NIL,
 											 outer_must_be_local,
 											 inner_must_be_local,
-											 false,
 											 false);
 	}
 
@@ -4191,7 +4190,6 @@ create_mergejoin_path(PlannerInfo *root,
 											 innermotionkeys,
 											 preserve_outer_ordering,
 											 preserve_inner_ordering,
-											 false,
 											 false);
 	}
 
@@ -4332,8 +4330,7 @@ create_hashjoin_path(PlannerInfo *root,
 					 List *restrict_clauses,
 					 Relids required_outer,
 					 List *redistribution_clauses, /* CDB */
-					 List *hashclauses,
-					 bool uninterested_broadcast) /* GPDB parallel */
+					 List *hashclauses)
 {
 	HashPath   *pathnode;
 	CdbPathLocus join_locus;
@@ -4378,8 +4375,7 @@ create_hashjoin_path(PlannerInfo *root,
 											 NIL,
 											 outer_must_be_local,
 											 inner_must_be_local,
-											 parallel_hash,
-											 uninterested_broadcast);
+											 parallel_hash);
 	}
 
 	if (CdbPathLocus_IsNull(join_locus))

diff --git a/src/backend/utils/misc/guc_gp.c b/src/backend/utils/misc/guc_gp.c
@@ -303,7 +303,6 @@ bool		optimizer_enable_indexjoin;
 bool		optimizer_enable_motions_masteronly_queries;
 bool		optimizer_enable_motions;
 bool		optimizer_enable_motion_broadcast;
-bool		parallel_hash_enable_motion_broadcast;
 bool		optimizer_enable_motion_gather;
 bool		optimizer_enable_motion_redistribute;
 bool		optimizer_enable_sort;
@@ -2090,16 +2089,6 @@ struct config_bool ConfigureNamesBool_gp[] =
 		true,
 		NULL, NULL, NULL
 	},
-	{
-		{"parallel_hash_enable_motion_broadcast", PGC_USERSET, DEVELOPER_OPTIONS,
-			gettext_noop("Enable plans with Motion Broadcast operators in parallel hash join."),
-			NULL,
-			GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE
-		},
-		&parallel_hash_enable_motion_broadcast,
-		true,
-		NULL, NULL, NULL
-	},
 	{
 		{"optimizer_enable_motion_gather", PGC_USERSET, DEVELOPER_OPTIONS,
 			gettext_noop("Enable plans with Motion Gather operators in the optimizer."),

diff --git a/src/include/cdb/cdbpath.h b/src/include/cdb/cdbpath.h
@@ -74,7 +74,6 @@ cdbpath_motion_for_parallel_join(PlannerInfo    *root,
 						List           *inner_pathkeys,
 						bool            outer_require_existing_order,
 						bool            inner_require_existing_order,
-						bool			parallel_aware,
-						bool			uninterested_broadcast); /* for parallel hash join, do not use Broadcast if possible */
+						bool			parallel_aware);
 
 #endif   /* CDBPATH_H */
diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h
@@ -216,8 +216,7 @@ extern Path *create_hashjoin_path(PlannerInfo *root,
 									  List *restrict_clauses,
 									  Relids required_outer,
 									  List *redistribution_clauses,    /*CDB*/
-									  List *hashclauses,
-									  bool uninterested_broadcast); /* GPDB parallel */
+									  List *hashclauses);
 
 extern ProjectionPath *create_projection_path(PlannerInfo *root,
 											  RelOptInfo *rel,

diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
@@ -496,7 +496,6 @@ extern bool optimizer_enable_indexjoin;
 extern bool optimizer_enable_motions_masteronly_queries;
 extern bool optimizer_enable_motions;
 extern bool optimizer_enable_motion_broadcast;
-extern bool parallel_hash_enable_motion_broadcast;
 extern bool optimizer_enable_motion_gather;
 extern bool optimizer_enable_motion_redistribute;
 extern bool optimizer_enable_sort;

diff --git a/src/include/utils/unsync_guc_name.h b/src/include/utils/unsync_guc_name.h
@@ -424,7 +424,6 @@
 		"optimizer_enable_materialize",
 		"optimizer_enable_mergejoin",
 		"optimizer_enable_motion_broadcast",
-		"parallel_hash_enable_motion_broadcast",
 		"optimizer_enable_motion_gather",
 		"optimizer_enable_motion_redistribute",
 		"optimizer_enable_motions",