Handling exception in getAssignment method #881

nisgoel-amazon · 2023-05-23T11:25:27Z

Handling exception in getAssignment method

Description

getAssignment method is used to assign shard replication task via persistent task cluster service. In case of stale entries present in persistent tasks in cluster state when this method is trying to assign task it is not handled properly.

Most of the times stale entries are not present in cluster state as we are clearing them at the time of cancelling the replication task but if cancelling is failed by any reason then these entries can remain in the cluster state.

Issues Resolved

#880

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

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.

Handling exception in getAssignment method Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]>

codecov · 2023-05-23T11:47:41Z

Codecov Report

Merging #881 (10b3104) into main (3e3787d) will decrease coverage by 26.22%.
The diff coverage is 37.50%.

❗ Current head 10b3104 differs from pull request most recent head e6a1398. Consider uploading reports for the commit e6a1398 to get more accurate results

@@              Coverage Diff              @@
##               main     #881       +/-   ##
=============================================
- Coverage     72.39%   46.18%   -26.22%     
+ Complexity     1007      630      -377     
=============================================
  Files           141      141               
  Lines          4695     4699        +4     
  Branches        527      527               
=============================================
- Hits           3399     2170     -1229     
- Misses          963     2270     +1307     
+ Partials        333      259       -74

Impacted Files	Coverage Δ
...arch/replication/task/autofollow/AutoFollowTask.kt	`0.63% <0.00%> (-72.16%)`	⬇️
...replication/task/shard/ShardReplicationExecutor.kt	`56.25% <42.85%> (-18.75%)`	⬇️

... and 72 files with indirect coverage changes

nisgoel-amazon · 2023-05-23T12:44:12Z

Security and knn tests are failing due to known build failures with security plugin

ERROR][o.o.s.f.SecurityFilter   ] [leaderCluster-0] Unexpected exception java.lang.NoSuchMethodError: 'boolean org.opensearch.common.Strings.isNullOrEmpty(java.lang.String)'
»  java.lang.NoSuchMethodError: 'boolean org.opensearch.common.Strings.isNullOrEmpty(java.lang.String)'
»  	at org.opensearch.security.privileges.PrivilegesEvaluator.setUserInfoInThreadContext(PrivilegesEvaluator.java:204) ~[opensearch-security-3.0.0.0-SNAPSHOT.jar:3.0.0.0-SNAPSHOT]
»  	at org.opensearch.security.privileges.PrivilegesEvaluator.evaluate(PrivilegesEvaluator.java:248) ~[opensearch-security-3.0.0.0-SNAPSHOT.jar:3.0.0.0-SNAPSHOT]
»  	at org.opensearch.security.filter.SecurityFilter.apply0(SecurityFilter.java:303) [opensearch-security-3.0.0.0-SNAPSHOT.jar:3.0.0.0-SNAPSHOT]
»  	at org.opensearch.security.filter.SecurityFilter.apply(SecurityFilter.java:149) [opensearch-security-3.0.0.0-SNAPSHOT.jar:3.0.0.0-SNAPSHOT]
»  	at org.opensearch.action.support.TransportAction$RequestFilterChain.proceed(TransportAction.java:216) [opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
»  	at org.opensearch.action.support.TransportAction.execute(TransportAction.java:188) [opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
»  	at org.opensearch.action.support.TransportAction.execute(TransportAction.java:107) [opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
»  	at org.opensearch.client.node.NodeClient.executeLocally(NodeClient.java:110) [opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
»  	at org.opensearch.client.node.NodeClient.doExecute(NodeClient.java:97) [opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
»  	at org.opensearch.client.support.AbstractClient.execute(AbstractClient.java:476) [opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
»  	at org.opensearch.client.support.AbstractClient$ClusterAdmin.execute(AbstractClient.java:788) [opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
»  	at org.opensearch.client.support.AbstractClient$ClusterAdmin.health(AbstractClient.java:803) [opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
»  	at org.opensearch.rest.action.admin.cluster.RestClusterHealthAction.lambda$prepareRequest$0(RestClusterHealthAction.java:85) [opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
»  	at org.opensearch.rest.BaseRestHandler.handleRequest(BaseRestHandler.java:128) [opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
»  	at org.opensearch.security.filter.SecurityRestFilter$1.handleRequest(SecurityRestFilter.java:127) [opensearch-security-3.0.0.0-SNAPSHOT.jar:3.0.0.0-SNAPSHOT]
»  	at org.opensearch.rest.RestController.dispatchRequest(RestController.java:320) [opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
»  	at org.opensearch.rest.RestController.tryAllHandlers(RestController.java:411) [opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
»  	at org.opensearch.rest.RestController.dispatchRequest(RestController.java:249) [opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
»  	at org.opensearch.security.ssl.http.netty.ValidatingDispatcher.dispatchRequest(ValidatingDispatcher.java:63) [opensearch-security-3.0.0.0-SNAPSHOT.jar:3.0.0.0-SNAPSHOT]

ERROR][o.o.b.OpenSearchUncaughtExceptionHandler] [followCluster-0] fatal error in thread [DefaultDispatcher-worker-1 @coroutine#1], exiting
»  java.lang.NoClassDefFoundError: org/opensearch/core/common/Strings
»  	at org.opensearch.commons.authuser.User.parse(User.java:162) ~[common-utils-3.0.0.0-SNAPSHOT.jar:?]
»  	at org.opensearch.replication.util.SecurityContext$Companion.fromSecurityThreadContext(SecurityContext.kt:65) ~[opensearch-cross-cluster-replication-3.0.0.0-SNAPSHOT.jar:3.0.0.0-SNAPSHOT]
»  	at org.opensearch.replication.action.index.TransportReplicateIndexAction$doExecute$1.invokeSuspend(TransportReplicateIndexAction.kt:66) ~[opensearch-cross-cluster-replication-3.0.0.0-SNAPSHOT.jar:3.0.0.0-SNAPSHOT]

monusingh-1 · 2023-05-23T15:04:18Z

@nisgoel-amazon will this be backported to all other branches ?

nisgoel-amazon · 2023-05-24T04:13:55Z

@nisgoel-amazon will this be backported to all other branches ?

yes

saikaranam-amazon · 2023-05-24T05:01:01Z

src/main/kotlin/org/opensearch/replication/task/shard/ShardReplicationExecutor.kt

+        } catch (e: Exception) {
+            log.error("Failed to assign shard replication task with id  ${params.followerShardId}", e)
+            return SHARD_NOT_ACTIVE
+        }
    }


Let's add UTs for the assignment logic and can we validate the changes required for Index Replication task as well? (if any)

Index Replication task does not attach to particular node, so there assignment can happen on any node.
Those tasks are working fine

UT has been added

saikaranam-amazon · 2023-05-24T05:01:56Z

src/main/kotlin/org/opensearch/replication/task/shard/ShardReplicationExecutor.kt

-        if (!primaryShard.active()) return SHARD_NOT_ACTIVE
-        return Assignment(primaryShard.currentNodeId(), "node with primary shard")
+        try {
+            val primaryShard = clusterState.routingTable().shardRoutingTable(params.followerShardId).primaryShard()


For testing, will _stop API clear these entries when the shards are not active?

For this we are taking another issue
#903

Signed-off-by: Nishant Goel <[email protected]>

* Handling exception in getAssignment method Handling exception in getAssignment method Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]> * Adding UT for getAssignment Method Signed-off-by: Nishant Goel <[email protected]> --------- Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]> (cherry picked from commit 14b9268)

* Handling exception in getAssignment method Handling exception in getAssignment method * Adding UT for getAssignment Method --------- Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]>

* Handling exception in getAssignment method Handling exception in getAssignment method Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]> * Adding UT for getAssignment Method Signed-off-by: Nishant Goel <[email protected]> --------- Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]> (cherry picked from commit 14b9268) Co-authored-by: Nishant Goel <[email protected]>

* Handling exception in getAssignment method (#881) * Handling exception in getAssignment method Handling exception in getAssignment method Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]> * Adding UT for getAssignment Method Signed-off-by: Nishant Goel <[email protected]> --------- Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]> (cherry picked from commit 14b9268) * Fixing build Signed-off-by: Nishant Goel <[email protected]> --------- Signed-off-by: Nishant Goel <[email protected]>

* Handling exception in getAssignment method (#881) * Handling exception in getAssignment method Handling exception in getAssignment method Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]> * Adding UT for getAssignment Method Signed-off-by: Nishant Goel <[email protected]> --------- Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]> (cherry picked from commit 14b9268) * Fixing build Signed-off-by: Nishant Goel <[email protected]> --------- Signed-off-by: Nishant Goel <[email protected]> (cherry picked from commit 2c28901)

* Handling exception in getAssignment method (#881) * Handling exception in getAssignment method Handling exception in getAssignment method Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]> * Adding UT for getAssignment Method Signed-off-by: Nishant Goel <[email protected]> --------- Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]> (cherry picked from commit 14b9268) * Fixing build Signed-off-by: Nishant Goel <[email protected]> --------- Signed-off-by: Nishant Goel <[email protected]> (cherry picked from commit 2c28901) Co-authored-by: Nishant Goel <[email protected]>

* Handling exception in getAssignment method (#937) * Handling exception in getAssignment method (#881) * Handling exception in getAssignment method Handling exception in getAssignment method Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]> * Adding UT for getAssignment Method Signed-off-by: Nishant Goel <[email protected]> --------- Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]> (cherry picked from commit 14b9268) * Fixing build Signed-off-by: Nishant Goel <[email protected]> --------- Signed-off-by: Nishant Goel <[email protected]> (cherry picked from commit 2c28901) * Fixing build Signed-off-by: Nishant Goel <[email protected]> --------- Signed-off-by: Nishant Goel <[email protected]>

Handling exception in getAssignment method

7bf4de9

Handling exception in getAssignment method Signed-off-by: Nishant Goel <[email protected]> Signed-off-by: Nishant Goel <[email protected]>

nisgoel-amazon requested a review from gbbafna as a code owner May 23, 2023 11:25

ankitkala previously approved these changes May 23, 2023

View reviewed changes

saikaranam-amazon reviewed May 24, 2023

View reviewed changes

ankitkala added backport 1.1 backport 1.2 backport 1.3 backport 2.x backport 1.x backport 2.3 backport 2.5 backport 2.7 labels May 29, 2023

nisgoel-amazon dismissed ankitkala’s stale review via b3f7b42 May 30, 2023 19:38

saikaranam-amazon added the backport 2.8 label May 31, 2023

Adding UT for getAssignment Method

e6a1398

Signed-off-by: Nishant Goel <[email protected]>

ankitkala approved these changes May 31, 2023

View reviewed changes

saikaranam-amazon approved these changes May 31, 2023

View reviewed changes

ankitkala merged commit 14b9268 into opensearch-project:main May 31, 2023

opensearch-trigger-bot bot mentioned this pull request May 31, 2023

[Backport 1.1] Handling exception in getAssignment method #920

Closed

opensearch-trigger-bot bot mentioned this pull request May 31, 2023

[Backport 1.2] Handling exception in getAssignment method #921

Closed

This was referenced May 31, 2023

[Backport 1.3] Handling exception in getAssignment method #922

Closed

[Backport 2.x] Handling exception in getAssignment method #923

Merged

opensearch-trigger-bot bot mentioned this pull request May 31, 2023

[Backport 1.x] Handling exception in getAssignment method #924

Closed

This was referenced May 31, 2023

[Backport 2.3] Handling exception in getAssignment method #925

Closed

[Backport 2.5] Handling exception in getAssignment method #926

Closed

This was referenced May 31, 2023

[Backport 2.7] Handling exception in getAssignment method #927

Merged

[Backport 2.8] Handling exception in getAssignment method #928

Merged

nisgoel-amazon deleted the patch-1 branch May 31, 2023 09:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handling exception in getAssignment method #881

Handling exception in getAssignment method #881

nisgoel-amazon commented May 23, 2023

codecov bot commented May 23, 2023 •

edited

Loading

nisgoel-amazon commented May 23, 2023

monusingh-1 commented May 23, 2023

nisgoel-amazon commented May 24, 2023

saikaranam-amazon May 24, 2023 •

edited

Loading

nisgoel-amazon May 29, 2023

nisgoel-amazon May 30, 2023

saikaranam-amazon May 24, 2023

nisgoel-amazon May 29, 2023

Handling exception in getAssignment method #881

Handling exception in getAssignment method #881

Conversation

nisgoel-amazon commented May 23, 2023

Description

Issues Resolved

Check List

codecov bot commented May 23, 2023 • edited Loading

Codecov Report

nisgoel-amazon commented May 23, 2023

monusingh-1 commented May 23, 2023

nisgoel-amazon commented May 24, 2023

saikaranam-amazon May 24, 2023 • edited Loading

Choose a reason for hiding this comment

nisgoel-amazon May 29, 2023

Choose a reason for hiding this comment

nisgoel-amazon May 30, 2023

Choose a reason for hiding this comment

saikaranam-amazon May 24, 2023

Choose a reason for hiding this comment

nisgoel-amazon May 29, 2023

Choose a reason for hiding this comment

codecov bot commented May 23, 2023 •

edited

Loading

saikaranam-amazon May 24, 2023 •

edited

Loading