From 4edaf7b22d02b998b29157bf358575008cc4e578 Mon Sep 17 00:00:00 2001 From: Alexey Zatelepin Date: Mon, 23 Oct 2023 21:43:26 +0200 Subject: [PATCH] c/partition_allocator: make _highest_group updates monotonic 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 https://github.com/redpanda-data/redpanda/issues/12610 (cherry picked from commit 3f9629708e6cac19510d9ae19328a56e47f119eb) --- src/v/cluster/scheduling/allocation_state.cc | 2 -- src/v/cluster/scheduling/partition_allocator.h | 1 - src/v/cluster/tests/partition_allocator_tests.cc | 2 +- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/v/cluster/scheduling/allocation_state.cc b/src/v/cluster/scheduling/allocation_state.cc index c44d9d175b303..e0819af79927f 100644 --- a/src/v/cluster/scheduling/allocation_state.cc +++ b/src/v/cluster/scheduling/allocation_state.cc @@ -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); } } diff --git a/src/v/cluster/scheduling/partition_allocator.h b/src/v/cluster/scheduling/partition_allocator.h index 1eb975486efb7..1ad4102285cd6 100644 --- a/src/v/cluster/scheduling/partition_allocator.h +++ b/src/v/cluster/scheduling/partition_allocator.h @@ -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( diff --git a/src/v/cluster/tests/partition_allocator_tests.cc b/src/v/cluster/tests/partition_allocator_tests.cc index d6916d6da57a9..5d62d173bb4e9 100644 --- a/src/v/cluster/tests/partition_allocator_tests.cc +++ b/src/v/cluster/tests/partition_allocator_tests.cc @@ -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);