Add a ReplicaMovementStrategy that prioritizes (At/Under)MinISR partitions with offline replicas #1559
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
…tions with offline replicas
zornhsu
reviewed
May 26, 2021
...a/com/linkedin/kafka/cruisecontrol/executor/strategy/PostponeUrpReplicaMovementStrategy.java
Outdated
Show resolved
Hide resolved
zornhsu
reviewed
May 27, 2021
...ontrol/src/test/java/com/linkedin/kafka/cruisecontrol/executor/ExecutionTaskPlannerTest.java
Outdated
Show resolved
Hide resolved
zornhsu
approved these changes
May 27, 2021
| partitions.add(generatePartitionInfo(_partitionMovement4, false)); | ||
|
|
||
| Cluster expectedCluster = new Cluster(null, _expectedNodes, partitions, Collections.<String>emptySet(), Collections.<String>emptySet()); | ||
| // This ensures that the _partitionMovement1 and _partitionMovement3 are AtMinISR, while the other partitions are not. |
There was a problem hiding this comment.
Maybe we want to add a test for UnderMinISR is prioritized before AtMinISR partitions?
I am a little hesitant because I don't see a quick way to add this without involving 3 replicas in the replica set.
efeg
added a commit
to efeg/cruise-control
that referenced
this pull request
May 27, 2021
…tions with offline replicas (linkedin#1559)
efeg
added a commit
to efeg/cruise-control
that referenced
this pull request
May 27, 2021
…tions with offline replicas (linkedin#1559)
8 tasks
kaitlynp1206
pushed a commit
to kaitlynp1206/cruise-control
that referenced
this pull request
Jun 10, 2021
…tions with offline replicas (linkedin#1559)
zornhsu
pushed a commit
to zornhsu/cruise-control
that referenced
this pull request
Jun 21, 2021
…tions with offline replicas (linkedin#1559)
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.
This PR resolves #1558.
TopicMinIsrCache#minIsrWithTimeByTopic()return a snapshot within asynchronizedblock to ensure proper synchronization.StrategyOptionsto move any options intended to be used during application of a replica movement strategy. The existing code was limited toClusteras the sole option, and didn't allow adding new options to a replica movement strategy without an API change inReplicaMovementStrategyinterface. WithStrategyOptionsnew options can be added w/o changing the API.BaseReplicaMovementStrategyto the default replica movement task strategy while creating anExecutionTaskPlannerobject. The redundancy was due to lack of check for the configureddefault.replica.movement.strategies. If the config had a list of entries containingBaseReplicaMovementStrategy(i.e. this is the default config), then the existing was still chaining an extraBaseReplicaMovementStrategy, causing inefficiency for no reason.ExecutionUtils#populateMinIsrStatefunction to control whether to retrieve (At/Under)MinISR partitions each containing at least an offline replica, or to retrieve (At/Under)MinISR partitions without any offline replicas.PrioritizeMinIsrWithOfflineReplicasStrategystrategy, then minISR-based concurrency adjustment is enabled. Otherwise, thePrioritizeMinIsrWithOfflineReplicasStrategystrategy is just a no-op.default.replica.movement.strategiesfromAnalyzer Configurations-- it is already documented in the correct section (i.e.Executor Configurations)