-
Notifications
You must be signed in to change notification settings - Fork 555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[v23.3.x] transform: create smp and scheduling groups #16139
[v23.3.x] transform: create smp and scheduling groups #16139
Conversation
Pass RPC requests by value, as we start to write some handlers using coroutines, it can be a footgun to access have a suspension point, then the request is destroyed. An example that motivated this: ```c++ ss::future<reply> service::handle(request&& r, rpc::streaming_context&) { co_await ss::coroutine::switch_to(get_scheduling_group()); co_return co_await handle_request(std::move(r)); } ``` In this case `r` is destroyed because the coroutine does not own the value, and handle request would have a stack-use-after-return issue. Now that we pass by value, we don't have to remember to move the value into the current method. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit 5becd89)
Add a scheduling group parameter for all the work executed on the queue. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit 566e2b1)
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit 4b3251b)
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit 6928acc)
When RPCs come in for transform requests, ensure that they are running on the transform scheduling groups. Node local service requests are assumed to be already running on the transform scheduling group, which is mostly true except for deploys. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit 895af5f)
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit d018c59)
The transform/rpc subsystem makes cross shard calls when accessing the correct partition for a transform, use a `smp_group` to manage this, like other subsystems. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit cb310c5)
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com> (cherry picked from commit 99a3ece)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I'm clear, the cherry pick was otherwise clean, right? As in, you didn't have to change the contents of any commits from #16114?
Correct the bot only failed because of conflicts in transform/rpc/service.cc because git incorrectly associated the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backport looks good
Backport of PR #16114
To get a clean backport I also cherry picked #15971 into this backport