Fix decommission status update to non leader nodes #4800

imRishN · 2022-10-16T06:29:51Z

Description

This PR resolves decommission status update metadata bug where in the non leader nodes were not getting status update locally as the same object was updated during submit state update causing the diff to be 0. Detailed explanation can be found in the issue.

Issues Resolved

#4799

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 <[email protected]>

anshu1106 · 2022-10-16T06:43:40Z

Thanks @imRishN. Fix looks good to me. Please add tests.

github-actions · 2022-10-16T06:46:37Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/4744/
CommitID: 1c97e42

github-actions · 2022-10-16T07:08:11Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/4745/
CommitID: 9df0e7d

codecov-commenter · 2022-10-16T07:08:56Z

Codecov Report

Merging #4800 (dd8b4ec) into main (1d65485) will decrease coverage by 0.07%.
The diff coverage is 57.14%.

@@             Coverage Diff              @@
##               main    #4800      +/-   ##
============================================
- Coverage     70.76%   70.69%   -0.08%     
+ Complexity    57926    57835      -91     
============================================
  Files          4689     4689              
  Lines        277306   277305       -1     
  Branches      40370    40370              
============================================
- Hits         196234   196037     -197     
- Misses        64822    64903      +81     
- Partials      16250    16365     +115

Impacted Files	Coverage Δ
...er/decommission/DecommissionAttributeMetadata.java	`65.78% <25.00%> (+1.23%)`	⬆️
...h/cluster/decommission/DecommissionController.java	`80.72% <100.00%> (+0.47%)`	⬆️
...n/indices/forcemerge/ForceMergeRequestBuilder.java	`0.00% <0.00%> (-75.00%)`	⬇️
...a/org/opensearch/client/cluster/ProxyModeInfo.java	`0.00% <0.00%> (-55.00%)`	⬇️
.../java/org/opensearch/node/NodeClosedException.java	`50.00% <0.00%> (-50.00%)`	⬇️
...ava/org/opensearch/action/NoSuchNodeException.java	`0.00% <0.00%> (-50.00%)`	⬇️
.../java/org/opensearch/search/dfs/AggregatedDfs.java	`51.61% <0.00%> (-45.17%)`	⬇️
.../opensearch/client/indices/CloseIndexResponse.java	`48.75% <0.00%> (-42.50%)`	⬇️
.../admin/cluster/reroute/ClusterRerouteResponse.java	`60.00% <0.00%> (-40.00%)`	⬇️
...h/action/ingest/SimulateDocumentVerboseResult.java	`60.71% <0.00%> (-39.29%)`	⬇️
... and 489 more

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

Signed-off-by: Rishab Nahata <[email protected]>

imRishN · 2022-10-16T07:26:33Z

...alClusterTest/java/org/opensearch/cluster/coordination/AwarenessAttributeDecommissionIT.java

+ * compatible open source license.
+ */
+
+package org.opensearch.cluster.coordination;


Added it in this package as all the integ test for decommissioning the zone need access to pkg private methods of Coordinator.
PR for integ tests - #4715

github-actions · 2022-10-16T07:49:21Z

Gradle Check (Jenkins) Run Completed with:

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

github-actions · 2022-10-16T07:51:46Z

Gradle Check (Jenkins) Run Completed with:

RESULT: null ❌
URL: https://build.ci.opensearch.org/job/gradle-check/4752/
CommitID: 24263c5

Signed-off-by: Rishab Nahata <[email protected]>

github-actions · 2022-10-16T07:55:48Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/4751/
CommitID: edf8797

anshu1106 · 2022-10-16T08:02:42Z

...alClusterTest/java/org/opensearch/cluster/coordination/AwarenessAttributeDecommissionIT.java

+        // and won't receive status update to SUCCESSFUL
+        String randomDecommissionedNode = randomFrom(clusterManagerNodes.get(2), dataNodes.get(2));
+        ClusterService decommissionedNodeClusterService = internalCluster().getInstance(ClusterService.class, randomDecommissionedNode);
+        assertEquals(


This assert is done with assumption that decommission can take some time and during this check status would mostly be in progress?

No, when the decommissioned nodes were kicked out all the nodes would be having status as IN_PROGRESS. Status is updated to SUCCESSFUL after the decommissioned nodes are kicked out. And hence the last status that decommissioned nodes would be seeing is IN_PROGRESS. If not, the test must fail

github-actions · 2022-10-16T08:28:41Z

Gradle Check (Jenkins) Run Completed with:

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

imRishN · 2022-10-16T09:42:10Z

@Bukhtawar Can you please review this?

Bukhtawar · 2022-10-16T16:12:12Z

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

+        // if the current status is the expected status already or new status is FAILED, we let the check pass
+        if (newStatus.equals(status) || newStatus.equals(DecommissionStatus.FAILED)) {
+            return;


This is assuming that all steps can have a self-loop for state transitions

Didn't get this. This is added to ensure that if any step wants to mark the status as FAILED during decommission, we will allow it to do so. Let me know if you have any specific case in mind

And this behaviour is same as before. Had to refactor this method a bit because we were updating the same instance which led to relative diff as 0 for decommission state update

The self loops which we might come up during multiple concurrent requests is handled seperately as part of this PR #4684. Today we need status to get into same state as per current service implementation

The other condition makes it look paranoid

Bukhtawar

Lets revisit this as a part of #4684

github-actions · 2022-10-18T06:39:56Z

Gradle Check (Jenkins) Run Completed with:

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

…t#4800) * Fix decommission status update to non leader nodes Signed-off-by: Rishab Nahata <[email protected]>

* Add DecommissionService and helper to execute awareness attribute decommissioning #4084 * Add APIs (GET/PUT) to decommission awareness attribute #4261 * Controlling discovery for decommissioned nodes #4590 * Fix decommission status update to non leader nodes #4800 * Remove redundant field from GetDecommissionStateResponse #4751 * Service Layer changes for Recommission API #4320 * Recommission api level support #4604 * Fix bug in AwarenessAttributeDecommissionIT #4822 Signed-off-by: Rishab Nahata <[email protected]>

* Add DecommissionService and helper to execute awareness attribute decommissioning opensearch-project#4084 * Add APIs (GET/PUT) to decommission awareness attribute opensearch-project#4261 * Controlling discovery for decommissioned nodes opensearch-project#4590 * Fix decommission status update to non leader nodes opensearch-project#4800 * Remove redundant field from GetDecommissionStateResponse opensearch-project#4751 * Service Layer changes for Recommission API opensearch-project#4320 * Recommission api level support opensearch-project#4604 * Fix bug in AwarenessAttributeDecommissionIT opensearch-project#4822 Signed-off-by: Rishab Nahata <[email protected]>

* Awareness attribute decommission backports (#4970) * Add DecommissionService and helper to execute awareness attribute decommissioning #4084 * Add APIs (GET/PUT) to decommission awareness attribute #4261 * Controlling discovery for decommissioned nodes #4590 * Fix decommission status update to non leader nodes #4800 * Remove redundant field from GetDecommissionStateResponse #4751 * Service Layer changes for Recommission API #4320 * Recommission api level support #4604 * Fix bug in AwarenessAttributeDecommissionIT #4822 Signed-off-by: Rishab Nahata <[email protected]>

…t#4800) * Fix decommission status update to non leader nodes Signed-off-by: Rishab Nahata <[email protected]>

imRishN added 2 commits October 16, 2022 11:16

Fix metadata update

ce226bd

Signed-off-by: Rishab Nahata <[email protected]>

Fix spotless check

1c97e42

Signed-off-by: Rishab Nahata <[email protected]>

imRishN requested review from a team and reta as code owners October 16, 2022 06:29

imRishN changed the title ~~Decommission/metadata bug~~ Fix decommission status update to non leader nodes Oct 16, 2022

Add changelog

9df0e7d

Signed-off-by: Rishab Nahata <[email protected]>

imRishN added 4 commits October 16, 2022 12:47

Add integ test

83ea615

Signed-off-by: Rishab Nahata <[email protected]>

Fix spotless check

1826003

Signed-off-by: Rishab Nahata <[email protected]>

Fix

edf8797

Signed-off-by: Rishab Nahata <[email protected]>

Fix

24263c5

Signed-off-by: Rishab Nahata <[email protected]>

imRishN commented Oct 16, 2022

View reviewed changes

Empty-Commit

0e89f8b

Signed-off-by: Rishab Nahata <[email protected]>

anshu1106 reviewed Oct 16, 2022

View reviewed changes

anshu1106 approved these changes Oct 16, 2022

View reviewed changes

Bukhtawar reviewed Oct 16, 2022

View reviewed changes

Bukhtawar approved these changes Oct 17, 2022

View reviewed changes

Merge branch 'main' into decommission/metadata-bug

dd8b4ec

Bukhtawar merged commit cdc7a2f into opensearch-project:main Oct 18, 2022

imRishN added a commit to imRishN/OpenSearch that referenced this pull request Oct 28, 2022

Fix decommission status update to non leader nodes (opensearch-projec…

91123ba

…t#4800) * Fix decommission status update to non leader nodes Signed-off-by: Rishab Nahata <[email protected]>

imRishN mentioned this pull request Oct 28, 2022

Awareness attribute decommission backports #4970

Merged

6 tasks

imRishN mentioned this pull request Nov 3, 2022

Awareness attribute decommission 2.4 backport #5056

Closed

6 tasks

ashking94 pushed a commit to ashking94/OpenSearch that referenced this pull request Nov 7, 2022

Fix decommission status update to non leader nodes (opensearch-projec…

bfdb3f7

…t#4800) * Fix decommission status update to non leader nodes Signed-off-by: Rishab Nahata <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix decommission status update to non leader nodes #4800

Fix decommission status update to non leader nodes #4800

imRishN commented Oct 16, 2022 •

edited

Loading

anshu1106 commented Oct 16, 2022

github-actions bot commented Oct 16, 2022

github-actions bot commented Oct 16, 2022

codecov-commenter commented Oct 16, 2022 •

edited

Loading

imRishN Oct 16, 2022

github-actions bot commented Oct 16, 2022

github-actions bot commented Oct 16, 2022

github-actions bot commented Oct 16, 2022

anshu1106 Oct 16, 2022 •

edited

Loading

imRishN Oct 16, 2022 •

edited

Loading

github-actions bot commented Oct 16, 2022

imRishN commented Oct 16, 2022

Bukhtawar Oct 16, 2022

imRishN Oct 16, 2022

imRishN Oct 16, 2022

imRishN Oct 17, 2022

Bukhtawar Oct 17, 2022

Bukhtawar left a comment

github-actions bot commented Oct 18, 2022

Fix decommission status update to non leader nodes #4800

Fix decommission status update to non leader nodes #4800

Conversation

imRishN commented Oct 16, 2022 • edited Loading

Description

Issues Resolved

Check List

anshu1106 commented Oct 16, 2022

github-actions bot commented Oct 16, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Oct 16, 2022

Gradle Check (Jenkins) Run Completed with:

codecov-commenter commented Oct 16, 2022 • edited Loading

Codecov Report

imRishN Oct 16, 2022

Choose a reason for hiding this comment

github-actions bot commented Oct 16, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Oct 16, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Oct 16, 2022

Gradle Check (Jenkins) Run Completed with:

anshu1106 Oct 16, 2022 • edited Loading

Choose a reason for hiding this comment

imRishN Oct 16, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Oct 16, 2022

Gradle Check (Jenkins) Run Completed with:

imRishN commented Oct 16, 2022

Bukhtawar Oct 16, 2022

Choose a reason for hiding this comment

imRishN Oct 16, 2022

Choose a reason for hiding this comment

imRishN Oct 16, 2022

Choose a reason for hiding this comment

imRishN Oct 17, 2022

Choose a reason for hiding this comment

Bukhtawar Oct 17, 2022

Choose a reason for hiding this comment

Bukhtawar left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 18, 2022

Gradle Check (Jenkins) Run Completed with:

imRishN commented Oct 16, 2022 •

edited

Loading

codecov-commenter commented Oct 16, 2022 •

edited

Loading

anshu1106 Oct 16, 2022 •

edited

Loading

imRishN Oct 16, 2022 •

edited

Loading