Skip to content

Fixed a flaky test that is order dependent #19762

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+30 −4
Open

Fixed a flaky test that is order dependent #19762

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 30 additions & 4 deletions ...ava/org/opensearch/cluster/routing/allocation/decider/ShardsLimitAllocationDeciderIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;

import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
Expand Down Expand Up @@ -233,8 +234,16 @@ public void testIndexSpecificShardLimit() {
}
}

public void testCombinedClusterAndIndexSpecificShardLimits() {
public void testCombinedClusterAndIndexSpecificShardLimits() throws Exception {
startTestNodes(3);

// Keep disk thresholds from interfering (use typed Setting, not raw String)
client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().put("cluster.routing.allocation.disk.threshold_enabled", false).build())
.get();

// Set the cluster-wide shard limit to 6
updateClusterSetting(getShardsPerNodeKey(false), 6);

Expand All @@ -247,6 +256,24 @@ public void testCombinedClusterAndIndexSpecificShardLimits() {
.build();
createIndex("test1", indexSettingsWithLimit);

// Ensure test1 primaries are placed before adding other indices (prevents starvation)
assertBusy(() -> {
ClusterState s = client().admin().cluster().prepareState().get().getState();
int primariesStarted = 0, unassigned = 0;
for (IndexRoutingTable irt : s.getRoutingTable()) {
if (irt.getIndex().getName().equals("test1")) {
for (IndexShardRoutingTable isrt : irt) {
for (ShardRouting sr : isrt) {
if (sr.primary() && sr.started()) primariesStarted++;
if (sr.unassigned()) unassigned++;
}
}
}
}
assertEquals(3, primariesStarted); // 3 primaries started
assertEquals(3, unassigned); // 3 unassigned (the replicas)
});
Comment on lines +259 to +275
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you replace this with a call to the ensureYellow("test1") helper method in the parent test class? The index should be red until all primaries are assigned, and will be yellow if replicas are unassigned.


// Create the second index with 4 shards and 1 replica
createIndex(
"test2",
Expand Down Expand Up @@ -309,12 +336,11 @@ public void testCombinedClusterAndIndexSpecificShardLimits() {
// Check shard distribution across nodes
List<Integer> shardCounts = new ArrayList<>(nodeShardCounts.values());
Collections.sort(shardCounts, Collections.reverseOrder());

assertEquals("Two nodes should have 6 shards", 6, shardCounts.get(0).intValue());
assertEquals("Two nodes should have 6 shards", 6, shardCounts.get(1).intValue());
assertEquals("One node should have 5 shards", 5, shardCounts.get(2).intValue());

// Check that all nodes have only one shard of the first index
});
}, 30, TimeUnit.SECONDS);
} catch (Exception e) {
throw new RuntimeException(e);
}
Expand Down
Loading