-
Notifications
You must be signed in to change notification settings - Fork 593
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
Fixes and tests for altering topic properties #947
Merged
mmaslankaprv
merged 11 commits into
redpanda-data:dev
from
mmaslankaprv:alter-config-tests
Mar 24, 2021
Merged
Fixes and tests for altering topic properties #947
mmaslankaprv
merged 11 commits into
redpanda-data:dev
from
mmaslankaprv:alter-config-tests
Mar 24, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mmaslankaprv
force-pushed
the
alter-config-tests
branch
from
March 24, 2021 16:07
43e6501
to
8b840ab
Compare
emaxerrno
reviewed
Mar 24, 2021
emaxerrno
reviewed
Mar 24, 2021
emaxerrno
reviewed
Mar 24, 2021
emaxerrno
reviewed
Mar 24, 2021
emaxerrno
reviewed
Mar 24, 2021
emaxerrno
reviewed
Mar 24, 2021
emaxerrno
reviewed
Mar 24, 2021
emaxerrno
reviewed
Mar 24, 2021
emaxerrno
reviewed
Mar 24, 2021
emaxerrno
reviewed
Mar 24, 2021
emaxerrno
previously approved these changes
Mar 24, 2021
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.
LGTM;
Please do minor nits and add the salient key comments on our live code-review session.
Exposed `insert_linearizable_barrier` from `raft::state_machine`. Signed-off-by: Michal Maslanka <michal@vectorized.io>
Added graceful handling of lexical cast exception thrown when parsing topic configuration property values. Signed-off-by: Michal Maslanka <michal@vectorized.io>
Fixed passing error code returned by `topics_frontend` to alter configuration responses. Signed-off-by: Michal Maslanka <michal@vectorized.io>
Added `update_topic_properties` RPC method. This is requires since both `AlterConfigs` and `IncrementalAlterConfigs` requests can be called on any broker. Since we replicate topic properties update to controller it is required to forward the update topic properties to leader controller. Signed-off-by: Michal Maslanka <michal@vectorized.io>
Implemented dispatching topic properties update to leader controller. Both `AlterConfigs` and `IncrementalAlterConfigs` can be executed on any of the nodes. We have to forward topic properties update request to current leader controller. Signed-off-by: Michal Maslanka <michal@vectorized.io>
Leveraging raft linearizable barrier to speed up applying commands on followers. In Kafka Metadata are eventually consistent. Our metadata cache and controller design leverages that fact (we do not require linearizable reads when requesting metadata). In raft followers do not know about committed index updates until then receive heartbeat following commit index update on the leader. In redpanda heartbeats are by default send every 150ms. This way follower metadata is updated with a delay of ~150ms. The delay of metadata propagation together with the fact that clients requests metadata update from arbitrary broker in the cluster may lead to situation in which clients will receive stale metadata. This is correct from the protocol perspective but it may be misleading and may cause additional delays when client backoff timeout is triggered. In order to reduce metadata propagation lag we will use `raft::insert_linearizable_barrier` method. This way we will trigger round of heartbeats immediately after data were replicated. This way we will reduce delay of metadata propagation. Signed-off-by: Michal Maslanka <michal@vectorized.io>
We have to use standard naming of topic properties. Replaced custom string with predefined constants of all topic property names. Signed-off-by: Michal Maslanka <michal@vectorized.io>
We have to make sure that all our types are presented to clients in Kafka way. Fixed printing out `model::timestamp_type`. Signed-off-by: Michal Maslanka <michal@vectorized.io>
Signed-off-by: Michal Maslanka <michal@vectorized.io>
Signed-off-by: Michal Maslanka <michal@vectorized.io>
Signed-off-by: Michal Maslanka <michal@vectorized.io>
mmaslankaprv
force-pushed
the
alter-config-tests
branch
from
March 24, 2021 19:14
8b840ab
to
a58870b
Compare
emaxerrno
approved these changes
Mar 24, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cover letter
This PR fixes handling of
AlterConfigs
andIncrementalAlterConfigs
by allowing those requests to be called from any of the brokers. Additionally it minimizes duration between leader and follower state updates.This PR introduces
ducktape
tests for altering topic configuration.Fixes #282 #935 #689
Release notes
Full support for altering topic configurations
Release note: [1-2 sentences of what this PR changes]