Skip to content

Commit b068355

Browse files
shiv0408shiv0408
authored
Add unit tests for read flow of RemoteClusterStateService and bug fix for transient settings (opensearch-project#14476)
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
1 parent c4d960f commit b068355

File tree

8 files changed

+1531
-325
lines changed

8 files changed

+1531
-325
lines changed

‎server/src/main/java/org/opensearch/cluster/coordination/PublicationTransportHandler.java

-1
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,6 @@ PublishWithJoinResponse handleIncomingRemotePublishRequest(RemotePublishRequest
284284
)
285285
);
286286
ClusterState clusterState = remoteClusterStateService.getClusterStateUsingDiff(
287-
request.getClusterName(),
288287
manifest,
289288
lastSeen,
290289
transportService.getLocalNode().getId()

‎server/src/main/java/org/opensearch/gateway/remote/ClusterStateDiffManifest.java

+19-9
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import java.io.IOException;
2727
import java.util.ArrayList;
28+
import java.util.Collections;
2829
import java.util.List;
2930
import java.util.Map;
3031
import java.util.Objects;
@@ -152,17 +153,17 @@ public ClusterStateDiffManifest(
152153
this.settingsMetadataUpdated = settingsMetadataUpdated;
153154
this.transientSettingsMetadataUpdated = transientSettingsMetadataUpdate;
154155
this.templatesMetadataUpdated = templatesMetadataUpdated;
155-
this.customMetadataUpdated = customMetadataUpdated;
156-
this.customMetadataDeleted = customMetadataDeleted;
157-
this.indicesUpdated = indicesUpdated;
158-
this.indicesDeleted = indicesDeleted;
156+
this.customMetadataUpdated = Collections.unmodifiableList(customMetadataUpdated);
157+
this.customMetadataDeleted = Collections.unmodifiableList(customMetadataDeleted);
158+
this.indicesUpdated = Collections.unmodifiableList(indicesUpdated);
159+
this.indicesDeleted = Collections.unmodifiableList(indicesDeleted);
159160
this.clusterBlocksUpdated = clusterBlocksUpdated;
160161
this.discoveryNodesUpdated = discoveryNodesUpdated;
161-
this.indicesRoutingUpdated = indicesRoutingUpdated;
162-
this.indicesRoutingDeleted = indicesRoutingDeleted;
162+
this.indicesRoutingUpdated = Collections.unmodifiableList(indicesRoutingUpdated);
163+
this.indicesRoutingDeleted = Collections.unmodifiableList(indicesRoutingDeleted);
163164
this.hashesOfConsistentSettingsUpdated = hashesOfConsistentSettingsUpdated;
164-
this.clusterStateCustomUpdated = clusterStateCustomUpdated;
165-
this.clusterStateCustomDeleted = clusterStateCustomDeleted;
165+
this.clusterStateCustomUpdated = Collections.unmodifiableList(clusterStateCustomUpdated);
166+
this.clusterStateCustomDeleted = Collections.unmodifiableList(clusterStateCustomDeleted);
166167
}
167168

168169
public ClusterStateDiffManifest(StreamInput in) throws IOException {
@@ -563,7 +564,16 @@ public static class Builder {
563564
private List<String> clusterStateCustomUpdated;
564565
private List<String> clusterStateCustomDeleted;
565566

566-
public Builder() {}
567+
public Builder() {
568+
customMetadataUpdated = Collections.emptyList();
569+
customMetadataDeleted = Collections.emptyList();
570+
indicesUpdated = Collections.emptyList();
571+
indicesDeleted = Collections.emptyList();
572+
indicesRoutingUpdated = Collections.emptyList();
573+
indicesRoutingDeleted = Collections.emptyList();
574+
clusterStateCustomUpdated = Collections.emptyList();
575+
clusterStateCustomDeleted = Collections.emptyList();
576+
}
567577

568578
public Builder fromStateUUID(String fromStateUUID) {
569579
this.fromStateUUID = fromStateUUID;

‎server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java

+19-9
Original file line numberDiff line numberDiff line change
@@ -976,7 +976,8 @@ public ClusterState getLatestClusterState(String clusterName, String clusterUUID
976976
return getClusterStateForManifest(clusterName, clusterMetadataManifest.get(), nodeId, includeEphemeral);
977977
}
978978

979-
private ClusterState readClusterStateInParallel(
979+
// package private for testing
980+
ClusterState readClusterStateInParallel(
980981
ClusterState previousState,
981982
ClusterMetadataManifest manifest,
982983
String clusterUUID,
@@ -1285,7 +1286,7 @@ public ClusterState getClusterStateForManifest(
12851286
manifest.getCustomMetadataMap(),
12861287
manifest.getCoordinationMetadata() != null,
12871288
manifest.getSettingsMetadata() != null,
1288-
manifest.getTransientSettingsMetadata() != null,
1289+
includeEphemeral && manifest.getTransientSettingsMetadata() != null,
12891290
manifest.getTemplatesMetadata() != null,
12901291
includeEphemeral && manifest.getDiscoveryNodesMetadata() != null,
12911292
includeEphemeral && manifest.getClusterBlocksMetadata() != null,
@@ -1321,13 +1322,9 @@ public ClusterState getClusterStateForManifest(
13211322

13221323
}
13231324

1324-
public ClusterState getClusterStateUsingDiff(
1325-
String clusterName,
1326-
ClusterMetadataManifest manifest,
1327-
ClusterState previousState,
1328-
String localNodeId
1329-
) throws IOException {
1330-
assert manifest.getDiffManifest() != null;
1325+
public ClusterState getClusterStateUsingDiff(ClusterMetadataManifest manifest, ClusterState previousState, String localNodeId)
1326+
throws IOException {
1327+
assert manifest.getDiffManifest() != null : "Diff manifest null which is required for downloading cluster state";
13311328
ClusterStateDiffManifest diff = manifest.getDiffManifest();
13321329
List<UploadedIndexMetadata> updatedIndices = diff.getIndicesUpdated().stream().map(idx -> {
13331330
Optional<UploadedIndexMetadata> uploadedIndexMetadataOptional = manifest.getIndices()
@@ -1586,6 +1583,19 @@ private boolean isValidClusterUUID(ClusterMetadataManifest manifest) {
15861583
return manifest.isClusterUUIDCommitted();
15871584
}
15881585

1586+
// package private setter which are required for injecting mock managers, these setters are not supposed to be used elsewhere
1587+
void setRemoteIndexMetadataManager(RemoteIndexMetadataManager remoteIndexMetadataManager) {
1588+
this.remoteIndexMetadataManager = remoteIndexMetadataManager;
1589+
}
1590+
1591+
void setRemoteGlobalMetadataManager(RemoteGlobalMetadataManager remoteGlobalMetadataManager) {
1592+
this.remoteGlobalMetadataManager = remoteGlobalMetadataManager;
1593+
}
1594+
1595+
void setRemoteClusterStateAttributesManager(RemoteClusterStateAttributesManager remoteClusterStateAttributeManager) {
1596+
this.remoteClusterStateAttributesManager = remoteClusterStateAttributeManager;
1597+
}
1598+
15891599
public void writeMetadataFailed() {
15901600
getStats().stateFailed();
15911601
}

‎server/src/test/java/org/opensearch/gateway/remote/RemoteClusterStateAttributesManagerTests.java

+21-144
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@
88

99
package org.opensearch.gateway.remote;
1010

11-
import org.opensearch.Version;
1211
import org.opensearch.action.LatchedActionListener;
13-
import org.opensearch.cluster.AbstractNamedDiffable;
1412
import org.opensearch.cluster.ClusterName;
1513
import org.opensearch.cluster.ClusterState;
1614
import org.opensearch.cluster.ClusterState.Custom;
@@ -23,11 +21,8 @@
2321
import org.opensearch.common.util.TestCapturingListener;
2422
import org.opensearch.core.action.ActionListener;
2523
import org.opensearch.core.common.io.stream.NamedWriteableRegistry;
26-
import org.opensearch.core.common.io.stream.StreamInput;
27-
import org.opensearch.core.common.io.stream.StreamOutput;
2824
import org.opensearch.core.compress.Compressor;
2925
import org.opensearch.core.compress.NoneCompressor;
30-
import org.opensearch.core.xcontent.XContentBuilder;
3126
import org.opensearch.gateway.remote.model.RemoteClusterBlocks;
3227
import org.opensearch.gateway.remote.model.RemoteClusterStateCustoms;
3328
import org.opensearch.gateway.remote.model.RemoteDiscoveryNodes;
@@ -51,6 +46,10 @@
5146
import static org.opensearch.gateway.remote.RemoteClusterStateAttributesManager.CLUSTER_STATE_ATTRIBUTE;
5247
import static org.opensearch.gateway.remote.RemoteClusterStateAttributesManager.CLUSTER_STATE_ATTRIBUTES_CURRENT_CODEC_VERSION;
5348
import static org.opensearch.gateway.remote.RemoteClusterStateAttributesManager.DISCOVERY_NODES;
49+
import static org.opensearch.gateway.remote.RemoteClusterStateTestUtils.TestClusterStateCustom1;
50+
import static org.opensearch.gateway.remote.RemoteClusterStateTestUtils.TestClusterStateCustom2;
51+
import static org.opensearch.gateway.remote.RemoteClusterStateTestUtils.TestClusterStateCustom3;
52+
import static org.opensearch.gateway.remote.RemoteClusterStateTestUtils.TestClusterStateCustom4;
5453
import static org.opensearch.gateway.remote.RemoteClusterStateUtils.CLUSTER_STATE_EPHEMERAL_PATH_TOKEN;
5554
import static org.opensearch.gateway.remote.RemoteClusterStateUtils.CLUSTER_STATE_PATH_TOKEN;
5655
import static org.opensearch.gateway.remote.RemoteClusterStateUtils.CUSTOM_DELIMITER;
@@ -338,22 +337,22 @@ public void testGetAsyncMetadataReadAction_Exception() throws IOException, Inter
338337

339338
public void testGetUpdatedCustoms() {
340339
Map<String, ClusterState.Custom> previousCustoms = Map.of(
341-
TestCustom1.TYPE,
342-
new TestCustom1("data1"),
343-
TestCustom2.TYPE,
344-
new TestCustom2("data2"),
345-
TestCustom3.TYPE,
346-
new TestCustom3("data3")
340+
TestClusterStateCustom1.TYPE,
341+
new TestClusterStateCustom1("data1"),
342+
TestClusterStateCustom2.TYPE,
343+
new TestClusterStateCustom2("data2"),
344+
TestClusterStateCustom3.TYPE,
345+
new TestClusterStateCustom3("data3")
347346
);
348347
ClusterState previousState = ClusterState.builder(new ClusterName("test-cluster")).customs(previousCustoms).build();
349348

350349
Map<String, Custom> currentCustoms = Map.of(
351-
TestCustom2.TYPE,
352-
new TestCustom2("data2"),
353-
TestCustom3.TYPE,
354-
new TestCustom3("data3-changed"),
355-
TestCustom4.TYPE,
356-
new TestCustom4("data4")
350+
TestClusterStateCustom2.TYPE,
351+
new TestClusterStateCustom2("data2"),
352+
TestClusterStateCustom3.TYPE,
353+
new TestClusterStateCustom3("data3-changed"),
354+
TestClusterStateCustom4.TYPE,
355+
new TestClusterStateCustom4("data4")
357356
);
358357

359358
ClusterState currentState = ClusterState.builder(new ClusterName("test-cluster")).customs(currentCustoms).build();
@@ -368,136 +367,14 @@ public void testGetUpdatedCustoms() {
368367
assertThat(customsDiff.getDeletes(), is(Collections.emptyList()));
369368

370369
Map<String, ClusterState.Custom> expectedCustoms = Map.of(
371-
TestCustom3.TYPE,
372-
new TestCustom3("data3-changed"),
373-
TestCustom4.TYPE,
374-
new TestCustom4("data4")
370+
TestClusterStateCustom3.TYPE,
371+
new TestClusterStateCustom3("data3-changed"),
372+
TestClusterStateCustom4.TYPE,
373+
new TestClusterStateCustom4("data4")
375374
);
376375

377376
customsDiff = remoteClusterStateAttributesManager.getUpdatedCustoms(currentState, previousState, true, false);
378377
assertThat(customsDiff.getUpserts(), is(expectedCustoms));
379-
assertThat(customsDiff.getDeletes(), is(List.of(TestCustom1.TYPE)));
380-
}
381-
382-
private static abstract class AbstractTestCustom extends AbstractNamedDiffable<Custom> implements ClusterState.Custom {
383-
384-
private final String value;
385-
386-
AbstractTestCustom(String value) {
387-
this.value = value;
388-
}
389-
390-
AbstractTestCustom(StreamInput in) throws IOException {
391-
this.value = in.readString();
392-
}
393-
394-
@Override
395-
public Version getMinimalSupportedVersion() {
396-
return Version.CURRENT;
397-
}
398-
399-
@Override
400-
public void writeTo(StreamOutput out) throws IOException {
401-
out.writeString(value);
402-
}
403-
404-
@Override
405-
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
406-
return builder;
407-
}
408-
409-
@Override
410-
public boolean isPrivate() {
411-
return true;
412-
}
413-
414-
@Override
415-
public boolean equals(Object o) {
416-
if (this == o) return true;
417-
if (o == null || getClass() != o.getClass()) return false;
418-
419-
AbstractTestCustom that = (AbstractTestCustom) o;
420-
421-
if (!value.equals(that.value)) return false;
422-
423-
return true;
424-
}
425-
426-
@Override
427-
public int hashCode() {
428-
return value.hashCode();
429-
}
430-
}
431-
432-
private static class TestCustom1 extends AbstractTestCustom {
433-
434-
private static final String TYPE = "custom_1";
435-
436-
TestCustom1(String value) {
437-
super(value);
438-
}
439-
440-
TestCustom1(StreamInput in) throws IOException {
441-
super(in);
442-
}
443-
444-
@Override
445-
public String getWriteableName() {
446-
return TYPE;
447-
}
448-
}
449-
450-
private static class TestCustom2 extends AbstractTestCustom {
451-
452-
private static final String TYPE = "custom_2";
453-
454-
TestCustom2(String value) {
455-
super(value);
456-
}
457-
458-
TestCustom2(StreamInput in) throws IOException {
459-
super(in);
460-
}
461-
462-
@Override
463-
public String getWriteableName() {
464-
return TYPE;
465-
}
466-
}
467-
468-
private static class TestCustom3 extends AbstractTestCustom {
469-
470-
private static final String TYPE = "custom_3";
471-
472-
TestCustom3(String value) {
473-
super(value);
474-
}
475-
476-
TestCustom3(StreamInput in) throws IOException {
477-
super(in);
478-
}
479-
480-
@Override
481-
public String getWriteableName() {
482-
return TYPE;
483-
}
484-
}
485-
486-
private static class TestCustom4 extends AbstractTestCustom {
487-
488-
private static final String TYPE = "custom_4";
489-
490-
TestCustom4(String value) {
491-
super(value);
492-
}
493-
494-
TestCustom4(StreamInput in) throws IOException {
495-
super(in);
496-
}
497-
498-
@Override
499-
public String getWriteableName() {
500-
return TYPE;
501-
}
378+
assertThat(customsDiff.getDeletes(), is(List.of(TestClusterStateCustom1.TYPE)));
502379
}
503380
}

0 commit comments

Comments
 (0)