Account for simulated utilization threshold in WriteLoadConstraintDeciderIT #133894

nicktindall · 2025-09-01T00:25:37Z

Occasionally the WriteLoadConstraintDecider would prevent shards moving to node 2 because those moves would exceed the utilisation threshold

I ran the test with all the extremes and it still works (i.e. randomUtilizationThresholdPercent = 50, numberOfWritePoolThreads = 10, shardWriteLoad = 0.2, randomNumberOfShards = 20)

Resolves: #133857

elasticsearchmachine · 2025-09-01T00:26:16Z

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

nicktindall · 2025-09-01T00:35:16Z

.../java/org/elasticsearch/cluster/routing/allocation/decider/WriteLoadConstraintDeciderIT.java

    public void testHighNodeWriteLoadPreventsNewShardAllocation() {
        int randomUtilizationThresholdPercent = randomIntBetween(50, 100);
+        int numberOfWritePoolThreads = randomIntBetween(10, 20);
+        double shardWriteLoad = randomDoubleBetween(0.0, 0.2, true);


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.

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.

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

ywangd

Some minor comments

ywangd · 2025-09-02T06:31:48Z

.../java/org/elasticsearch/cluster/routing/allocation/decider/WriteLoadConstraintDeciderIT.java

     */
    public void testHighNodeWriteLoadPreventsNewShardAllocation() {
        int randomUtilizationThresholdPercent = randomIntBetween(50, 100);
+        int numberOfWritePoolThreads = randomIntBetween(10, 20);


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

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.

Fixed in ba15f8f

ywangd · 2025-09-02T06:32:41Z

.../java/org/elasticsearch/cluster/routing/allocation/decider/WriteLoadConstraintDeciderIT.java

            thirdDiscoveryNode,
-            2,
+            numberOfWritePoolThreads,
            randomUtilizationThresholdPercent + 1 / 100,


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

No, the test works by

Create an index while cluster.routing.allocation.exclude._name = "secondDataNode, thirdDataNode" (all shards are assigned to firstDataNode)

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)

Refresh cluster info

Update cluster.routing.allocation.exclude._name = "firstDataNode"

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.

DiannaHohensee

Thanks for diagnosing and fixing this, Nick! I left a few cosmetic requests, but lgtm.

Could you change the commit message to something like "Account for simulated utilization threshold in WriteLoadConstraintDeciderIT" ? Fix is very vague.

DiannaHohensee · 2025-09-02T17:51:04Z

.../java/org/elasticsearch/cluster/routing/allocation/decider/WriteLoadConstraintDeciderIT.java

+        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;


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.

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

DiannaHohensee · 2025-09-02T17:51:17Z

.../java/org/elasticsearch/cluster/routing/allocation/decider/WriteLoadConstraintDeciderIT.java

    public void testHighNodeWriteLoadPreventsNewShardAllocation() {
        int randomUtilizationThresholdPercent = randomIntBetween(50, 100);
+        int numberOfWritePoolThreads = randomIntBetween(10, 20);
+        double shardWriteLoad = randomDoubleBetween(0.0, 0.2, true);


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.

DiannaHohensee · 2025-09-02T17:57:09Z

.../java/org/elasticsearch/cluster/routing/allocation/decider/WriteLoadConstraintDeciderIT.java

     */
    public void testHighNodeWriteLoadPreventsNewShardAllocation() {
        int randomUtilizationThresholdPercent = randomIntBetween(50, 100);
+        int numberOfWritePoolThreads = randomIntBetween(10, 20);


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.

…rdingly, ensure its non-zero

…ntDeciderIT # Conflicts: # muted-tests.yml

Copilot

Pull Request Overview

This PR fixes a flaky test in WriteLoadConstraintDeciderIT that was failing because the test didn't properly account for simulated utilization thresholds when moving shards. The fix ensures that node utilization calculations consider the additional load from all shards being moved, preventing test failures where shard movements would exceed the utilization threshold.

Extracts utilization calculation logic into a reusable method in ShardMovementWriteLoadSimulator
Updates the test to dynamically calculate maximum node utilization based on shard write load and thread pool configuration
Removes the test from the muted tests list since it's now fixed

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
ShardMovementWriteLoadSimulator.java	Extracts utilization calculation into a public static method for reuse
WriteLoadConstraintDeciderIT.java	Updates test to properly calculate node utilization thresholds accounting for shard movement load
muted-tests.yml	Removes the now-fixed test from the muted tests list

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

.../java/org/elasticsearch/cluster/routing/allocation/decider/WriteLoadConstraintDeciderIT.java

BASE=5a4c3abb0e6c32c231da4a8377e4c7d75ee9379a HEAD=dc48fbbc74234e603da579b67c144a25d100b957 Branch=main

Fix WriteLoadConstraintDeciderIT

d9b45d4

nicktindall added :Distributed Coordination/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) >test Issues or PRs that are addressing/adding tests labels Sep 1, 2025

elasticsearchmachine added v9.2.0 Team:Distributed Coordination Meta label for Distributed Coordination team labels Sep 1, 2025

Remove debug logging

f506534

nicktindall requested a review from DiannaHohensee September 1, 2025 00:29

nicktindall commented Sep 1, 2025

View reviewed changes

ywangd reviewed Sep 2, 2025

View reviewed changes

DiannaHohensee approved these changes Sep 2, 2025

View reviewed changes

Reduce minimum write pool threads, adjust shard write load range acco…

ba15f8f

…rdingly, ensure its non-zero

nicktindall changed the title ~~Fix WriteLoadConstraintDeciderIT~~ Account for simulated utilization threshold in WriteLoadConstraintDeciderIT Sep 3, 2025

nicktindall added 2 commits September 3, 2025 11:43

Merge remote-tracking branch 'origin/main' into fix_WriteLoadConstrai…

c995fb7

…ntDeciderIT # Conflicts: # muted-tests.yml

Add utility for converting write load to utilization

946469e

nicktindall requested a review from Copilot September 3, 2025 01:57

Copilot AI reviewed Sep 3, 2025

View reviewed changes

.../java/org/elasticsearch/cluster/routing/allocation/decider/WriteLoadConstraintDeciderIT.java Outdated Show resolved Hide resolved

Fix integer division issue

dc48fbb

nicktindall merged commit 068ca76 into elastic:main Sep 3, 2025
33 checks passed

nicktindall deleted the fix_WriteLoadConstraintDeciderIT branch September 3, 2025 03:11

phananh1010 added a commit to phananh1010/elasticsearch that referenced this pull request Sep 11, 2025

Mirror upstream elastic#133894 as single snapshot commit for AI review

0fa8da1

BASE=5a4c3abb0e6c32c231da4a8377e4c7d75ee9379a HEAD=dc48fbbc74234e603da579b67c144a25d100b957 Branch=main

phananh1010 added a commit to phananh1010/elasticsearch that referenced this pull request Sep 12, 2025

Mirror upstream elastic#133894 as single snapshot commit for AI review

5a3bf63

BASE=5a4c3abb0e6c32c231da4a8377e4c7d75ee9379a HEAD=dc48fbbc74234e603da579b67c144a25d100b957 Branch=main

phananh1010 added a commit to phananh1010/elasticsearch that referenced this pull request Sep 16, 2025

Mirror upstream elastic#133894 as single snapshot commit for AI review

a6871a6

BASE=5a4c3abb0e6c32c231da4a8377e4c7d75ee9379a HEAD=dc48fbbc74234e603da579b67c144a25d100b957 Branch=main

phananh1010 added a commit to phananh1010/elasticsearch that referenced this pull request Sep 16, 2025

Mirror upstream elastic#133894 as single snapshot commit for AI review

e0e9c3f

BASE=5a4c3abb0e6c32c231da4a8377e4c7d75ee9379a HEAD=dc48fbbc74234e603da579b67c144a25d100b957 Branch=main

phananh1010 added a commit to phananh1010/elasticsearch that referenced this pull request Sep 16, 2025

Mirror upstream elastic#133894 as single snapshot commit for AI review

40084eb

BASE=5a4c3abb0e6c32c231da4a8377e4c7d75ee9379a HEAD=dc48fbbc74234e603da579b67c144a25d100b957 Branch=main

phananh1010 added a commit to phananh1010/elasticsearch that referenced this pull request Oct 9, 2025

Mirror upstream elastic#133894 as single snapshot commit for AI review

eac5a74

BASE=5a4c3abb0e6c32c231da4a8377e4c7d75ee9379a HEAD=dc48fbbc74234e603da579b67c144a25d100b957 Branch=main

phananh1010 added a commit to phananh1010/elasticsearch that referenced this pull request Oct 16, 2025

Mirror upstream elastic#133894 as single snapshot commit for AI review

f33b035

BASE=5a4c3abb0e6c32c231da4a8377e4c7d75ee9379a HEAD=dc48fbbc74234e603da579b67c144a25d100b957 Branch=main

phananh1010 added a commit to phananh1010/elasticsearch that referenced this pull request Oct 24, 2025

Mirror upstream elastic#133894 as single snapshot commit for AI review

ebff5f5

BASE=5a4c3abb0e6c32c231da4a8377e4c7d75ee9379a HEAD=dc48fbbc74234e603da579b67c144a25d100b957 Branch=main

Account for simulated utilization threshold in WriteLoadConstraintDeciderIT #133894

Account for simulated utilization threshold in WriteLoadConstraintDeciderIT #133894

Uh oh!

Conversation

nicktindall commented Sep 1, 2025 • edited by ywangd Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 1, 2025

Uh oh!

nicktindall Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ywangd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicktindall Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DiannaHohensee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nicktindall commented Sep 1, 2025 •

edited by ywangd

Loading

nicktindall Sep 1, 2025 •

edited

Loading

nicktindall Sep 2, 2025 •

edited

Loading