Fail weight update when decommission ongoing and fail decommission when attribute not weighed away by imRishN · Pull Request #4839 · opensearch-project/OpenSearch

imRishN · 2022-10-19T15:23:23Z

Description

Fail weight update call if decommission action is in process
Fail Decommission call if the attribute is not weighed away

Issues Resolved

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

github-actions · 2022-10-19T15:54:36Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/4956/
CommitID: abf33ce

github-actions · 2022-10-19T15:58:02Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/4957/
CommitID: 0bcbac4

Bukhtawar · 2022-10-19T16:19:21Z

server/src/main/java/org/opensearch/cluster/decommission/DecommissionService.java

+            WeightedRouting weightedRouting = weightedRoutingMetadata.getWeightedRouting();
+            if (weightedRouting.attributeName().equals(decommissionAttribute.attributeName())) {
+                Double attributeValueWeight = weightedRouting.weights().get(decommissionAttribute.attributeValue());
+                if (attributeValueWeight == null || attributeValueWeight != 0.0) {


is this value float or double, should we make specific comparisons. For instance Double.valueOf(0.0)

It is Double. using .equals()

server/src/main/java/org/opensearch/cluster/decommission/DecommissionService.java

server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java

Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

Fix spotless check Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

github-actions · 2022-10-19T17:40:49Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/4970/
CommitID: 4bfd385

github-actions · 2022-10-19T17:45:38Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/4971/
CommitID: 494a7a0

github-actions · 2022-10-19T17:51:22Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/4973/
CommitID: 05eb55a

Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

github-actions · 2022-10-19T18:38:25Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/4982/
CommitID: 042fa3a

codecov-commenter · 2022-10-19T18:42:51Z

Codecov Report

Merging #4839 (042fa3a) into main (eb33015) will decrease coverage by 0.02%.
The diff coverage is 66.66%.

@@             Coverage Diff              @@
##               main    #4839      +/-   ##
============================================
- Coverage     70.73%   70.70%   -0.03%     
- Complexity    57884    57894      +10     
============================================
  Files          4689     4689              
  Lines        277296   277314      +18     
  Branches      40363    40367       +4     
============================================
- Hits         196137   196075      -62     
- Misses        64873    64968      +95     
+ Partials      16286    16271      -15

Impacted Files	Coverage Δ
...arch/cluster/decommission/DecommissionService.java	`32.03% <50.00%> (+2.35%)`	⬆️
...search/cluster/routing/WeightedRoutingService.java	`87.93% <100.00%> (+7.16%)`	⬆️
...n/admin/cluster/node/tasks/get/GetTaskRequest.java	`30.30% <0.00%> (-63.64%)`	⬇️
.../action/admin/indices/flush/ShardFlushRequest.java	`50.00% <0.00%> (-50.00%)`	⬇️
...pensearch/action/ingest/DeletePipelineRequest.java	`31.25% <0.00%> (-37.50%)`	⬇️
.../admin/cluster/node/tasks/get/GetTaskResponse.java	`28.57% <0.00%> (-35.72%)`	⬇️
...earch/action/admin/indices/flush/FlushRequest.java	`52.17% <0.00%> (-34.79%)`	⬇️
...ensearch/client/indices/DetailAnalyzeResponse.java	`20.54% <0.00%> (-34.25%)`	⬇️
...search/search/aggregations/pipeline/EwmaModel.java	`24.44% <0.00%> (-26.67%)`	⬇️
.../aggregations/support/values/ScriptLongValues.java	`46.51% <0.00%> (-23.26%)`	⬇️
... and 477 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

sachinpkale · 2022-10-20T09:44:47Z

server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java

+
+    public void ensureNoOngoingDecommissionAction(ClusterState state) {
+        DecommissionAttributeMetadata decommissionAttributeMetadata = state.metadata().decommissionAttributeMetadata();
+        if (decommissionAttributeMetadata != null && decommissionAttributeMetadata.status().equals(DecommissionStatus.FAILED) == false) {


What if status is SUCCESSFUL?

Only when Decommission has failed, we can allow the weight update can go through. Not for any other status

Isn't SUCCESSFUL a final state? The error message of this if condition says: a decommission action is ongoing with status...

A succesful decommission state means the transport execution has completed. The nodes are decommissioned in this state (not able to start pre voting or join). Hence DecommissionAction ongoing means a zone is decommissioned and the cluster rejects pings from decommissioned node

sachinpkale · 2022-10-20T09:45:33Z

Build is failing, please fix the build.

Bukhtawar · 2022-10-20T09:51:30Z

server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java

+
+    public void ensureNoOngoingDecommissionAction(ClusterState state) {
+        DecommissionAttributeMetadata decommissionAttributeMetadata = state.metadata().decommissionAttributeMetadata();
+        if (decommissionAttributeMetadata != null && decommissionAttributeMetadata.status().equals(DecommissionStatus.FAILED) == false) {


If decommission status is FAILED can certain nodes be out of service or decommissioned while others not as in partially failed?

No, if the decommission is FAILED, joins or prevoting is not blocked on those nodes and hence if some nodes are out of service (which can happen), the weights request can go through

But then the zone isn't fully equipped to take traffic right like 90% nodes are out of service and we place weights 10:1:1 when 10 is for the zone only operating at 10% capacity?

Bukhtawar

Left a comment, LGTM otherwise

Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

github-actions · 2022-10-20T10:58:25Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/5043/
CommitID: eafe468

…en attribute not weighed away (opensearch-project#4839) * Add checks for decommission before setting weights Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

* Fail weight update when decommission ongoing and fail decommission when attribute not weighed away (#4839) * Add changes for graceful node decommission (#4586) * Add delay timeout for decommission request (#4931) * Integ Tests for Awareness Attribute Decommissioning (#4715) Signed-off-by: Rishab Nahata <rnnahata@amazon.com> Signed-off-by: pranikum <109206473+pranikum@users.noreply.github.com>

* Fail weight update when decommission ongoing and fail decommission when attribute not weighed away (opensearch-project#4839) * Add changes for graceful node decommission (opensearch-project#4586) * Add delay timeout for decommission request (opensearch-project#4931) * Integ Tests for Awareness Attribute Decommissioning (opensearch-project#4715) Signed-off-by: Rishab Nahata <rnnahata@amazon.com> Signed-off-by: pranikum <109206473+pranikum@users.noreply.github.com>

…en attribute not weighed away (opensearch-project#4839) * Add checks for decommission before setting weights Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

imRishN added 3 commits October 19, 2022 19:47

Add checks for decommission before setting weights

81f3a69

Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

Precheck weights before decommission

49c4bf2

Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

Fix spotless check

abf33ce

Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

imRishN requested review from a team and reta as code owners October 19, 2022 15:23

imRishN changed the title ~~Wrr validation~~ Fail weight update when decommission ongoing and fail decommission when attribute not weighed away Oct 19, 2022

Add changelog

0bcbac4

Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

Bukhtawar reviewed Oct 19, 2022

View reviewed changes

server/src/main/java/org/opensearch/cluster/decommission/DecommissionService.java Outdated Show resolved Hide resolved

Bukhtawar reviewed Oct 19, 2022

View reviewed changes

server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java Outdated Show resolved Hide resolved

imRishN added 2 commits October 19, 2022 22:44

Update validation structure

4bfd385

Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

Changes

05eb55a

Fix spotless check Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

imRishN force-pushed the wrr-validation branch from 494a7a0 to 05eb55a Compare October 19, 2022 17:20

Fix

042fa3a

Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

imRishN mentioned this pull request Oct 19, 2022

Add changes for graceful node decommission #4586

Merged

6 tasks

sachinpkale reviewed Oct 20, 2022

View reviewed changes

Bukhtawar reviewed Oct 20, 2022

View reviewed changes

Bukhtawar approved these changes Oct 20, 2022

View reviewed changes

Empty-Commit

eafe468

Signed-off-by: Rishab Nahata <rnnahata@amazon.com>

sachinpkale approved these changes Oct 20, 2022

View reviewed changes

Bukhtawar merged commit 3344738 into opensearch-project:main Oct 20, 2022

imRishN mentioned this pull request Nov 3, 2022

[Backport 2.x]Graceful Decommission and Integ Tests #5060

Merged

6 tasks

Conversation

imRishN commented Oct 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Check List

Uh oh!

github-actions bot commented Oct 19, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Oct 19, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

Bukhtawar Oct 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Oct 19, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Oct 19, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Oct 19, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Oct 19, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

codecov-commenter commented Oct 19, 2022

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sachinpkale commented Oct 20, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bukhtawar left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 20, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

imRishN commented Oct 19, 2022 •

edited

Loading

Bukhtawar Oct 19, 2022 •

edited

Loading