Integrate remote cluster state in publish flow #3
Conversation
soosinha
force-pushed
the
remote_state_publish
branch
3 times, most recently
from
38eebe4 to
0345ea5
Compare
soosinha
force-pushed
the
remote_state_publish
branch
from
0345ea5 to
e06aeaf
Compare
| @@ -1308,6 +1312,12 @@ assert getLocalNode().equals(clusterState.getNodes().get(getLocalNode().getId()) | |||
| leaderChecker.setCurrentNodes(publishNodes); | |||
| followersChecker.setCurrentNodes(publishNodes); | |||
| lagDetector.setTrackedNodes(publishNodes); | |||
| if (clusterState.nodes().isLocalNodeElectedClusterManager() | |||
There was a problem hiding this comment.
you can add a comment here why you are doing it here. This add extra latency to overall publish flow. How will this change with remote only state.
| final Optional<ApplyCommitRequest> optionalApplyCommitRequest = coordinationState.get() | ||
| .handlePublishResponse(sourceNode, publishResponse); | ||
| optionalApplyCommitRequest.ifPresent(applyCommitRequest -> { | ||
| if (REMOTE_CLUSTER_STATE_ENABLED_SETTING.get(settings) == true) { | ||
| final CoordinationState.PersistedState remotePersistedState = remotePersistedStateSupplier.get(); | ||
| assert remotePersistedState != null : "Remote state has not been initialized"; | ||
| remotePersistedState.markLastAcceptedStateAsCommitted(); |
There was a problem hiding this comment.
This can be triggered when commit is being performed on the active cluster manager node. why do you need special handling here? Are you doing this to ensure it is always committed to remote first.
There was a problem hiding this comment.
Yes. Even while publishing I am storing in remote first. So to keep it consistent, I am committing to remote first.
| @@ -122,7 +129,8 @@ public void start( | |||
| MetaStateService metaStateService, | |||
| MetadataIndexUpgradeService metadataIndexUpgradeService, | |||
| MetadataUpgrader metadataUpgrader, | |||
| PersistedClusterStateService persistedClusterStateService | |||
| PersistedClusterStateService persistedClusterStateService, | |||
| RemoteClusterStateService remoteClusterStateService | |||
There was a problem hiding this comment.
With the current integration, it will always initialize RemoteClusterStateService even when remote cluster state is not enabled.
There was a problem hiding this comment.
RemoteClusterStateService is not a heavy object. It is contains few fields (<10). Anyway I will see if can initialize this only when remote state is enabled.
soosinha
force-pushed
the
remote_state_publish
branch
3 times, most recently
from
d4e3417 to
91c616f
Compare
soosinha
force-pushed
the
remote_state
branch
3 times, most recently
from
e8f6f86 to
be36097
Compare
soosinha
force-pushed
the
remote_state_publish
branch
4 times, most recently
from
1bd3276 to
6819419
Compare
Signed-off-by: Sooraj Sinha <soosinha@amazon.com>
Signed-off-by: Sooraj Sinha <soosinha@amazon.com>
Signed-off-by: Sooraj Sinha <soosinha@amazon.com>
Signed-off-by: Sooraj Sinha <soosinha@amazon.com>
Signed-off-by: Sooraj Sinha <soosinha@amazon.com>
soosinha
force-pushed
the
remote_state_publish
branch
from
155e369 to
3773049
Compare
soosinha
force-pushed
the
remote_state
branch
2 times, most recently
from
2506421 to
4824bb5
Compare
Description
Follow up PR for : opensearch-project#9160
In the above PR, RemotePersistedState and RemoteClusterStateService were created. In the current PR, these classes are integrated into the cluster state publish flow. The upload of cluster metadata will happen just before the cluster state publish request is sent to all nodes
Related Issues
opensearch-project#9338
Check List
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.