Skip to content

Commit

Permalink
c/partition_allocator: make _highest_group updates monotonic
Browse files Browse the repository at this point in the history
Previously, in case of failed allocation of partitions for new topics,
we tried to revert _highest_group value as well. When a large number of
topics were allocated in a single request concurrently (e.g. when MirrorMaker
creates required topics on the destination cluster) and some of these
requests bump into partition allocation limits, this could lead to
duplicate group_ids.

As there is no reason to try to reuse group_ids, simply remove the code
moving _highest_group backwards.

Fixes #12610

(cherry picked from commit 3f96297)
  • Loading branch information
ztlpn authored and vbotbuildovich committed Oct 24, 2023
1 parent 0c0001a commit 4edaf7b
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 4 deletions.
2 changes: 0 additions & 2 deletions src/v/cluster/scheduling/allocation_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ void allocation_state::rollback(
remove_allocation(bs, domain);
remove_final_count(bs, domain);
}
// rollback for each assignment as the groups are distinct
_highest_group = raft::group_id(_highest_group() - 1);
}
}

Expand Down
1 change: 0 additions & 1 deletion src/v/cluster/scheduling/partition_allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ class partition_allocator {
ss::future<> apply_snapshot(const controller_snapshot&);

private:
// reverts not only allocations but group_ids as well
class intermediate_allocation {
public:
intermediate_allocation(
Expand Down
2 changes: 1 addition & 1 deletion src/v/cluster/tests/partition_allocator_tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ FIXTURE_TEST(partial_assignment, partition_allocator_fixture) {

BOOST_REQUIRE_EQUAL(3, max_capacity());
BOOST_REQUIRE_EQUAL(
allocator.state().last_group_id()(), max_partitions_in_cluster - 1);
allocator.state().last_group_id()(), max_partitions_in_cluster);
}
FIXTURE_TEST(max_deallocation, partition_allocator_fixture) {
register_node(0, 3);
Expand Down

0 comments on commit 4edaf7b

Please sign in to comment.