Skip to content

Commit 2144e22

Browse files
atrisatris
committed
Refactor Virtual Shards to use range-based routing
Signed-off-by: Atri Sharma <atri.jiit@gmail.com>
1 parent 2ccbc63 commit 2144e22

File tree

4 files changed

+78
-28
lines changed

4 files changed

+78
-28
lines changed

‎server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2210,16 +2210,29 @@ public IndexMetadata build() {
22102210
}
22112211

22122212
final int numberOfVirtualShards = INDEX_NUMBER_OF_VIRTUAL_SHARDS_SETTING.get(settings);
2213-
if (numberOfVirtualShards != -1 && numberOfVirtualShards < numberOfShards) {
2214-
throw new IllegalArgumentException(
2215-
"number of virtual shards ["
2216-
+ numberOfVirtualShards
2217-
+ "] must be >= number of shards ["
2218-
+ numberOfShards
2219-
+ "] for ["
2220-
+ index
2221-
+ "]"
2222-
);
2213+
if (numberOfVirtualShards != -1) {
2214+
if (numberOfVirtualShards < numberOfShards) {
2215+
throw new IllegalArgumentException(
2216+
"number of virtual shards ["
2217+
+ numberOfVirtualShards
2218+
+ "] must be >= number of shards ["
2219+
+ numberOfShards
2220+
+ "] for ["
2221+
+ index
2222+
+ "]"
2223+
);
2224+
}
2225+
if (numberOfVirtualShards % numberOfShards != 0) {
2226+
throw new IllegalArgumentException(
2227+
"number of virtual shards ["
2228+
+ numberOfVirtualShards
2229+
+ "] must be a multiple of number of shards ["
2230+
+ numberOfShards
2231+
+ "] for ["
2232+
+ index
2233+
+ "]"
2234+
);
2235+
}
22232236
}
22242237

22252238
// fill missing slots in inSyncAllocationIds with empty set if needed and make all entries immutable

‎server/src/main/java/org/opensearch/cluster/metadata/VirtualShardRoutingHelper.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515

1616
/**
1717
* Resolves virtual shard routing to physical shard IDs.
18-
*
19-
* @opensearch.api
2018
*/
2119

2220
public final class VirtualShardRoutingHelper {
@@ -54,6 +52,7 @@ public static int resolvePhysicalShardId(IndexMetadata indexMetadata, int vShard
5452
}
5553
}
5654

57-
return Math.floorMod(vShardId, indexMetadata.getNumberOfShards());
55+
int virtualShardsPerPhysical = indexMetadata.getNumberOfVirtualShards() / indexMetadata.getNumberOfShards();
56+
return vShardId / virtualShardsPerPhysical;
5857
}
5958
}

‎server/src/test/java/org/opensearch/cluster/metadata/VirtualShardRoutingHelperTests.java‎

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,55 +17,76 @@
1717

1818
public class VirtualShardRoutingHelperTests extends OpenSearchTestCase {
1919

20-
public void testResolvePhysicalShardIdDefaultModulo() {
20+
public void testResolvePhysicalShardIdDefaultRangeBased() {
2121
int numPhysicalShards = 5;
2222
IndexMetadata metadata = IndexMetadata.builder("test")
23-
.settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT))
23+
.settings(
24+
Settings.builder()
25+
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
26+
.put(IndexMetadata.SETTING_NUMBER_OF_VIRTUAL_SHARDS, 20)
27+
)
2428
.numberOfShards(numPhysicalShards)
2529
.numberOfReplicas(1)
2630
.build();
2731

2832
assertEquals(0, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 0));
29-
assertEquals(2, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 7));
30-
assertEquals(4, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 4));
31-
assertEquals(0, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 5));
33+
assertEquals(0, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 3));
34+
assertEquals(1, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 4));
35+
assertEquals(1, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 7));
36+
assertEquals(4, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 19));
3237
}
3338

3439
public void testResolvePhysicalShardIdWithOverrides() {
3540
int numPhysicalShards = 5;
3641
Map<String, String> overrides = new HashMap<>();
37-
overrides.put("7", "1");
42+
overrides.put("7", "1"); // mapped out of standard routing
43+
overrides.put("8", "2"); // mapped out of standard routing
3844

3945
IndexMetadata.Builder builder = IndexMetadata.builder("test")
40-
.settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT))
46+
.settings(
47+
Settings.builder()
48+
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
49+
.put(IndexMetadata.SETTING_NUMBER_OF_VIRTUAL_SHARDS, 20)
50+
)
4151
.numberOfShards(numPhysicalShards)
4252
.numberOfReplicas(1);
4353
builder.putCustom(VirtualShardRoutingHelper.VIRTUAL_SHARDS_CUSTOM_METADATA_KEY, overrides);
4454

4555
IndexMetadata metadata = builder.build();
4656

4757
assertEquals(1, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 7));
58+
assertEquals(2, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 8));
4859

60+
// Default falls back to range-based formula (20 / 5 = 4 vshards per pshard)
4961
assertEquals(0, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 0));
62+
assertEquals(2, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 9));
5063
}
5164

52-
public void testInvalidOverridesFallBackToModulo() {
65+
public void testInvalidOverridesFallBackToRangeBased() {
5366
int numPhysicalShards = 5;
5467
Map<String, String> overrides = new HashMap<>();
5568
overrides.put("7", "not_a_number");
5669
overrides.put("8", "-1");
57-
overrides.put("9", "5");
70+
overrides.put("19", "5");
5871

5972
IndexMetadata.Builder builder = IndexMetadata.builder("test")
60-
.settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT))
73+
.settings(
74+
Settings.builder()
75+
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
76+
.put(IndexMetadata.SETTING_NUMBER_OF_VIRTUAL_SHARDS, 20)
77+
)
6178
.numberOfShards(numPhysicalShards)
6279
.numberOfReplicas(1);
6380
builder.putCustom(VirtualShardRoutingHelper.VIRTUAL_SHARDS_CUSTOM_METADATA_KEY, overrides);
6481

6582
IndexMetadata metadata = builder.build();
6683

67-
assertEquals(2, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 7));
68-
assertEquals(3, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 8));
69-
assertEquals(4, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 9));
84+
// Standard range-based routing expects 4 vshards per physical shard
85+
// vShard 7 -> 7 / 4 = 1
86+
assertEquals(1, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 7));
87+
// vShard 8 -> 8 / 4 = 2
88+
assertEquals(2, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 8));
89+
// vShard 19 -> 19 / 4 = 4
90+
assertEquals(4, VirtualShardRoutingHelper.resolvePhysicalShardId(metadata, 19));
7091
}
7192
}

‎server/src/test/java/org/opensearch/cluster/routing/VirtualShardOperationRoutingTests.java‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public void testGenerateVirtualShardId() {
3232
String routing = "user1";
3333
int routingHash = Murmur3HashFunction.hash(routing);
3434
int expectedVShard = Math.floorMod(routingHash, numVirtualShards);
35-
int expectedPShard = Math.floorMod(expectedVShard, numPhysicalShards);
35+
int expectedPShard = expectedVShard / (numVirtualShards / numPhysicalShards);
3636

3737
assertEquals(expectedPShard, OperationRouting.generateShardId(metadata, "doc1", routing));
3838
}
@@ -75,6 +75,23 @@ public void testVirtualShardValidation() {
7575
);
7676

7777
assertTrue(e.getMessage().contains("must be >= number of shards"));
78+
79+
int numVirtualShardsInvalid = 15;
80+
81+
IllegalArgumentException e2 = expectThrows(
82+
IllegalArgumentException.class,
83+
() -> IndexMetadata.builder("test2")
84+
.settings(
85+
Settings.builder()
86+
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
87+
.put(IndexMetadata.SETTING_NUMBER_OF_VIRTUAL_SHARDS, numVirtualShardsInvalid)
88+
)
89+
.numberOfShards(numPhysicalShards)
90+
.numberOfReplicas(1)
91+
.build()
92+
);
93+
94+
assertTrue(e2.getMessage().contains("must be a multiple of number of shards"));
7895
}
7996

8097
public void testVirtualShardWithRoutingPartition() {
@@ -99,7 +116,7 @@ public void testVirtualShardWithRoutingPartition() {
99116
int partitionOffset = Math.floorMod(Murmur3HashFunction.hash(id), partitionSize);
100117
int routingHash = Murmur3HashFunction.hash(routing) + partitionOffset;
101118
int expectedVShard = Math.floorMod(routingHash, numVirtualShards);
102-
int expectedPShard = Math.floorMod(expectedVShard, numPhysicalShards);
119+
int expectedPShard = expectedVShard / (numVirtualShards / numPhysicalShards);
103120

104121
assertEquals(expectedPShard, OperationRouting.generateShardId(metadata, id, routing));
105122
}

0 commit comments

Comments
 (0)