-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Description
Similar to #2992 but caused by destroying and creating a scheduling group instead of renaming.
Can be reproduced with the following test:
SEASTAR_THREAD_TEST_CASE(sg_remove_then_recreate_with_the_same_name_but_different_index) {
auto do_nothing = [] {};
inheriting_concrete_execution_stage<void> stage{"sg_remove_and_recreate_with_different_name_stage", do_nothing};
std::vector<scheduling_group> sgs;
sgs.push_back(create_scheduling_group("foo", 100).get());
with_scheduling_group(sgs[0], [&] { return stage(); }).get();
auto old_sg1_idx = internal::scheduling_group_index(sgs[0]);
destroy_scheduling_group(sgs[0]).get();
sgs[0] = create_scheduling_group("bar", 100).get();
BOOST_CHECK_NO_THROW({
sgs.push_back(create_scheduling_group("foo", 100).get());
with_scheduling_group(sgs[1], [&] { return stage(); }).get();
});
BOOST_REQUIRE_NE(old_sg1_idx, internal::scheduling_group_index(sgs[1]));
for (scheduling_group sg : sgs) {
destroy_scheduling_group(sg).get();
}
}
Fixing create_scheduling_group
and destroy_scheduling_group
seems non-trivial, because the underlying problem is that execution_stages
are kept in inheriting_concrete_execution_stage
. I think that the real solution should remove the destroyed scheduling groups from _stage_for_group
in inheriting_concrete_execution_stage
, but on the other hand, I don't like the idea of modifying inheriting_concrete_execution_stage
from execution_stage_manager
.
At least the following two additional test cases should be added and pass. However, please note that depending on the implementation of the fix, there might be also more nuanced race conditions between create_scheduling_group
and destroy_scheduling_group
(maybe a semaphore should be added to make sure those methods executions are mutually exclusive?).
SEASTAR_THREAD_TEST_CASE(sg_remove_then_recreate_with_the_same_index) {
auto do_nothing = [] {};
inheriting_concrete_execution_stage<void> stage{"sg_remove_then_recreate_with_the_same_index_stage", do_nothing};
auto sg = create_scheduling_group("foo", 100).get();
with_scheduling_group(sg, [&] { return stage(); }).get();
auto old_sg1_idx = internal::scheduling_group_index(sg);
destroy_scheduling_group(sg).get();
BOOST_CHECK_NO_THROW({
sg = create_scheduling_group("bar", 100).get();
with_scheduling_group(sg, [&] { return stage(); }).get();
});
BOOST_REQUIRE_EQUAL(old_sg1_idx, internal::scheduling_group_index(sg));
destroy_scheduling_group(sg).get();
}
SEASTAR_THREAD_TEST_CASE(sg_remove_then_recreate_with_the_same_name) {
auto do_nothing = [] {};
inheriting_concrete_execution_stage<void> stage{"sg_remove_and_recreate_with_the_same_name_stage", do_nothing};
auto sg = create_scheduling_group("foo", 100).get();
with_scheduling_group(sg, [&] { return stage(); }).get();
auto old_sg1_idx = internal::scheduling_group_index(sg);
destroy_scheduling_group(sg).get();
BOOST_CHECK_NO_THROW({
sg = create_scheduling_group("foo", 100).get();
with_scheduling_group(sg, [&] { return stage(); }).get();
});
BOOST_REQUIRE_EQUAL(old_sg1_idx, internal::scheduling_group_index(sg));
destroy_scheduling_group(sg).get();
}