-
Notifications
You must be signed in to change notification settings - Fork 589
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
Disabled partitions implementation #15141
Disabled partitions implementation #15141
Conversation
@@ -1603,6 +1607,12 @@ ss::future<add_paritions_tx_reply> tx_gateway_frontend::do_add_partition_to_tx( | |||
res_partition.partition_index = req_partition; | |||
res_partition.error_code = tx_errc::none; | |||
res_topic.results.push_back(res_partition); | |||
} else if ( | |||
disabled_set && disabled_set->is_disabled(req_partition)) { |
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.
nit: maybe we can add a method bool is_disabled(const model::ntp&) const
in metadata_cache
as i see this patter repeating all over the PR
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.
It is a small optimization - we query the disabled set for the topic once and then use it, instead of looking up the topic disabled set for each partition.
We also need to disable reads and writes in the transform subsystem. |
@rockwotj Can you give me some code pointers? Where should I insert those checks? |
83ff0f9
to
db57397
Compare
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/41814#018c1366-e570-4c0b-ae04-d9def320f523 |
db57397
to
b1449fe
Compare
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 only reviewed the last two commits, as I'm missing some context on the other commits here, but the transform changes LGTM.
controller_backend
Backports Required
Release Notes
Features