reactor: Deprecate add_task/add_urgent_task/add_high_priority_task#3341
reactor: Deprecate add_task/add_urgent_task/add_high_priority_task#3341xemul wants to merge 1 commit into
Conversation
These methods may have out-of-tree users and cannot be simply removed, but they should not be part of the public reactor API going forward. Deprecate them in favor of the seastar::schedule() and seastar::schedule_urgent() free functions. Internal call sites in reactor.cc are wrapped with diagnostic pragmas to suppress the deprecation warnings. add_high_priority_task() is additionally exposed via reactor::test::add_high_priority_task() to prepare for the eventual privatization of the method, with the pragma applied there so test code remains clean. Signed-off-by: Pavel Emelyanov <xemul@scylladb.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@scylladb/seastar-maint , please review |
| #pragma GCC diagnostic ignored "-Wdeprecated-declarations" | ||
| add_high_priority_task(make_task(default_scheduling_group(), [fn = std::move(reclaim_fn)] { | ||
| fn(); | ||
| })); |
There was a problem hiding this comment.
Seastar code shouldn't use deprecated declarations.
Once we deprecate something, from Seastar's persepective, it's ready to be removed at any time.
There was a problem hiding this comment.
The goal is not to remove those methods, but move them to private eventually and "undeprecated" them back.
To follow the suggestion not to use deprecated symbols, there must appear add_high_priority_task_private() method, the add_high_priority_task() (now deprecated) should call it, and after deprecation quiscent timeout we'll need to rename add_...private() back into add...() one. Does it worth it?
There was a problem hiding this comment.
@xemul maybe add corresponding seastar::internal entry points that would be called from the public entry points that are deprecated in this path, and change the library calls to use the internal entry points also in this patch.
There was a problem hiding this comment.
And mark those entry points as friends? I'll try to re-structure it somehow else
There was a problem hiding this comment.
Seems fine to me to delay that restructure/decision to the removal point?
4 participants
These methods may have out-of-tree users and cannot be simply removed, but they should not be part of the public reactor API going forward. Deprecate them in favor of the seastar::schedule() and seastar::schedule_urgent() free functions.
Internal call sites in reactor.cc are wrapped with diagnostic pragmas to suppress the deprecation warnings. add_high_priority_task() is additionally exposed via reactor::test::add_high_priority_task() to prepare for the eventual privatization of the method, with the pragma applied there so test code remains clean.