Prioritise movement of shards in non-preferred allocations #135058

nicktindall · 2025-09-19T07:32:32Z

This approach keeps track of the best NOT_PREFERRED moves seen when iterating through looking for NOs.

If we get to the end of the moveShards loop and we can still make moves, we will pick one of the NOT_PREFERRED moves to make.

There is no prioritisation of NO moves in this approach, anything with a decider returning NO will be moved as soon as we encounter it in the node-interleaved iterator.

...ain/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java

…hard_to_move_off_data_node_v2

nicktindall · 2025-09-22T02:56:36Z

...ain/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java

+            if (canRemain.type() == Type.NOT_PREFERRED && bestNonPreferredShardsByNode.containsKey(shardRouting.currentNodeId())) {
+                int compare = comparatorCache.computeIfAbsent(
+                    shardRouting.currentNodeId(),
+                    nodeId -> new ShardMovementPriorityComparator(allocation, allocation.routingNodes().node(nodeId))


By caching these, we are sorting all shards by the values calculated when the comparator was first created. It's likely that the threshold and maximum will move as shards are moved, but given that we don't move too much in each iteration, I think it's fine to accept the potential for slight differences.

…ode_v2

…hard_to_move_off_data_node_v2

ywangd · 2025-10-02T05:53:54Z

...ain/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java

+                    // Return after a single move so that the change can be simulated before further moves are made.
+                    return true;


Do we want to check if (completeEarlyOnShardAssignmentChange) here?

I opted not to because it's happening after all the NO moves and this is new behaviour, we know that we rely on the simulation being updated for these movements to make sense so I don't think we should ever attempt more than one.

DiannaHohensee

The code changes look good to me, I left a few more suggestions.

I still need to look at the testing, but I've got an appointment, so publishing the comments I have.

...ain/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java

DiannaHohensee

It would be good to have IT testing with the DesiredBalanceShardsAllocator. This test could be updated: would need to give it a range of shard write loads, and then check that a specific shard was moved.

I'm totally fine with that testing being a follow up, however you prefer.

Left a lot of comments, but I think they're simple, so approving now. Let me know if you want me to take another look.

...ava/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocatorTests.java

…ode_v2

nicktindall · 2025-10-06T04:51:30Z

It would be good to have IT testing with the DesiredBalanceShardsAllocator. This test could be updated: would need to give it [a range of shard write loads]

Thanks for the reviews! I've added the write loads to the test you mentioned and assert that the "best" shard is moved in 4170af3

nicktindall · 2025-10-06T05:14:10Z

Buildkite benchmark this with many-shards-quantitative please

elasticmachine · 2025-10-06T05:21:29Z

💚 Build Succeeded

Buildkite Build
Commit: 4170af3
Baseline: 3979d74 (env ID c8ec6a45-a3a2-47ed-a54f-90aa2109fe6e)
Contender: 4170af3 (env ID 253fe42a-29f9-4440-b80e-0fdf83fa1bf7)
Benchmark results

This build ran two many-shards-quantitative benchmarks to evaluate performance impact of this PR.

History

💚 Build #26 succeeded 3ee55e7

DiannaHohensee

Thanks for the IT testing. Looks good 👍

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

DiannaHohensee · 2025-10-06T21:09:36Z

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


    /**
     * Helper to create a list of dummy {@link ShardStats} for the given index, each shard reporting a {@code peakShardWriteLoad} stat.
     */


Method comment needs update

Fixed in 29a79e1

…routing/allocation/decider/WriteLoadConstraintDeciderIT.java Co-authored-by: Dianna Hohensee <[email protected]>

…ode_v2

New new approach

053edfa

elasticsearchmachine added the v9.2.0 label Sep 19, 2025

Add logging when moveDecision is stale

e38191a

henningandersen reviewed Sep 19, 2025

View reviewed changes

...ain/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java Outdated Show resolved Hide resolved

assert we're not abusing the comparator

909fd6d

nicktindall mentioned this pull request Sep 21, 2025

Implement move non preferred phase in allocator #134429

Closed

nicktindall and others added 5 commits September 22, 2025 12:26

Change comparator ranking, test

2696fd9

[CI] Update transport version definitions

10c042d

Use constant for missing write load

f03e4b8

Merge remote-tracking branch 'origin/main' into ES-12739_select_hot_s…

be6253c

…hard_to_move_off_data_node_v2

Distinguish between move and move-not-preferred movements

9112b1d

nicktindall commented Sep 22, 2025

View reviewed changes

nicktindall added 12 commits September 22, 2025 13:00

Update javadoc

49d11e3

Tweak test params

7e23a15

Simplify logic

fcde6c6

Simplify logic

ceff1e3

Simplify logic

6dcb6f0

Simplify logic

c03bcde

Tidy

fdca404

Simplify logic

36cff36

Tidy

98b5b80

Pedantry

df79e6f

Add test for NOT_PREFERRED movement

b2d67d7

Pedantry

3c5f592

nicktindall requested review from DiannaHohensee and mhl-b September 22, 2025 05:52

nicktindall added >non-issue :Distributed Coordination/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) labels Sep 22, 2025

nicktindall requested a review from henningandersen September 22, 2025 05:54

Merge branch 'main' into ES-12739_select_hot_shard_to_move_off_data_n…

ae504cd

…ode_v2

nicktindall added 3 commits October 2, 2025 14:39

Remove redundant check

105d7bf

Merge remote-tracking branch 'origin/main' into ES-12739_select_hot_s…

50d266f

…hard_to_move_off_data_node_v2

Improve javadoc

7837211

ywangd reviewed Oct 2, 2025

View reviewed changes

elasticsearchmachine added v9.3.0 and removed v9.2.0 labels Oct 2, 2025

DiannaHohensee reviewed Oct 2, 2025

View reviewed changes

DiannaHohensee approved these changes Oct 3, 2025

View reviewed changes

nicktindall added 12 commits October 6, 2025 10:48

Improve javadoc

cc147c9

Improve javadoc (again)

35dd5d4

Improve javadoc

c32d6e9

Use constant for threshold

291416a

Document test utility

0bfcf6b

Explain single shard indices

7668101

Improve naming

f3588e2

Improve naming

7643dd0

Reference comparator in javadoc

e514692

Document test decider

ddc7739

Merge branch 'main' into ES-12739_select_hot_shard_to_move_off_data_n…

7753a44

…ode_v2

Test movement prioritization in IT

4170af3

DiannaHohensee approved these changes Oct 6, 2025

View reviewed changes

nicktindall and others added 3 commits October 7, 2025 09:54

Update server/src/internalClusterTest/java/org/elasticsearch/cluster/…

43a2dd3

…routing/allocation/decider/WriteLoadConstraintDeciderIT.java Co-authored-by: Dianna Hohensee <[email protected]>

Fix Javadoc

29a79e1

Merge branch 'main' into ES-12739_select_hot_shard_to_move_off_data_n…

e64343f

…ode_v2

nicktindall enabled auto-merge (squash) October 6, 2025 23:00

nicktindall merged commit c19d4d7 into elastic:main Oct 7, 2025
34 checks passed

nicktindall deleted the ES-12739_select_hot_shard_to_move_off_data_node_v2 branch October 7, 2025 00:05

		// Return after a single move so that the change can be simulated before further moves are made.
		return true;

Prioritise movement of shards in non-preferred allocations #135058

Prioritise movement of shards in non-preferred allocations #135058

Conversation

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

Uh oh!

Uh oh!

Uh oh!

nicktindall Sep 22, 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!

ywangd Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

nicktindall Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

DiannaHohensee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DiannaHohensee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nicktindall commented Oct 6, 2025

Uh oh!

nicktindall commented Oct 6, 2025

Uh oh!

elasticmachine commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

History

Uh oh!

DiannaHohensee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DiannaHohensee Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

nicktindall Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nicktindall commented Sep 19, 2025 •

edited

Loading

nicktindall Sep 22, 2025 •

edited

Loading

elasticmachine commented Oct 6, 2025 •

edited

Loading