diff --git a/src/v/cluster/scheduling/leader_balancer_constraints.cc b/src/v/cluster/scheduling/leader_balancer_constraints.cc index afc73cdbb8f04..b6b45b5b29e23 100644 --- a/src/v/cluster/scheduling/leader_balancer_constraints.cc +++ b/src/v/cluster/scheduling/leader_balancer_constraints.cc @@ -46,15 +46,6 @@ void even_topic_distribution_constraint::update_index(const reassignment& r) { _topic_shard_index.at(topic_id).at(r.to) += 1; } -std::optional -even_topic_distribution_constraint::recommended_reassignment() { - // This method is deprecated and is ony used in `leader_balancer_greedy` - // which doesn't use the `even_topic_distributon_constraint`. Hence there is - // no need to implement it here. Once the greedy balancer has been removed - // this should be removed as well. - vassert(false, "not implemented"); -} - void even_topic_distribution_constraint::rebuild_indexes() { _topic_shard_index.clear(); _topic_replica_index.clear(); @@ -153,64 +144,6 @@ double even_topic_distribution_constraint::adjusted_error( return current_error; } -std::optional -even_shard_load_constraint::recommended_reassignment() { - auto [load, load_map] = build_load_indexes(); - const auto curr_error = error(); - - // Consider each group from high load core, and record the reassignment - // involving the lowest load "to" core. - for (const auto& from : boost::adaptors::reverse(load)) { - if (mi().muted_nodes().contains(from->first.node_id)) { - continue; - } - - constexpr size_t load_unset = std::numeric_limits::max(); - size_t lowest_load = load_unset; - reassignment lowest_reassign{}; - - // Consider each group from high load core, and record the - // reassignment involving the lowest load "to" core. - for (const auto& group : from->second) { - if (mi().muted_groups().contains( - static_cast(group.first))) { - continue; - } - - // iterate over all the replicas and look for the lowest load - // shard in the replica list - for (const auto& to_shard : group.second) { - auto load = load_map.at(to_shard); - if (likely(load >= lowest_load)) { - // there is no point in evaluating this move, it is - // worse than the best one we've found so far. - continue; - } - - if (mi().muted_nodes().contains(to_shard.node_id)) { - continue; - } - - lowest_load = load; - lowest_reassign = {group.first, from->first, to_shard}; - } - } - - if (lowest_load != load_unset) { - // We found a possible reassignment while looking at the current - // "from" shard, and while it is the best possible reassignment - // found it may not improve the error - auto new_error = adjusted_error( - curr_error, lowest_reassign.from, lowest_reassign.to); - if (new_error + error_jitter < curr_error) { - return lowest_reassign; - } - } - } - - return std::nullopt; -} - double even_shard_load_constraint::adjusted_error( double current_error, const model::broker_shard& from, @@ -320,12 +253,4 @@ double pinning_constraint::evaluate_internal(const reassignment& r) { return diff; } -std::optional pinning_constraint::recommended_reassignment() { - // This method is deprecated and is ony used in `leader_balancer_greedy` - // which doesn't use the `even_topic_distributon_constraint`. Hence there is - // no need to implement it here. Once the greedy balancer has been removed - // this should be removed as well. - vassert(false, "not implemented"); -} - } // namespace cluster::leader_balancer_types diff --git a/src/v/cluster/scheduling/leader_balancer_constraints.h b/src/v/cluster/scheduling/leader_balancer_constraints.h index eeb578c632257..f53f3809acb62 100644 --- a/src/v/cluster/scheduling/leader_balancer_constraints.h +++ b/src/v/cluster/scheduling/leader_balancer_constraints.h @@ -133,20 +133,6 @@ class even_topic_distribution_constraint final void update_index(const reassignment& r) override; - /* - * Uses a greedy algorithm to generate a recommended reassignment - * that will reduce error for this constraint. - * - * The gist is that it searches topics in order of how skewed they are. - * Then for each topic it finds a partiton on the node with the most - * leadership and tries to move it to one of its replica nodes. If this - * reassignment reduces error its returned. - * - * If no reassignment can reduce error for this constraint std::nullopt is - * returned. - */ - std::optional recommended_reassignment() override; - private: std::reference_wrapper _si; std::reference_wrapper _mi; @@ -233,23 +219,6 @@ class even_shard_load_constraint final _error = adjusted_error(_error, r.from, r.to); } - /* - * Find a group reassignment that improves overall error. The general - * approach is to select a group from the highest loaded shard and move - * leadership for that group to the least loaded shard that the group is - * compatible with. - * - * Clearly this is a costly method in terms of runtime complexity. - * Measurements for clusters with several thousand partitions indicate a - * real time execution cost of at most a couple hundred micros. Other - * strategies are sure to improve this as we tackle larger configurations. - * - * Muted nodes are nodes that should be treated as if they have no available - * capacity. So do not move leadership to a muted node, but any leaders on a - * muted node should not be touched in case the mute is temporary. - */ - std::optional recommended_reassignment() override; - std::vector stats() const; double calc_target_load() const; @@ -314,8 +283,6 @@ class pinning_constraint final : public soft_constraint { private: double evaluate_internal(const reassignment& r) override; - std::optional recommended_reassignment() override; - private: std::reference_wrapper _group2topic; preference_index _preference_idx; diff --git a/src/v/cluster/scheduling/leader_balancer_types.h b/src/v/cluster/scheduling/leader_balancer_types.h index d472b374ba8fa..a044fee586199 100644 --- a/src/v/cluster/scheduling/leader_balancer_types.h +++ b/src/v/cluster/scheduling/leader_balancer_types.h @@ -97,8 +97,6 @@ class soft_constraint { auto ret = evaluate_internal(r); return ret; } - - virtual std::optional recommended_reassignment() = 0; }; } // namespace cluster::leader_balancer_types diff --git a/src/v/cluster/tests/leader_balancer_constraints_test.cc b/src/v/cluster/tests/leader_balancer_constraints_test.cc index 914eaed723037..a6ddec80e83d2 100644 --- a/src/v/cluster/tests/leader_balancer_constraints_test.cc +++ b/src/v/cluster/tests/leader_balancer_constraints_test.cc @@ -228,7 +228,6 @@ BOOST_AUTO_TEST_CASE(even_topic_no_error_even_shard_error) { std::move(g_id_to_t_id), shard_index, muted_index); BOOST_REQUIRE(even_shard_con.error() > 0); - BOOST_REQUIRE(even_shard_con.recommended_reassignment().has_value()); } #include "cluster/scheduling/leader_balancer_random.h"