-
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
Added support for changing tx topic properties #16797
Added support for changing tx topic properties #16797
Conversation
Extracted topic manager topic creation logic to separate component. The component is going to be responsible for managing the tx_manager topic and its properties. Signed-off-by: Michal Maslanka <michal@redpanda.com>
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45491#018df4cc-0f80-4199-a9f0-eba8d8382020 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45684#018e0e04-9075-4201-a4ad-73eb8d102443 |
*/ | ||
auto u = _reconciliation_mutex.get_units(); | ||
|
||
vlog(txlog.trace, "Reconciling tx manager topic properties"); |
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.
should this happen only on one node? (controller leader?), otherwise we will as many updates as # nodes in the cluster?
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 was thinking about this, the topic update property is idempotent. Those redundant topic updates will not negatively impact the cluster. If we would want to do it only on controller leader we would need additional complexity to check for the leadership changes. Also this operation will be pretty rare.
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.
Right, a hack is probably to add some jitter before checking for reconciliation (like voter priority) but thats too much for this code I guess :-)
Added possibility to change the `__kafka_internal/tx` topic properties. The topic properties are reconciled with `cluster::tx_topic_manager`. Previously changing configuration properties related with `tx` topic had no effect despite being a properties that did not require restart. Signed-off-by: Michal Maslanka <michal@redpanda.com>
dfacc89
to
022b16f
Compare
new failures in https://buildkite.com/redpanda/redpanda/builds/45610#018e0956-025b-4287-b363-9241b0a2ae2c:
|
*/ | ||
auto u = _reconciliation_mutex.get_units(); | ||
|
||
vlog(txlog.trace, "Reconciling tx manager topic properties"); |
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.
Right, a hack is probably to add some jitter before checking for reconciliation (like voter priority) but thats too much for this code I guess :-)
/ci-repeat 1 |
/backport v23.3.x |
/backport v23.2.x |
Failed to create a backport PR to v23.2.x branch. I tried:
|
Failed to create a backport PR to v23.3.x branch. I tried:
|
Added possibility to change the
__kafka_internal/tx
topic properties.The topic properties are reconciled with
cluster::tx_topic_manager
.Previously changing configuration properties related with
tx
topichad no effect despite being a properties that did not require restart.
Fixes: #16795
Backports Required
Release Notes
Improvements