Skip to content

Commit 1c566d0

Browse files
committed
NGSOK-1265 Allow to use * in expected_executor_group_sets
The logic will populate all executor groups into Frontend to decide which one to use there.
1 parent 522bada commit 1c566d0

4 files changed

Lines changed: 65 additions & 7 deletions

File tree

be/src/scheduling/cluster-membership-mgr-test.cc

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,13 @@ TEST(ClusterMembershipMgrUnitTest, TestPopulateExpectedExecGroupSets) {
707707
EXPECT_FALSE(status.ok());
708708
EXPECT_EQ(status.msg().GetFullMessageDetails(),
709709
"Executor group set prefix specified multiple times: group-prefix1:10\n");
710+
711+
// Case 10: Star in FLAGS_expected_executor_group_sets
712+
FLAGS_expected_executor_group_sets = "*";
713+
expected_exec_group_sets.clear();
714+
status = ClusterMembershipMgr::PopulateExpectedExecGroupSets(expected_exec_group_sets);
715+
EXPECT_TRUE(status.ok());
716+
EXPECT_EQ(expected_exec_group_sets.size(), 0);
710717
}
711718

712719
/// This ensures that all executor group configuration scenarios possible using available
@@ -838,6 +845,30 @@ TEST(ClusterMembershipMgrUnitTest, PopulateExecutorMembershipRequest) {
838845
EXPECT_EQ(update_req.exec_group_sets[1].exec_group_name_prefix, "bar");
839846
snapshot_ptr->executor_groups.clear();
840847
}
848+
849+
// Case 3: Using executor groups, expected_exec_group_sets is *
850+
{
851+
FLAGS_expected_executor_group_sets = "*";
852+
853+
ExecutorGroup exec_group("foo-group1", 1);
854+
exec_group.AddExecutor(MakeBackendDescriptor(1, exec_group, 0));
855+
snapshot_ptr->executor_groups.insert({exec_group.name(), exec_group});
856+
ExecutorGroup exec_group2("bar-group1", 1);
857+
exec_group2.AddExecutor(MakeBackendDescriptor(1, exec_group2, 1));
858+
exec_group2.AddExecutor(MakeBackendDescriptor(2, exec_group2, 2));
859+
snapshot_ptr->executor_groups.insert({exec_group2.name(), exec_group2});
860+
ClusterMembershipMgr::SnapshotPtr ptr = snapshot_ptr;
861+
PopulateExecutorMembershipRequest(ptr, empty_exec_group_sets, update_req);
862+
EXPECT_EQ(update_req.exec_group_sets.size(), 2);
863+
// reverse order is ok
864+
EXPECT_EQ(update_req.exec_group_sets[1].curr_num_executors, 1);
865+
EXPECT_EQ(update_req.exec_group_sets[1].expected_num_executors, -1);
866+
EXPECT_EQ(update_req.exec_group_sets[1].exec_group_name_prefix, "foo-group1");
867+
EXPECT_EQ(update_req.exec_group_sets[0].curr_num_executors, 2);
868+
EXPECT_EQ(update_req.exec_group_sets[0].expected_num_executors, -1);
869+
EXPECT_EQ(update_req.exec_group_sets[0].exec_group_name_prefix, "bar-group1");
870+
snapshot_ptr->executor_groups.clear();
871+
}
841872
}
842873

843874
template <class T>

be/src/scheduling/cluster-membership-mgr.cc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,17 @@ void PopulateExecutorMembershipRequest(const ClusterMembershipMgr::SnapshotPtr&
737737
}
738738
}
739739
} else {
740+
if (FLAGS_expected_executor_group_sets == "*") {
741+
LOG(INFO) <<
742+
"Special case handling for FLAGS_expected_executor_group_sets == \"*\"";
743+
for (const auto& it : snapshot->executor_groups) {
744+
// order does not matter
745+
exec_group_sets.emplace_back();
746+
exec_group_sets.back().__set_exec_group_name_prefix(it.first);
747+
// We set expected_num_executors to -1 to identify automation from Frontend
748+
exec_group_sets.back().__set_expected_num_executors(-1);
749+
}
750+
} else
740751
if (expected_exec_group_sets.empty()) {
741752
// Add a default exec group set if no expected group sets were specified.
742753
exec_group_sets.emplace_back();
@@ -745,6 +756,7 @@ void PopulateExecutorMembershipRequest(const ClusterMembershipMgr::SnapshotPtr&
745756
exec_group_sets.insert(exec_group_sets.begin(), expected_exec_group_sets.begin(),
746757
expected_exec_group_sets.end());
747758
}
759+
748760
int matching_exec_groups_found = 0;
749761
for (auto& set : exec_group_sets) {
750762
int max_num_executors = -1;
@@ -788,6 +800,11 @@ Status ClusterMembershipMgr::PopulateExpectedExecGroupSets(
788800
expected_exec_group_sets.clear();
789801
std::unordered_set<string> parsed_group_prefixes;
790802
vector<StringPiece> groups;
803+
804+
if (FLAGS_expected_executor_group_sets == "*") {
805+
return Status::OK();
806+
}
807+
791808
groups = strings::Split(FLAGS_expected_executor_group_sets, ",", strings::SkipEmpty());
792809
if (groups.empty()) return Status::OK();
793810

be/src/service/impala-server.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,8 @@ DEFINE_string(expected_executor_group_sets, "",
311311
"prefix1-group1, prefix1-group2, etc. The expected group size (number of executors "
312312
"in each group) is used during planning when no healthy executor group is available. "
313313
"If this flag is used then any executor groups that do not map to the specified group"
314-
" sets will never be used to schedule queries.");
314+
" sets will never be used to schedule queries. If this flag set to “*“ will populate "
315+
"all healthy resource groups.");
315316

316317
// TODO: can we automatically choose a startup grace period based on the max admission
317318
// control queue timeout + some margin for error?

fe/src/main/java/org/apache/impala/service/Frontend.java

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2113,12 +2113,21 @@ public static List<TExecutorGroupSet> setupThresholdsForExecutorGroupSets(
21132113

21142114
// Executor groups exist in the cluster. Identify those that can be used.
21152115
for (TExecutorGroupSet e : executorGroupSets) {
2116-
// If defined, request_pool can be a suffix of the group name prefix. For example
2117-
// group_set_prefix = root.queue1
2118-
// request_pool = queue1
2119-
if (StringUtils.isNotEmpty(request_pool)
2120-
&& !e.getExec_group_name_prefix().endsWith(request_pool)) {
2121-
continue;
2116+
if (StringUtils.isNotEmpty(request_pool)) {
2117+
// If defined, request_pool can be a suffix of the group name prefix. For example
2118+
// group_set_prefix = root.queue1
2119+
// request_pool = queue1
2120+
if (!e.getExec_group_name_prefix().endsWith(request_pool)
2121+
&& e.getExpected_num_executors() >= 0) {
2122+
continue;
2123+
}
2124+
// in case of automation (we set expected_num_executors == -1 in that case) we will have
2125+
// group_set_prefix = queue1-1-2-3
2126+
// request_pool = queue1
2127+
if (!e.getExec_group_name_prefix().startsWith(request_pool)
2128+
&& e.getExpected_num_executors() < 0) {
2129+
continue;
2130+
}
21222131
}
21232132
TExecutorGroupSet new_entry = new TExecutorGroupSet(e);
21242133
if (poolService != null) {

0 commit comments

Comments
 (0)