[CORE-7750] Creating topic with huge number of partitions leads to segfault #24135
Conversation
IoannisRP
force-pushed
the
ik-partitions-segfault
branch
from
afa961d to
2768d61
Compare
|
@rockwotj good points. I'll address them and i'll add the |
|
/dt |
IoannisRP
force-pushed
the
ik-partitions-segfault
branch
2 times, most recently
from
26e44b1 to
155d046
Compare
IoannisRP
changed the title
|
the below tests from https://buildkite.com/redpanda/redpanda/builds/58192#01933fcb-6e85-465d-9b8e-7ea931b6e484 have failed and will be retried |
|
seems like this is primarily a replication team code area. added reviewers from that team |
| @@ -802,8 +809,12 @@ ss::future<topic_result> topics_frontend::do_create_topic( | |||
| partition_allocator::shard, | |||
| [assignable_config, topic_aware = _partition_autobalancing_topic_aware()]( | |||
| partition_allocator& al) { | |||
| if (assignable_config.has_custom_assignment()) { | |||
| return al.allocate( | |||
| make_custom_allocation_request(assignable_config, topic_aware)); | |||
There was a problem hiding this comment.
Is this path (or through do_increase_replication_factor) not susceptible to the problem?
There was a problem hiding this comment.
Mainly, we cannot be lazy about these paths, as the request has per-partition requirements.
Maybe we could improve this by delaying the transform, but I don't think there would be any obvious benefits.
The raw transformation takes an integer and instantiates an array of that size.
This path translates an array into another array.
While it is very easy to request for a "billion" new partitions, it is not as easy to request for a billion custom assignments. If someone were to indeed ask for that, the point of failure would probably be around the parsing of the request and the population of this array. It would have to be handled there.
move static make_allocation_request helper functions into an anonymous namespace
Create a new partition allocation path. The new path doesn't explicitly instantiate a full allocation_request upfront, as the size of this request scales with the number of requested partitions. This allows to check for system limitations and reject a request earlier, without the need to create a potentially resource-intense allocation_request.
Where applicable, use the simple_allocation_request interface.
IoannisRP
force-pushed
the
ik-partitions-segfault
branch
from
155d046 to
16a95fb
Compare
|
| @@ -341,6 +381,27 @@ FIXTURE_TEST(allocation_units_test, partition_allocator_fixture) { | |||
| // we do not decrement the highest raft group | |||
| BOOST_REQUIRE_EQUAL(allocator().state().last_group_id()(), 10); | |||
| } | |||
| FIXTURE_TEST(allocation_units_test_raw_req, partition_allocator_fixture) { | |||
|
/backport v24.3.x |
|
/backport v24.2.x |
|
Failed to create a backport PR to v24.2.x branch. I tried: |
Fixes: CORE-7750
The size of the
allocation_requestcreated as part of the partition allocation scales linearly with the number of partitions needed to be created. Currently, the check to see if the request can be accepted happens on theallocation_requestobject. When the number of requested partitions is too big, we run out of memory while creating this object.The fix is to be lazy on the creation of the
allocation_request, where possible, and do the first step of the validation before we create the full object.Backports Required
Release Notes
Bug Fixes