Skip to content

Account for simulated utilization threshold in WriteLoadConstraintDeciderIT #133894

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

Merged
merged 6 commits into from
Sep 3, 2025
Merged

Account for simulated utilization threshold in WriteLoadConstraintDeciderIT #133894

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d9b45d4
Fix WriteLoadConstraintDeciderIT
nicktindall Sep 1, 2025
f506534
Remove debug logging
nicktindall Sep 1, 2025
ba15f8f
Reduce minimum write pool threads, adjust shard write load range acco…
nicktindall Sep 3, 2025
c995fb7
Merge remote-tracking branch 'origin/main' into fix_WriteLoadConstrai…
nicktindall Sep 3, 2025
946469e
Add utility for converting write load to utilization
nicktindall Sep 3, 2025
dc48fbb
Fix integer division issue
nicktindall Sep 3, 2025
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
3 changes: 0 additions & 3 deletions muted-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,9 +549,6 @@ tests:
- class: org.elasticsearch.xpack.esql.qa.single_node.EsqlClientYamlAsyncIT
method: test {p0=esql/60_usage/Basic ESQL usage output (telemetry) non-snapshot version}
issue: https://github.com/elastic/elasticsearch/issues/133842
- class: org.elasticsearch.cluster.routing.allocation.decider.WriteLoadConstraintDeciderIT
method: testHighNodeWriteLoadPreventsNewShardAllocation
issue: https://github.com/elastic/elasticsearch/issues/133857
- class: org.elasticsearch.xpack.kql.parser.KqlParserBooleanQueryTests
method: testParseOrQuery
issue: https://github.com/elastic/elasticsearch/issues/133863
Expand Down
21 changes: 13 additions & 8 deletions ...va/org/elasticsearch/cluster/routing/allocation/decider/WriteLoadConstraintDeciderIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ protected Collection<Class<? extends Plugin>> getMockPlugins() {
*/
public void testHighNodeWriteLoadPreventsNewShardAllocation() {
int randomUtilizationThresholdPercent = randomIntBetween(50, 100);
int numberOfWritePoolThreads = randomIntBetween(10, 20);
Copy link
Member

Choose a reason for hiding this comment

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

I think the lower bound of the randomization should be 2 since that's a realistic setup.

Copy link
Contributor

Choose a reason for hiding this comment

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

Lowering the thread count might result in the shards being unable to fit on a single node, I expect. Not sure how much we care about realism. We could lower the shardWriteLoad to 0.001, to remove that concern -- and give the threshold randomization a bigger range of values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in ba15f8f

double shardWriteLoad = randomDoubleBetween(0.0, 0.2, true);
Copy link
Contributor Author

@nicktindall nicktindall Sep 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Would probably be a lot easier to reason about if shard write loads were zero (then there would be no risk of the decider preventing the movement because it would overload the target), but also less realistic.

Copy link
Contributor

Choose a reason for hiding this comment

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

The only constraint for the shard write load in this test is that it is greater than zero so that the WriteLoadDecider considers it. So we could use a very small shard write load. I didn't originally realize the simulator factor when writing this test, clearly :) So I just picked a random value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh of course... I changed lowerInclusive to false in ba15f8f to ensure we don't get zeros

Settings settings = Settings.builder()
.put(
WriteLoadConstraintSettings.WRITE_LOAD_DECIDER_ENABLED_SETTING.getKey(),
Expand Down Expand Up @@ -115,7 +117,11 @@ public void testHighNodeWriteLoadPreventsNewShardAllocation() {
);

String indexName = randomIdentifier();
int randomNumberOfShards = randomIntBetween(15, 40); // Pick a high number of shards, so it is clear assignment is not accidental.
int randomNumberOfShards = randomIntBetween(10, 20); // Pick a high number of shards, so it is clear assignment is not accidental.

// Calculate the maximum utilization a node can report while still being able to accept all relocating shards
double additionalLoadFromAllShards = (shardWriteLoad * randomNumberOfShards) / numberOfWritePoolThreads;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you make this a method helper in this test file -- takes total shard write load, number of threads, returns needed overheard?

I lean towards making this sort of calculation a static method on the ShardMovementWriteLoadSimulator, because it's fragile to simulator calculation changes, but of more immediately interest is we will likely repeat this calculation for future testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a bit basic, but I added it here. Perhaps having a name for it makes it clearer what's going on 946469e

int maxUtilizationPercent = randomUtilizationThresholdPercent - (int) (additionalLoadFromAllShards * 100) - 1;

var verifyAssignmentToFirstNodeListener = ClusterServiceUtils.addMasterTemporaryStateListener(clusterState -> {
var indexRoutingTable = clusterState.routingTable().index(indexName);
Expand Down Expand Up @@ -154,19 +160,19 @@ public void testHighNodeWriteLoadPreventsNewShardAllocation() {
final DiscoveryNode thirdDiscoveryNode = getDiscoveryNode(thirdDataNodeName);
final NodeUsageStatsForThreadPools firstNodeNonHotSpottingNodeStats = createNodeUsageStatsForThreadPools(
firstDiscoveryNode,
2,
0.5f,
numberOfWritePoolThreads,
randomIntBetween(0, maxUtilizationPercent) / 100f,
0
);
final NodeUsageStatsForThreadPools secondNodeNonHotSpottingNodeStats = createNodeUsageStatsForThreadPools(
secondDiscoveryNode,
2,
0.5f,
numberOfWritePoolThreads,
randomIntBetween(0, maxUtilizationPercent) / 100f,
0
);
final NodeUsageStatsForThreadPools thirdNodeHotSpottingNodeStats = createNodeUsageStatsForThreadPools(
thirdDiscoveryNode,
2,
numberOfWritePoolThreads,
randomUtilizationThresholdPercent + 1 / 100,
Copy link
Member

Choose a reason for hiding this comment

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

Should this be randomIntBetween(0, maxUtilizationPercent) / 100f, as well?

Copy link
Contributor Author

@nicktindall nicktindall Sep 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

No, the test works by

  1. Create an index while cluster.routing.allocation.exclude._name = "secondDataNode, thirdDataNode" (all shards are assigned to firstDataNode)
  2. Mock the TransportNodeUsageStatsForThreadPoolsAction on the three data nodes such that the first and second nodes report being under the utilisation threshold, and the third node reports being over the utilisation threshold.
    • Also mock the shard write load responses (random between 0 and something, I don't think this is critical to the test)
  3. Refresh cluster info
  4. Update cluster.routing.allocation.exclude._name = "firstDataNode"
  5. Wait to see all the shards relocated to secondDataNode because first is excluded, and third is over the utilisation threshold.

So we have to have it over the threshold here.

0
);
Expand Down Expand Up @@ -197,12 +203,11 @@ public void testHighNodeWriteLoadPreventsNewShardAllocation() {
.getMetadata()
.getProject()
.index(indexName);
double shardWriteLoadDefault = 0.2;
MockTransportService.getInstance(firstDataNodeName)
.addRequestHandlingBehavior(IndicesStatsAction.NAME + "[n]", (handler, request, channel, task) -> {
List<ShardStats> shardStats = new ArrayList<>(indexMetadata.getNumberOfShards());
for (int i = 0; i < indexMetadata.getNumberOfShards(); i++) {
shardStats.add(createShardStats(indexMetadata, i, shardWriteLoadDefault, firstDataNodeId));
shardStats.add(createShardStats(indexMetadata, i, shardWriteLoad, firstDataNodeId));
}
TransportIndicesStatsAction instance = internalCluster().getInstance(TransportIndicesStatsAction.class, firstDataNodeName);
channel.sendResponse(instance.new NodeResponse(firstDataNodeId, indexMetadata.getNumberOfShards(), shardStats, List.of()));
Expand Down