Skip to content

Commit

Permalink
reactor: deprecate at_destroy()
Browse files Browse the repository at this point in the history
at_destroy() tasks are unordered with respect to each other, so
are not very useful. They also consume a scheduling group.

Since they are public, we can't remove them outright, but prepare
by deprecating the public interface. Internal users are converted
to a private interface.
  • Loading branch information
avikivity committed Feb 12, 2025
1 parent 3f8bdf5 commit c66b32a
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 3 deletions.
22 changes: 21 additions & 1 deletion include/seastar/core/reactor.hh
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ size_t scheduling_group_count();

void increase_thrown_exceptions_counter() noexcept;

template <typename Func>
void at_destroy(Func&& func);

}

class io_queue;
Expand Down Expand Up @@ -162,6 +165,8 @@ private:
class io_queue_submission_pollfn;
class syscall_pollfn;
class execution_stage_pollfn;
template <typename Func>
friend void internal::at_destroy(Func&&);
friend class manual_clock;
friend class file_data_source_impl; // for fstream statistics
friend class internal::reactor_stall_sampler;
Expand Down Expand Up @@ -526,10 +531,16 @@ public:
void at_exit(noncopyable_function<future<> ()> func);

template <typename Func>
[[deprecated("Use app_template::run() for orderly shutdown")]]
void at_destroy(Func&& func) {
do_at_destroy(std::forward<Func>(func));
}
private:
template <typename Func>
void do_at_destroy(Func&& func) {
_at_destroy_tasks->_q.push_back(make_task(default_scheduling_group(), std::forward<Func>(func)));
}

public:
task* current_task() const { return _current_task; }
// If a task wants to resume a different task instead of returning control to the reactor,
// it should set _current_task to the resumed task.
Expand Down Expand Up @@ -688,4 +699,13 @@ inline int hrtimer_signal() {

extern logger seastar_logger;

namespace internal {

template <typename Func>
void at_destroy(Func&& func) {
engine().do_at_destroy(std::forward<Func>(func));
}

} // namespace internal

}
2 changes: 1 addition & 1 deletion src/net/net.cc
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ device::receive(std::function<future<> (packet)> next_packet) {
void device::set_local_queue(std::unique_ptr<qp> dev) {
assert(!_queues[this_shard_id()]);
_queues[this_shard_id()] = dev.get();
engine().at_destroy([dev = std::move(dev)] {});
internal::at_destroy([dev = std::move(dev)] {});
}


Expand Down
3 changes: 2 additions & 1 deletion tests/unit/scheduling_group_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,8 @@ SEASTAR_THREAD_TEST_CASE(sg_key_constructor_exception_when_creating_new_key) {
SEASTAR_THREAD_TEST_CASE(sg_create_with_destroy_tasks) {
struct nada{};

engine().at_destroy([] {}); // nothing really
// at_destroy() functionality is deprecated, but test until removed.
internal::at_destroy([] {}); // nothing really

scheduling_group_key_config sg_conf = make_scheduling_group_key_config<nada>();
scheduling_group_key_create(sg_conf).get();
Expand Down

0 comments on commit c66b32a

Please sign in to comment.