This repository has been archived by the owner on Aug 2, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 19
Polarize actions based on impact vectors #332
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
8ff2566
Polarize actions based on impact vectors
ktkrg 5e58403
Merge from master
ktkrg 7649d14
Merge from master
ktkrg 95f958d
Fix import order
ktkrg 490d7f2
Use InstanceDetails.Id and Ip instead of String
ktkrg 3dcd77c
Address PR comments
ktkrg 19d6f2c
Refactor collator constructor to not take evalInterval
ktkrg 9ce128c
Merge from master
ktkrg 002179c
Handle multi node impacting actions in the collator
ktkrg 7d9484a
Merge from master
ktkrg 8919220
Add missing javadoc
ktkrg a4fbb59
Address review comments
ktkrg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
68 changes: 0 additions & 68 deletions
68
.../amazon/opendistro/elasticsearch/performanceanalyzer/decisionmaker/deciders/Collator.java
This file was deleted.
Oops, something went wrong.
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
192 changes: 192 additions & 0 deletions
192
...pendistro/elasticsearch/performanceanalyzer/decisionmaker/deciders/collator/Collator.java
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,192 @@ | ||
/* | ||
* Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"). | ||
* You may not use this file except in compliance with the License. | ||
* A copy of the License is located at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* or in the "license" file accompanying this file. This file is distributed | ||
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either | ||
* express or implied. See the License for the specific language governing | ||
* permissions and limitations under the License. | ||
*/ | ||
|
||
package com.amazon.opendistro.elasticsearch.performanceanalyzer.decisionmaker.deciders.collator; | ||
|
||
import com.amazon.opendistro.elasticsearch.performanceanalyzer.decisionmaker.actions.Action; | ||
import com.amazon.opendistro.elasticsearch.performanceanalyzer.decisionmaker.actions.ImpactVector; | ||
import com.amazon.opendistro.elasticsearch.performanceanalyzer.decisionmaker.actions.ImpactVector.Impact; | ||
import com.amazon.opendistro.elasticsearch.performanceanalyzer.decisionmaker.deciders.Decider; | ||
import com.amazon.opendistro.elasticsearch.performanceanalyzer.decisionmaker.deciders.Decision; | ||
import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.api.AnalysisGraph; | ||
import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.store.rca.cluster.NodeKey; | ||
import com.google.common.annotations.VisibleForTesting; | ||
import java.io.Serializable; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.Comparator; | ||
import java.util.List; | ||
import java.util.Map; | ||
import org.checkerframework.checker.nullness.qual.NonNull; | ||
|
||
/** | ||
* Collator collects and prunes the candidate decisions from each decider so that their impacts are | ||
* aligned. | ||
* | ||
* <p>Decisions can increase or decrease pressure on different key resources on an Elasticsearch | ||
* node. This is encapsulated in each Action via the {@link ImpactVector}. Since each decider | ||
* independently evaluates its decision, it is possible to have conflicting ImpactVectors from | ||
* actions across deciders. | ||
* | ||
* <p>The collator prunes them to ensure we only take actions that either increase, or decrease | ||
* pressure on a particular node's resources. To resolve conflicts, we prefer stability over | ||
* performance. In order for the above guarantee to work, there should be only one collator instance | ||
* in an {@link AnalysisGraph}. | ||
*/ | ||
public class Collator extends Decider { | ||
|
||
public static final String NAME = "collator"; | ||
|
||
/* Deciders can choose to publish decisions at different frequencies based on the | ||
* type of resources monitored and rca signals. The collator should however, not introduce any | ||
* unnecessary delays. As soon as a decision is taken, it should be evaluated and published downstream. | ||
*/ | ||
private static final int collatorFrequency = 1; // Measured in terms of number of evaluationIntervalPeriods | ||
|
||
private static final int evalIntervalSeconds = 5; | ||
|
||
private final ImpactAssessor impactAssessor; | ||
|
||
private final List<Decider> deciders; | ||
|
||
private final Comparator<Action> actionComparator; | ||
|
||
public Collator(Decider... deciders) { | ||
super(evalIntervalSeconds, collatorFrequency); | ||
this.deciders = Arrays.asList(deciders); | ||
this.actionComparator = new ImpactBasedActionComparator(); | ||
this.impactAssessor = new ImpactAssessor(); | ||
} | ||
|
||
/** | ||
* Constructor used for unit testing purposes only. | ||
* | ||
* @param impactAssessor the impact assessor. | ||
* @param actionComparator comparator for sorting actions. | ||
* @param deciders The participating deciders. | ||
*/ | ||
@VisibleForTesting | ||
public Collator(final ImpactAssessor impactAssessor, final Comparator<Action> actionComparator, | ||
Decider... deciders) { | ||
super(evalIntervalSeconds, collatorFrequency); | ||
this.deciders = Arrays.asList(deciders); | ||
this.actionComparator = actionComparator; | ||
this.impactAssessor = impactAssessor; | ||
} | ||
|
||
@Override | ||
public String name() { | ||
return NAME; | ||
} | ||
|
||
/** | ||
* Process all the actions proposed by the deciders and prune them based on their impact vectors. | ||
* | ||
* @return A {@link Decision} instance that contains the list of polarized actions. | ||
*/ | ||
@Override | ||
public Decision operate() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it might be a good idea to add a javadoc comment for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added. |
||
Decision finalDecision = new Decision(System.currentTimeMillis(), NAME); | ||
List<Action> allowedActions = new ArrayList<>(); | ||
|
||
// First get all the actions proposed by the deciders and assess the overall impact all | ||
// actions combined have on all the affected nodes. | ||
|
||
List<Action> allActions = getProposedActions(); | ||
Map<NodeKey, ImpactAssessment> overallImpactAssessment = | ||
impactAssessor.assessOverallImpact(allActions); | ||
|
||
// We need to identify and prune conflicting actions based on the overall impact. In order to | ||
// do that, we re-assess each of the proposed actions with the overall impact assessment in | ||
// mind. In each such assessment, we ensure the impact of an action aligns with the instance's | ||
// current pressure heading(increasing/decreasing). Actions that don't align are pruned and | ||
// their effects on the overall impact are undone. As the order in which we reassess matters | ||
// as to what actions get picked and what don't, we sort the list of actions based on a | ||
// simple heuristic where actions that reduce pressure the most are re-assessed later | ||
// thereby decreasing the chance of them getting pruned because of another action. | ||
|
||
allActions.sort(actionComparator); | ||
allActions.forEach(action -> { | ||
if (impactAssessor.isImpactAligned(action, overallImpactAssessment)) { | ||
allowedActions.add(action); | ||
} else { | ||
impactAssessor.undoActionImpactOnOverallAssessment(action, overallImpactAssessment); | ||
} | ||
}); | ||
|
||
finalDecision.addAllActions(allowedActions); | ||
return finalDecision; | ||
} | ||
|
||
/** | ||
* Combines all actions proposed by the deciders into a list. | ||
* | ||
* @return A list of actions. | ||
*/ | ||
@NonNull | ||
private List<Action> getProposedActions() { | ||
final List<Action> proposedActions = new ArrayList<>(); | ||
if (deciders != null) { | ||
for (final Decider decider : deciders) { | ||
List<Decision> decisions = decider.getFlowUnits(); | ||
decisions.forEach(decision -> { | ||
if (!decision.getActions().isEmpty()) { | ||
proposedActions.addAll(decision.getActions()); | ||
} | ||
}); | ||
} | ||
} | ||
return proposedActions; | ||
} | ||
|
||
/** | ||
* A comparator for actions to sort them based on their impact from least pressure decreasing | ||
* to most. | ||
*/ | ||
@VisibleForTesting | ||
static final class ImpactBasedActionComparator implements Comparator<Action>, Serializable { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting implementation. |
||
|
||
@Override | ||
public int compare(Action action1, Action action2) { | ||
int numberOfPressureReductions1 = getImpactedDimensionCount(action1, Impact.DECREASES_PRESSURE); | ||
int numberOfPressureReductions2 = getImpactedDimensionCount(action2, Impact.DECREASES_PRESSURE); | ||
|
||
if (numberOfPressureReductions1 != numberOfPressureReductions2) { | ||
return numberOfPressureReductions1 - numberOfPressureReductions2; | ||
} | ||
|
||
int numberOfPressureIncreases1 = getImpactedDimensionCount(action1, Impact.INCREASES_PRESSURE); | ||
int numberOfPressureIncreases2 = getImpactedDimensionCount(action2, Impact.INCREASES_PRESSURE); | ||
|
||
if (numberOfPressureIncreases1 != numberOfPressureIncreases2) { | ||
return numberOfPressureIncreases2 - numberOfPressureIncreases1; | ||
} | ||
|
||
return 0; | ||
} | ||
|
||
private int getImpactedDimensionCount(final Action action, Impact requiredImpact) { | ||
int count = 0; | ||
for (ImpactVector impactVector : action.impact().values()) { | ||
for (Impact impact : impactVector.getImpact().values()) { | ||
if (impact.equals(requiredImpact)) { | ||
count++; | ||
} | ||
} | ||
} | ||
return count; | ||
} | ||
} | ||
} |
Oops, something went wrong.
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.
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.
There can only be one collator node in a graph right ? Can we add that to the comments ?
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.
Done.