AdmissionControl RequestSize AutoTuning #597

mitalawachat · 2021-05-13T14:45:51Z

Fixes #: #579

Description of changes:
AutoTune AdmissionControl RequestSizeController based on heap occupancy (percent).
AdmissionControl currently has default values of of 85% JVMMemoryPressure threshold and 10% RequestSize threshold. As part of this change we are tuning RequestSize threshold based on how much heap is currently occupied. If we have more heap available then we expand the threshold for RequestSize controller and if less heap is available then we contract the RequestSize controller threshold. This tuning happens when heap occupancy (percent) range change is detected.

Tests:

Unit Test
Created cluster, ran load, verified that when heap range changes the flow-units are passed through chain (Rca -> ClusterRca -> Decider -> Publisher)

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

khushbr

High level comments.
1/ Add as much details around the RCA, Decider and Action logic as possible. It is tricky for the reviewer to read the code if no overview is provided in PR and no javadoc in the code.
2/ Create an issue and add the scoping and requirement details there. It helps us keep track of changes going in the new release.
3/ Mention the reasoning for picking specific input metrics for the RCA.
4/ Add details on testing.
5/ Add detail on if this RCA, Decider and Action will be enabled or disabled by default.

.../elasticsearch/performanceanalyzer/util/admission_control/RequestSizeRangeConfiguration.java

...o/elasticsearch/performanceanalyzer/rca/store/rca/admission_control/AdmissionControlRca.java

...endistro/elasticsearch/performanceanalyzer/decisionmaker/actions/AdmissionControlAction.java

...distro/elasticsearch/performanceanalyzer/decisionmaker/deciders/AdmissionControlDecider.java

...endistro/elasticsearch/performanceanalyzer/decisionmaker/actions/AdmissionControlAction.java

...va/com/amazon/opendistro/elasticsearch/performanceanalyzer/util/admission_control/Range.java

...sticsearch/performanceanalyzer/util/admission_control/RequestSizeHeapRangeConfiguration.java

...distro/elasticsearch/performanceanalyzer/decisionmaker/deciders/AdmissionControlDecider.java

sruti1312 · 2021-06-15T14:52:18Z

Can we also add Gauntlet test to test this from end to end?
https://github.com/opendistro-for-elasticsearch/performance-analyzer-rca/blob/main/docs/gauntlet.md

sruti1312 · 2021-06-16T16:53:02Z

pa_config/rca.conf

+            "lower-bound": 0,
+            "upper-bound": 75,


What happens when the user changes this such that we have gap's in range and no threshold declared for that? Are we catching that?
Can we add a test case for this scenario?

One option might be is to have the thresholds as p75_threshold instead of range? But this makes the range not configurable on the fly then.

Sure, will add test around it. No action should be taken if there is gap.

Instead if there is a gap here, can we detect it and fall back to the defaults in this case?

Gaps could be left intentionally. For example, we can configure autotune only when jvm reaches above 85%, and want to work with default admissioncontrol behavior for below usage.

sruti1312 · 2021-06-16T16:57:08Z

...mazon/opendistro/elasticsearch/performanceanalyzer/rca/store/ElasticSearchAnalysisGraph.java

-    admissionControllerRca.addAllUpstreams(Arrays.asList(acCurrent, acThreshold, acRejectionCount));
+  private AdmissionControlDecider buildAdmissionControlDecider(Metric heapUsed, Metric heapMax) {
+    AdmissionControlRca admissionControlRca = new AdmissionControlRca(RCA_PERIOD, heapUsed, heapMax);
+    admissionControlRca.addTag(TAG_LOCUS, LOCUS_DATA_MASTER_NODE);


Looks like admissionControlRca exists on both data and master node. Can we add the rca.conf changes to all the rca*.conf files in the pa_config directory? We have conf specific to master and idle_master

My bad. admissionControlRca should act only on data nodes. Will fix this.

...endistro/elasticsearch/performanceanalyzer/util/range/RequestSizeHeapRangeConfiguration.java

sruti1312 · 2021-06-16T17:13:26Z

...endistro/elasticsearch/performanceanalyzer/decisionmaker/actions/AdmissionControlAction.java

+        // We mark increase/decrease pressure based on desired value
+        final ImpactVector impactVector = new ImpactVector();
+        if (desiredValue > currentValue) {
+            impactVector.increasesPressure(ADMISSION_CONTROL);


Can we see indirect impact on HEAP because of this tuning. If yes, @khushbr Do you think we need to add them here?

AdmissionControl would increase/decrease threshold based on available jvm, and will kick in only when node is under duress. For example, if JVM reaches 95% then the customer will see more 429 compared to when JVM was at 85%, eventually reducing node drops and 5xx.
Increase in heap pressure will be driven from customer workload.

Should you have a cool-off period when heap decreases to avoid flip-flops?

Yes we have 5 mins cool-off period
protected static final long DEFAULT_COOL_OFF_PERIOD_IN_MILLIS = TimeUnit.MINUTES.toMillis(5);

I agree wtih Sruti. how are we going to define the impact of ADMISSION_CONTROL ? and what happen if let's say you have a different type of admission control action in the future and if they both have impact on ADMISSION_CONTROL, then one will be blocked by the other ?

Each controller of admissioncontrol works in its own perview. When we add new controllers in admissioncontrol they would work on their own thresholds. Ofcourse these should be validated when we add more admissioncontrol AutoTune

...ro/elasticsearch/performanceanalyzer/rca/store/rca/admissioncontrol/AdmissionControlRca.java

...ticsearch/performanceanalyzer/rca/store/rca/admissioncontrol/AdmissionControlClusterRca.java

khushbr · 2021-07-08T04:11:57Z

...mazon/opendistro/elasticsearch/performanceanalyzer/rca/store/ElasticSearchAnalysisGraph.java

+  private AdmissionControlDecider buildAdmissionControlDecider(Metric heapUsed, Metric heapMax) {
+    AdmissionControlRca admissionControlRca = new AdmissionControlRca(RCA_PERIOD, heapUsed, heapMax);
+    admissionControlRca.addTag(TAG_LOCUS, LOCUS_DATA_NODE);
+    admissionControlRca.addAllUpstreams(Arrays.asList(heapUsed, heapMax));


Trying to understand why we changed the input metrics to AdmissionControlRca.
Previously we were using Admission Control current value, threshold and rejection count. Now, we are using only heap metrics.

What happens if the heap usage is spiky ? Even with coolOffPeriod, we can end up suggesting rejection threshold change frequently and drastically, where we jump from a lower heap-range bucket to a higher bucket. This will impact the heap stability and the wider customer experience.

Previous metrics were unused. Will add new metrics as required.
Suggestion on avoiding large threshold diff on drastic heap range change?

For frequency, we can extend the cool-off time period to a larger value and for flip-flop we check for delta change suggested ?

Again, I still have my doubts on the efficiency of the RCA to suggest Admission Control Threshold change just based off raw JVM metrics.

We can change default cool-off period to 15 mins instead of 5 and later tune it as required based on case studies.

AdmissionControl decides it's default limits based on % of jvm, that's why only jvm is considered as initial input to rca. More metrics could be added as inputs to rca when we find such suitable metrics.

Changed DEFAULT_COOL_OFF_PERIOD_IN_MILLIS to 15 mins in AdmissionControlAction

mitalawachat added the feature New feature label May 13, 2021

mitalawachat requested review from getsaurabh02, khushbr and Bukhtawar May 13, 2021 14:45

khushbr reviewed May 28, 2021

View reviewed changes

khushbr requested a review from sruti1312 May 28, 2021 20:19

mitalawachat requested review from khushbr and rramachand21 June 6, 2021 13:39

Bukhtawar suggested changes Jun 8, 2021

View reviewed changes

AdmissionControl RequestSize AutoTuning

038429c

mitalawachat requested a review from Bukhtawar June 14, 2021 19:07

sruti1312 reviewed Jun 15, 2021

View reviewed changes

...distro/elasticsearch/performanceanalyzer/decisionmaker/deciders/AdmissionControlDecider.java Outdated Show resolved Hide resolved

sruti1312 reviewed Jun 16, 2021

View reviewed changes

Merge branch 'opendistro-for-elasticsearch:main' into main

ad1353b

mitalawachat requested a review from sruti1312 June 28, 2021 17:54

sruti1312 requested a review from rguo-aws July 1, 2021 04:12

khushbr reviewed Jul 8, 2021

View reviewed changes

mitalawachat requested a review from khushbr July 13, 2021 17:45

AdmissionControl RequestSize AutoTuning

9152922

khushbr approved these changes Jul 15, 2021

View reviewed changes

getsaurabh02 approved these changes Jul 16, 2021

View reviewed changes

sruti1312 approved these changes Jul 16, 2021

View reviewed changes

mitalawachat merged commit af97de5 into opendistro-for-elasticsearch:main Jul 16, 2021

mitalawachat mentioned this pull request Aug 4, 2021

Porting commit from opendistro-for-elasticsearch/performance-analyzer-rca opensearch-project/performance-analyzer-rca#39

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AdmissionControl RequestSize AutoTuning #597

AdmissionControl RequestSize AutoTuning #597

mitalawachat commented May 13, 2021 •

edited

khushbr left a comment •

edited

sruti1312 commented Jun 15, 2021

sruti1312 Jun 16, 2021

mitalawachat Jun 17, 2021

sruti1312 Jul 13, 2021

mitalawachat Jul 14, 2021

sruti1312 Jun 16, 2021

mitalawachat Jun 17, 2021

sruti1312 Jun 16, 2021

mitalawachat Jun 17, 2021

Bukhtawar Jun 17, 2021

mitalawachat Jun 27, 2021

rguo-aws Jul 9, 2021

mitalawachat Jul 9, 2021

khushbr Jul 8, 2021

mitalawachat Jul 8, 2021

khushbr Jul 13, 2021

mitalawachat Jul 14, 2021 •

edited

mitalawachat Jul 14, 2021

AdmissionControl RequestSize AutoTuning #597

AdmissionControl RequestSize AutoTuning #597

Conversation

mitalawachat commented May 13, 2021 • edited

khushbr left a comment • edited

Choose a reason for hiding this comment

sruti1312 commented Jun 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mitalawachat Jul 14, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mitalawachat commented May 13, 2021 •

edited

khushbr left a comment •

edited

mitalawachat Jul 14, 2021 •

edited