Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v23.1.x] c/members_backend: fixed calculation of unevenness error #9763

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/v/cluster/members_backend.cc
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ void members_backend::reallocations_for_even_partition_count(
members_backend::update_meta& meta, partition_allocation_domain domain) {
size_t prev_reallocations_count = meta.partition_reallocations.size();
calculate_reallocations_batch(meta, domain);
auto current_error = calculate_unevenness_error(domain);
auto current_error = calculate_unevenness_error(meta, domain);
auto [it, _] = meta.last_unevenness_error.try_emplace(domain, 1.0);
const auto min_improvement
= std::max<size_t>(
Expand Down Expand Up @@ -452,14 +452,28 @@ size_t members_backend::calculate_total_replicas(
**/
members_backend::unevenness_error_info
members_backend::calculate_unevenness_error(
partition_allocation_domain domain) const {
const update_meta& update, partition_allocation_domain domain) const {
static const std::vector<partition_allocation_domain> domains{
partition_allocation_domains::consumer_offsets,
partition_allocation_domains::common};

const auto node_cnt = _allocator.local().state().available_nodes();

const auto node_replicas = calculate_replicas_per_node(domain);
auto node_replicas = calculate_replicas_per_node(domain);
/**
* adjust per node replicas with the replicas that are going to be removed
* from the node after successful reallocation
*/
for (const auto& r : update.partition_reallocations) {
if (r.allocation_units) {
for (const auto& to_remove : r.replicas_to_remove) {
auto it = node_replicas.find(to_remove);
if (it != node_replicas.end()) {
it->second.allocated_replicas--;
}
}
}
}
const auto total_replicas = calculate_total_replicas(node_replicas);

if (total_replicas == 0) {
Expand Down
4 changes: 2 additions & 2 deletions src/v/cluster/members_backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ class members_backend {
absl::node_hash_map<model::node_id, node_replicas>
calculate_replicas_per_node(partition_allocation_domain) const;

unevenness_error_info
calculate_unevenness_error(partition_allocation_domain) const;
unevenness_error_info calculate_unevenness_error(
const update_meta&, partition_allocation_domain) const;
bool should_stop_rebalancing_update(const update_meta&) const;

static size_t calculate_total_replicas(const node_replicas_map_t&);
Expand Down