Skip to content

Add support to detect and self heal slow broker.#985

Merged
kidkun merged 8 commits into
linkedin:masterfrom
kidkun:slow_broker_2
Dec 7, 2019
Merged

Add support to detect and self heal slow broker.#985
kidkun merged 8 commits into
linkedin:masterfrom
kidkun:slow_broker_2

Conversation

@kidkun
Copy link
Copy Markdown

@kidkun kidkun commented Nov 6, 2019

Fix issue #975

@kidkun kidkun requested a review from efeg November 6, 2019 05:59
@kidkun kidkun mentioned this pull request Nov 6, 2019
Closed
efeg
efeg requested changes Nov 7, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator

@efeg efeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch -- added some comments.

Comment thread ...trol-core/src/main/java/com/linkedin/cruisecontrol/detector/metricanomaly/MetricAnomaly.java
Comment thread ...rc/main/java/com/linkedin/cruisecontrol/detector/metricanomaly/MetricAnomalyFinderUtils.java
Comment thread ...rc/main/java/com/linkedin/cruisecontrol/detector/metricanomaly/MetricAnomalyFinderUtils.java Outdated
Comment thread ...rc/main/java/com/linkedin/cruisecontrol/detector/metricanomaly/MetricAnomalyFinderUtils.java Outdated
Comment thread ...rc/main/java/com/linkedin/cruisecontrol/detector/metricanomaly/MetricAnomalyFinderUtils.java
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/DiskFailureDetector.java Outdated
Comment thread ...se-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/AnomalyDetectorUtils.java Outdated
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/KafkaAnomaly.java Outdated
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/KafkaAnomaly.java
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/KafkaMetricAnomaly.java Outdated
@kidkun kidkun force-pushed the slow_broker_2 branch 2 times, most recently from a0da7ff to 9f83f3f Compare November 8, 2019 00:02
@kidkun kidkun requested a review from efeg November 8, 2019 00:07
@kidkun kidkun force-pushed the slow_broker_2 branch 2 times, most recently from b77e30f to 458b9d0 Compare November 12, 2019 19:39
efeg
efeg reviewed Nov 13, 2019
View reviewed changes
Comment thread ...va/com/linkedin/cruisecontrol/detector/metricanomaly/PercentileMetricAnomalyFinderUtils.java Outdated
Comment thread ...rc/main/java/com/linkedin/cruisecontrol/detector/metricanomaly/MetricAnomalyFinderUtils.java
@kidkun kidkun requested a review from efeg November 18, 2019 21:44
efeg
efeg requested changes Nov 21, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator

@efeg efeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates -- added more comments.

Comment thread ...va/com/linkedin/cruisecontrol/detector/metricanomaly/PercentileMetricAnomalyFinderUtils.java Outdated
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/AnomalyDetector.java Outdated
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/AnomalyDetector.java
@kidkun kidkun requested a review from efeg November 21, 2019 22:30
efeg
efeg requested changes Nov 21, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator

@efeg efeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates -- added more comments.

Comment thread ...-control/src/main/java/com/linkedin/kafka/cruisecontrol/config/KafkaCruiseControlConfig.java
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/AnomalyDetector.java Outdated
Comment thread ...-control/src/main/java/com/linkedin/kafka/cruisecontrol/config/KafkaCruiseControlConfig.java
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/AnomalyDetector.java Outdated
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/AnomalyDetector.java Outdated
@kidkun kidkun requested a review from efeg November 22, 2019 04:47
efeg
efeg requested changes Nov 23, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator

@efeg efeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates -- added some comments.

Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/AnomalyDetector.java Outdated
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/AnomalyDetector.java
_shutdown = true;
}
_anomalies.addFirst(SHUTDOWN_ANOMALY);
_anomalies.add(SHUTDOWN_ANOMALY);
Copy link
Copy Markdown
Collaborator

@efeg efeg Nov 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Can we add this assumption as a comment to make sure that in the future we don't break it by mistake.

Comment thread ...se-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/AnomalyDetectorState.java
Comment thread ...trol-core/src/main/java/com/linkedin/cruisecontrol/detector/metricanomaly/MetricAnomaly.java Outdated
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/KafkaMetricAnomaly.java Outdated
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/SlowBrokerFinder.java Outdated
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/SlowBrokerFinder.java Outdated
Comment thread cruise-control/src/test/java/com/linkedin/kafka/cruisecontrol/detector/AnomalyDetectorTest.java Outdated
Comment thread cruise-control/src/test/java/com/linkedin/kafka/cruisecontrol/detector/AnomalyDetectorTest.java
Comment thread ...ol/src/test/java/com/linkedin/kafka/cruisecontrol/detector/KafkaMetricAnomalyFinderTest.java Outdated
@kidkun kidkun requested a review from efeg November 26, 2019 00:49
efeg
efeg approved these changes Dec 5, 2019
View reviewed changes
Copy link
Copy Markdown
Collaborator

@efeg efeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kidkun Thanks for the updates -- added some comments we can merge after fix/release test.

Comment thread cruise-control-core/src/main/java/com/linkedin/cruisecontrol/detector/AnomalyType.java
Comment thread ...in/java/com/linkedin/cruisecontrol/detector/metricanomaly/PercentileMetricAnomalyFinder.java
Comment thread ...-control/src/main/java/com/linkedin/kafka/cruisecontrol/config/KafkaCruiseControlConfig.java Outdated
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/SlowBrokerFinder.java
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/SlowBrokerFinder.java Outdated
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/SlowBrokerFinder.java Outdated
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/SlowBrokerFinder.java
Comment thread cruise-control/src/main/java/com/linkedin/kafka/cruisecontrol/detector/SlowBrokerFinder.java Outdated
Comment thread cruise-control/src/test/java/com/linkedin/kafka/cruisecontrol/detector/AnomalyDetectorTest.java Outdated
mockDetectorScheduler.shutdown();
EasyMock.expectLastCall().andDelegateTo(executorService);
EasyMock.expect(mockDetectorScheduler.awaitTermination(MOCK_ANOMALY_DETECTION_INTERVAL_MS, TimeUnit.MILLISECONDS)).andDelegateTo(executorService);
EasyMock.expect(mockDetectorScheduler.awaitTermination(5000, TimeUnit.MILLISECONDS)).andDelegateTo(executorService);
Copy link
Copy Markdown
Collaborator

@efeg efeg Dec 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Can we use a constant rather than hardcoding the value?

@kidkun kidkun merged commit 3b0ea9c into linkedin:master Dec 7, 2019
@kidkun kidkun mentioned this pull request Dec 13, 2019
Closed
annaleeli pushed a commit to TeamCtrlC/cruise-control that referenced this pull request Dec 17, 2019
@annaleeli
Add support to detect and self heal slow broker. (linkedin#985)
95a3695
@kidkun kidkun mentioned this pull request Jan 9, 2020
Closed
efeg pushed a commit to efeg/cruise-control that referenced this pull request Nov 10, 2020
Ensure that topology distribution goals compute balance constraints p…
c39b12d 
…roperly.

* Ensure that RackAwareDistributionGoal, TopicReplicaDistributionGoal, ReplicaDistributionGoal, LeaderReplicaDistributionGoal do not include brokers excluded for replica moves while computing balance constraints.
* Fix incorect numOfflineTopicReplicasInB1 within rebalanceByMovingReplicasIn of TopicReplicaDistributionGoal.
* Update the following dead configs, which are not supported after in cruisecontrol.properties in linkedin#985
  broker.failure.exclude.recently.demoted.brokers=true
  broker.failure.exclude.recently.removed.brokers=true
  goal.violation.exclude.recently.demoted.brokers=true
  goal.violation.exclude.recently.removed.brokers=true
efeg pushed a commit to efeg/cruise-control that referenced this pull request Nov 10, 2020
Ensure that topology distribution goals compute balance constraints p…
b3b866f 
…roperly.

* Ensure that RackAwareDistributionGoal, TopicReplicaDistributionGoal, ReplicaDistributionGoal, LeaderReplicaDistributionGoal do not include brokers excluded for replica moves while computing balance constraints.
* Fix incorect numOfflineTopicReplicasInB1 within rebalanceByMovingReplicasIn of TopicReplicaDistributionGoal.
* Update the following dead configs, which are not supported after in cruisecontrol.properties in linkedin#985
  broker.failure.exclude.recently.demoted.brokers=true
  broker.failure.exclude.recently.removed.brokers=true
  goal.violation.exclude.recently.demoted.brokers=true
  goal.violation.exclude.recently.removed.brokers=true
efeg pushed a commit to efeg/cruise-control that referenced this pull request Nov 10, 2020
Ensure that topology distribution goals compute balance constraints p…
94aafe2 
…roperly.

* Ensure that RackAwareDistributionGoal, TopicReplicaDistributionGoal, ReplicaDistributionGoal, LeaderReplicaDistributionGoal do not include brokers excluded for replica moves while computing balance constraints.
* Fix incorect numOfflineTopicReplicasInB1 within rebalanceByMovingReplicasIn of TopicReplicaDistributionGoal.
* Update the following dead configs, which are not supported after in cruisecontrol.properties in linkedin#985
  broker.failure.exclude.recently.demoted.brokers=true
  broker.failure.exclude.recently.removed.brokers=true
  goal.violation.exclude.recently.demoted.brokers=true
  goal.violation.exclude.recently.removed.brokers=true
@efeg efeg mentioned this pull request Nov 10, 2020
Merged
efeg added a commit that referenced this pull request Nov 11, 2020
@efeg
Ensure that topology distribution goals compute balance constraints p…
0e273c0 
…roperly (#1383)

* Ensure that RackAwareDistributionGoal, TopicReplicaDistributionGoal, ReplicaDistributionGoal, LeaderReplicaDistributionGoal do not include brokers excluded for replica moves while computing balance constraints.
* Fix incorect numOfflineTopicReplicasInB1 within rebalanceByMovingReplicasIn of TopicReplicaDistributionGoal.
* Update the following dead configs, which are not supported after in cruisecontrol.properties in #985
  broker.failure.exclude.recently.demoted.brokers=true
  broker.failure.exclude.recently.removed.brokers=true
  goal.violation.exclude.recently.demoted.brokers=true
  goal.violation.exclude.recently.removed.brokers=true
efeg added a commit to efeg/cruise-control that referenced this pull request Nov 11, 2020
@efeg
Ensure that topology distribution goals compute balance constraints p…
eac857b 
…roperly (linkedin#1383)

* Ensure that RackAwareDistributionGoal, TopicReplicaDistributionGoal, ReplicaDistributionGoal, LeaderReplicaDistributionGoal do not include brokers excluded for replica moves while computing balance constraints.
* Fix incorect numOfflineTopicReplicasInB1 within rebalanceByMovingReplicasIn of TopicReplicaDistributionGoal.
* Update the following dead configs, which are not supported after in cruisecontrol.properties in linkedin#985
  broker.failure.exclude.recently.demoted.brokers=true
  broker.failure.exclude.recently.removed.brokers=true
  goal.violation.exclude.recently.demoted.brokers=true
  goal.violation.exclude.recently.removed.brokers=true
efeg added a commit to efeg/cruise-control that referenced this pull request Nov 11, 2020
@efeg
Ensure that topology distribution goals compute balance constraints p…
d82de5a 
…roperly (linkedin#1383)

* Ensure that RackAwareDistributionGoal, TopicReplicaDistributionGoal, ReplicaDistributionGoal, LeaderReplicaDistributionGoal do not include brokers excluded for replica moves while computing balance constraints.
* Fix incorect numOfflineTopicReplicasInB1 within rebalanceByMovingReplicasIn of TopicReplicaDistributionGoal.
* Update the following dead configs, which are not supported after in cruisecontrol.properties in linkedin#985
  broker.failure.exclude.recently.demoted.brokers=true
  broker.failure.exclude.recently.removed.brokers=true
  goal.violation.exclude.recently.demoted.brokers=true
  goal.violation.exclude.recently.removed.brokers=true
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@efeg efeg efeg approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kidkun @efeg