Skip to content

Commit 7247266

Browse files
BukhtawarShailendra Singh
Bukhtawar
and
Shailendra Singh
authored
Simplify diff calculation for remote routing table (#15100)
Simplify diff calculation logic for remote routing table Signed-off-by: Bukhtawar Khan <[email protected]> Co-authored-by: Shailendra Singh <[email protected]>
1 parent 6bae704 commit 7247266

20 files changed

+975
-530
lines changed

‎server/src/internalClusterTest/java/org/opensearch/gateway/remote/RemoteRoutingTableServiceIT.java

Lines changed: 80 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.opensearch.test.OpenSearchIntegTestCase;
2525
import org.junit.Before;
2626

27+
import java.io.IOException;
2728
import java.nio.charset.StandardCharsets;
2829
import java.nio.file.Path;
2930
import java.util.ArrayList;
@@ -46,7 +47,7 @@
4647
public class RemoteRoutingTableServiceIT extends RemoteStoreBaseIntegTestCase {
4748
private static final String INDEX_NAME = "test-index";
4849
private static final String INDEX_NAME_1 = "test-index-1";
49-
BlobPath indexRoutingPath;
50+
List<BlobPath> indexRoutingPaths;
5051
AtomicInteger indexRoutingFiles = new AtomicInteger();
5152
private final RemoteStoreEnums.PathType pathType = RemoteStoreEnums.PathType.HASHED_PREFIX;
5253

@@ -91,7 +92,7 @@ public void testRemoteRoutingTableIndexLifecycle() throws Exception {
9192
updateIndexSettings(INDEX_NAME, IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2);
9293
ensureGreen(INDEX_NAME);
9394
assertBusy(() -> {
94-
int indexRoutingFilesAfterUpdate = repository.blobStore().blobContainer(indexRoutingPath).listBlobs().size();
95+
int indexRoutingFilesAfterUpdate = repository.blobStore().blobContainer(indexRoutingPaths.get(0)).listBlobs().size();
9596
// At-least 3 new index routing files will be created as shards will transition from INIT -> UNASSIGNED -> STARTED state
9697
assertTrue(indexRoutingFilesAfterUpdate >= indexRoutingFiles.get() + 3);
9798
});
@@ -112,6 +113,47 @@ public void testRemoteRoutingTableIndexLifecycle() throws Exception {
112113
assertTrue(areRoutingTablesSame(routingTableVersions));
113114
}
114115

116+
public void testRemoteRoutingTableWithMultipleIndex() throws Exception {
117+
BlobStoreRepository repository = prepareClusterAndVerifyRepository();
118+
119+
RemoteClusterStateService remoteClusterStateService = internalCluster().getClusterManagerNodeInstance(
120+
RemoteClusterStateService.class
121+
);
122+
RemoteManifestManager remoteManifestManager = remoteClusterStateService.getRemoteManifestManager();
123+
Optional<ClusterMetadataManifest> latestManifest = remoteManifestManager.getLatestClusterMetadataManifest(
124+
getClusterState().getClusterName().value(),
125+
getClusterState().getMetadata().clusterUUID()
126+
);
127+
List<String> expectedIndexNames = new ArrayList<>();
128+
List<String> deletedIndexNames = new ArrayList<>();
129+
verifyUpdatesInManifestFile(latestManifest, expectedIndexNames, 1, deletedIndexNames, true);
130+
131+
List<RoutingTable> routingTables = getRoutingTableFromAllNodes();
132+
// Verify indices in routing table
133+
Set<String> expectedIndicesInRoutingTable = Set.of(INDEX_NAME);
134+
assertEquals(routingTables.get(0).getIndicesRouting().keySet(), expectedIndicesInRoutingTable);
135+
// Verify routing table across all nodes is equal
136+
assertTrue(areRoutingTablesSame(routingTables));
137+
138+
// Create new index
139+
createIndex(INDEX_NAME_1, remoteStoreIndexSettings(1, 5));
140+
ensureGreen(INDEX_NAME_1);
141+
142+
latestManifest = remoteManifestManager.getLatestClusterMetadataManifest(
143+
getClusterState().getClusterName().value(),
144+
getClusterState().getMetadata().clusterUUID()
145+
);
146+
147+
updateIndexRoutingPaths(repository);
148+
verifyUpdatesInManifestFile(latestManifest, expectedIndexNames, 2, deletedIndexNames, true);
149+
routingTables = getRoutingTableFromAllNodes();
150+
// Verify indices in routing table
151+
expectedIndicesInRoutingTable = Set.of(INDEX_NAME, INDEX_NAME_1);
152+
assertEquals(routingTables.get(0).getIndicesRouting().keySet(), expectedIndicesInRoutingTable);
153+
// Verify routing table across all nodes is equal
154+
assertTrue(areRoutingTablesSame(routingTables));
155+
}
156+
115157
public void testRemoteRoutingTableEmptyRoutingTableDiff() throws Exception {
116158
prepareClusterAndVerifyRepository();
117159

@@ -166,7 +208,7 @@ public void testRemoteRoutingTableIndexNodeRestart() throws Exception {
166208
assertRemoteStoreRepositoryOnAllNodes(REMOTE_ROUTING_TABLE_REPO);
167209

168210
assertBusy(() -> {
169-
int indexRoutingFilesAfterNodeDrop = repository.blobStore().blobContainer(indexRoutingPath).listBlobs().size();
211+
int indexRoutingFilesAfterNodeDrop = repository.blobStore().blobContainer(indexRoutingPaths.get(0)).listBlobs().size();
170212
assertTrue(indexRoutingFilesAfterNodeDrop > indexRoutingFiles.get());
171213
});
172214

@@ -201,7 +243,7 @@ public void testRemoteRoutingTableIndexMasterRestart() throws Exception {
201243
assertRemoteStoreRepositoryOnAllNodes(REMOTE_ROUTING_TABLE_REPO);
202244

203245
assertBusy(() -> {
204-
int indexRoutingFilesAfterNodeDrop = repository.blobStore().blobContainer(indexRoutingPath).listBlobs().size();
246+
int indexRoutingFilesAfterNodeDrop = repository.blobStore().blobContainer(indexRoutingPaths.get(0)).listBlobs().size();
205247
assertTrue(indexRoutingFilesAfterNodeDrop > indexRoutingFiles.get());
206248
});
207249

@@ -240,10 +282,14 @@ private BlobStoreRepository prepareClusterAndVerifyRepository() throws Exception
240282

241283
BlobPath baseMetadataPath = getBaseMetadataPath(repository);
242284
List<IndexRoutingTable> indexRoutingTables = new ArrayList<>(getClusterState().routingTable().indicesRouting().values());
243-
indexRoutingPath = getIndexRoutingPath(baseMetadataPath.add(INDEX_ROUTING_TABLE), indexRoutingTables.get(0).getIndex().getUUID());
285+
indexRoutingPaths = new ArrayList<>();
286+
for (IndexRoutingTable indexRoutingTable : indexRoutingTables) {
287+
indexRoutingPaths.add(getIndexRoutingPath(baseMetadataPath.add(INDEX_ROUTING_TABLE), indexRoutingTable.getIndex().getUUID()));
288+
}
244289

245290
assertBusy(() -> {
246-
indexRoutingFiles.set(repository.blobStore().blobContainer(indexRoutingPath).listBlobs().size());
291+
int totalRoutingFiles = calculateTotalRoutingFiles(repository);
292+
indexRoutingFiles.set(totalRoutingFiles);
247293
// There would be >=3 files as shards will transition from UNASSIGNED -> INIT -> STARTED state
248294
assertTrue(indexRoutingFiles.get() >= 3);
249295
});
@@ -280,11 +326,19 @@ private void verifyUpdatesInManifestFile(
280326
assertTrue(latestManifest.isPresent());
281327
ClusterMetadataManifest manifest = latestManifest.get();
282328

283-
assertEquals(expectedIndexNames, manifest.getDiffManifest().getIndicesRoutingUpdated());
284329
assertEquals(expectedDeletedIndex, manifest.getDiffManifest().getIndicesDeleted());
285330
assertEquals(expectedIndicesRoutingFilesInManifest, manifest.getIndicesRouting().size());
331+
332+
// Check if all paths in manifest.getIndicesRouting() are present in indexRoutingPaths
286333
for (ClusterMetadataManifest.UploadedIndexMetadata uploadedFilename : manifest.getIndicesRouting()) {
287-
assertTrue(uploadedFilename.getUploadedFilename().contains(indexRoutingPath.buildAsString()));
334+
boolean pathFound = false;
335+
for (BlobPath indexRoutingPath : indexRoutingPaths) {
336+
if (uploadedFilename.getUploadedFilename().contains(indexRoutingPath.buildAsString())) {
337+
pathFound = true;
338+
break;
339+
}
340+
}
341+
assertTrue("Uploaded file not found in indexRoutingPaths: " + uploadedFilename.getUploadedFilename(), pathFound);
288342
}
289343
assertEquals(isRoutingTableDiffFileExpected, manifest.getDiffManifest().getIndicesRoutingDiffPath() != null);
290344
}
@@ -305,6 +359,24 @@ private List<RoutingTable> getRoutingTableFromAllNodes() throws ExecutionExcepti
305359
return routingTables;
306360
}
307361

362+
private void updateIndexRoutingPaths(BlobStoreRepository repository) {
363+
BlobPath baseMetadataPath = getBaseMetadataPath(repository);
364+
List<IndexRoutingTable> indexRoutingTables = new ArrayList<>(getClusterState().routingTable().indicesRouting().values());
365+
366+
indexRoutingPaths.clear(); // Clear the list to avoid stale data
367+
for (IndexRoutingTable indexRoutingTable : indexRoutingTables) {
368+
indexRoutingPaths.add(getIndexRoutingPath(baseMetadataPath.add(INDEX_ROUTING_TABLE), indexRoutingTable.getIndex().getUUID()));
369+
}
370+
}
371+
372+
private int calculateTotalRoutingFiles(BlobStoreRepository repository) throws IOException {
373+
int totalRoutingFiles = 0;
374+
for (BlobPath path : indexRoutingPaths) {
375+
totalRoutingFiles += repository.blobStore().blobContainer(path).listBlobs().size();
376+
}
377+
return totalRoutingFiles;
378+
}
379+
308380
private boolean areRoutingTablesSame(List<RoutingTable> routingTables) {
309381
if (routingTables == null || routingTables.isEmpty()) {
310382
return false;
@@ -356,7 +428,6 @@ private void deleteIndexAndVerify(RemoteManifestManager remoteManifestManager) {
356428
);
357429
assertTrue(latestManifest.isPresent());
358430
ClusterMetadataManifest manifest = latestManifest.get();
359-
assertTrue(manifest.getDiffManifest().getIndicesRoutingUpdated().isEmpty());
360431
assertTrue(manifest.getDiffManifest().getIndicesDeleted().contains(INDEX_NAME));
361432
assertTrue(manifest.getIndicesRouting().isEmpty());
362433
}

‎server/src/main/java/org/opensearch/cluster/routing/IndexShardRoutingTable.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434

3535
import org.apache.logging.log4j.LogManager;
3636
import org.apache.logging.log4j.Logger;
37+
import org.opensearch.cluster.AbstractDiffable;
38+
import org.opensearch.cluster.Diff;
3739
import org.opensearch.cluster.node.DiscoveryNode;
3840
import org.opensearch.cluster.node.DiscoveryNodes;
3941
import org.opensearch.common.Nullable;
@@ -75,7 +77,7 @@
7577
* @opensearch.api
7678
*/
7779
@PublicApi(since = "1.0.0")
78-
public class IndexShardRoutingTable implements Iterable<ShardRouting> {
80+
public class IndexShardRoutingTable extends AbstractDiffable<IndexShardRoutingTable> implements Iterable<ShardRouting> {
7981

8082
final ShardShuffler shuffler;
8183
// Shuffler for weighted round-robin shard routing. This uses rotation to permute shards.
@@ -545,6 +547,12 @@ private static List<ShardRouting> rankShardsAndUpdateStats(
545547
return sortedShards;
546548
}
547549

550+
@Override
551+
public void writeTo(StreamOutput out) throws IOException {
552+
this.shardId().getIndex().writeTo(out);
553+
Builder.writeToThin(this, out);
554+
}
555+
548556
private static class NodeRankComparator implements Comparator<ShardRouting> {
549557
private final Map<String, Double> nodeRanks;
550558

@@ -1067,6 +1075,14 @@ private void populateInitializingShardWeightsMap(WeightedRouting weightedRouting
10671075
}
10681076
}
10691077

1078+
public static IndexShardRoutingTable readFrom(StreamInput in) throws IOException {
1079+
return IndexShardRoutingTable.Builder.readFrom(in);
1080+
}
1081+
1082+
public static Diff<IndexShardRoutingTable> readDiffFrom(StreamInput in) throws IOException {
1083+
return readDiffFrom(IndexShardRoutingTable::readFrom, in);
1084+
}
1085+
10701086
/**
10711087
* Builder of an index shard routing table.
10721088
*

‎server/src/main/java/org/opensearch/cluster/routing/RoutingTable.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,10 @@ public Diff<RoutingTable> diff(RoutingTable previousState) {
378378
return new RoutingTableDiff(previousState, this);
379379
}
380380

381+
public Diff<RoutingTable> incrementalDiff(RoutingTable previousState) {
382+
return new RoutingTableIncrementalDiff(previousState, this);
383+
}
384+
381385
public static Diff<RoutingTable> readDiffFrom(StreamInput in) throws IOException {
382386
return new RoutingTableDiff(in);
383387
}
@@ -403,7 +407,7 @@ public void writeTo(StreamOutput out) throws IOException {
403407
}
404408
}
405409

406-
private static class RoutingTableDiff implements Diff<RoutingTable> {
410+
private static class RoutingTableDiff implements Diff<RoutingTable>, StringKeyDiffProvider<IndexRoutingTable> {
407411

408412
private final long version;
409413

@@ -432,6 +436,11 @@ public void writeTo(StreamOutput out) throws IOException {
432436
out.writeLong(version);
433437
indicesRouting.writeTo(out);
434438
}
439+
440+
@Override
441+
public DiffableUtils.MapDiff<String, IndexRoutingTable, Map<String, IndexRoutingTable>> provideDiff() {
442+
return (DiffableUtils.MapDiff<String, IndexRoutingTable, Map<String, IndexRoutingTable>>) indicesRouting;
443+
}
435444
}
436445

437446
public static Builder builder() {

0 commit comments

Comments
 (0)