[BUG] ClusterManager operations are not called with "task" param

[saikaranam-amazon]

Describe the bug
Recent changes on main (and backport of these changes on v2.2 branch) has a bug where corresponding method of clusterManagerOperation with task param is not called.

** Issue/Fix **
clusterManagerOperations with task param needs to call corresponding previous method with task param

protected void clusterManagerOperation(Task task, Request request, ClusterState state, ActionListener<Response> listener)
        throws Exception {
        masterOperation(task, request, state, listener);
    }

Ref: https://github.com/opensearch-project/OpenSearch/blob/2.x/server/src/main/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeAction.java#L141-L144

[saikaranam-amazon]

@Rishikesh1159 - Could you take a look ?

[tlfeng]

Hi @saikaranam-amazon , I'm afraid this is not a bug caused by the inclusive terminology renaming.
This is the code in version 2.1 before the renaming.

OpenSearch/server/src/main/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeAction.java

Lines 122 to 127 in 388c80a

	/**
	* Override this operation if access to the task parameter is needed
	*/
	protected void masterOperation(Task task, Request request, ClusterState state, ActionListener<Response> listener) throws Exception {
	masterOperation(request, state, listener);
	}

(https://github.com/opensearch-project/OpenSearch/blob/2.1.0/server/src/main/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeAction.java#L122-L127)

The task parameter in method masterOperation(...) was not called by default, and the comment told to override the method if accessing task parameter is needed.

[saikaranam-amazon]

@tlfeng

CCR is using the TransportMasterNodeAction (and there are no other changes apart from the version bump to 2.2) and masterOperation (with task param) method is overriden to get the relevant task details.
Problem is, this method is never called evident from the below relevant stack trace as eventually all methods under TransportClusterManagerNodeAction are wired to call masterOperation (without task params)

at org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction.clusterManagerOperation(TransportClusterManagerNodeAction.java:129)
        at org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction.clusterManagerOperation(TransportClusterManagerNodeAction.java:143)
        at org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction$AsyncSingleAction.lambda$doStart$3(TransportClusterManagerNodeAction.java:221)
        at org.opensearch.action.ActionRunnable$2.doRun(ActionRunnable.java:88)
        at org.opensearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:52)
        at org.opensearch.common.util.concurrent.OpenSearchExecutors$DirectExecutorService.execute(OpenSearchExecutors.java:301)
        at org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction$AsyncSingleAction.doStart(TransportClusterManagerNodeAction.java:221)
        at org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction.doExecute(TransportClusterManagerNodeAction.java:159)
        at org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction.doExecute(TransportClusterManagerNodeAction.java:74)
        at org.opensearch.action.support.TransportAction$RequestFilterChain.proceed(TransportAction.java:204)
        at org.opensearch.action.support.TransportAction.execute(TransportAction.java:174)
        at org.opensearch.action.support.TransportAction.execute(TransportAction.java:102)
        at org.opensearch.client.node.NodeClient.executeLocally(NodeClient.java:110)
        at org.opensearch.client.node.NodeClient.doExecute(NodeClient.java:97)
        at org.opensearch.client.support.AbstractClient.execute(AbstractClient.java:423)
        at org.opensearch.replication.util.CoroutinesKt$suspendExecute$2.invokeSuspend(Coroutines.kt:80)
        at org.opensearch.replication.util.CoroutinesKt$suspendExecute$2.invoke(Coroutines.kt)
        at org.opensearch.replication.util.CoroutinesKt$suspendExecute$2.invoke(Coroutines.kt)
        at kotlinx.coroutines.intrinsics.UndispatchedKt.startUndispatchedOrReturn(Undispatched.kt:89)
        at kotlinx.coroutines.BuildersKt__Builders_commonKt.withContext(Builders.common.kt:165)
        at kotlinx.coroutines.BuildersKt.withContext(Unknown Source)
        at org.opensearch.replication.util.CoroutinesKt.suspendExecute(Coroutines.kt:79)
        at org.opensearch.replication.util.CoroutinesKt.suspendExecute$default(Coroutines.kt:75)

[tlfeng]

Thanks for pointing out this problem! 👍

I found that the cause is the method masterOperation(Task task, Request request, ClusterState state, ActionListener<Response> listener) is totally bypassed.

OpenSearch/server/src/main/java/org/opensearch/action/support/clustermanager/TransportClusterManagerNodeAction.java

Line 221 in 01123f8

.execute(ActionRunnable.wrap(delegate, l -> clusterManagerOperation(task, request, clusterState, l)));

The above line should be reverted to using masterOperation (with task param). I will make the code change for it.

[tlfeng]

Created PR #4103 to solve the problem. Thank you for reporting this problem.

[saikaranam-amazon]

Created PR #4103 to solve the problem

Thanks @tlfeng

[tlfeng]

The fix is backported to 2.x and 2.2 branch by PR #4114 and #4115.