-
Notifications
You must be signed in to change notification settings - Fork 553
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
transform: create smp and scheduling groups #16114
Conversation
Add a scheduling group parameter for all the work executed on the queue. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
65304bc
to
7d17b82
Compare
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
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>
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
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>
Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>
7d17b82
to
99a3ece
Compare
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/43815#018d1472-5af1-48b9-90a0-a32b262d8698 |
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.
I'm not overly familiar with either the motivation for this change or the SMP group bits, but generally this lgtm. Might want to wait for Noah to sign off.
src/v/resource_mgmt/smp_groups.h
Outdated
return destroy_smp_service_group(*_kafka) | ||
.then([this] { return destroy_smp_service_group(*_raft); }) | ||
.then([this] { return destroy_smp_service_group(*_cluster); }) | ||
.then([this] { return destroy_smp_service_group(*_proxy); }); | ||
co_await destroy_smp_service_group(*_kafka); | ||
co_await destroy_smp_service_group(*_raft); | ||
co_await destroy_smp_service_group(*_cluster); | ||
co_await destroy_smp_service_group(*_proxy); | ||
co_await destroy_smp_service_group(*_transform); |
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.
question: Is this just for style points, or is there a specific reason to prefer coro style over continuation style here?
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.
Totally style points. coro4lyfe
I'm going to merge this so that I can get it into an install pack (via nightly) for our cloud for my testing. @dotnwat if you want to take a look I am happy to send another round of patches to address any feedback. This is mostly just plumbing around some scheduling groups and smp groups. |
/backport v23.3.x |
Failed to create a backport PR to v23.3.x branch. I tried:
|
Introduce SMP and CPU scheduling groups for transforms.
The main motivation for this is to allow for internal metrics on CPU usage for the transform subsystem. While doing this change, I also see that we generally also have smp groups too, so I created one for the internal data path, where there are a few cross shard calls to produce.
Backports Required
Release Notes
Improvements