Allow relocation to NOT_PREFERRED node for evacuating shards by nicktindall · Pull Request #140197 · elastic/elasticsearch

nicktindall · 2026-01-06T02:45:26Z

A change in #137228 meant we sometimes return canMove=NOT_PREFERRED from BalancedShardsAllocator#decideMove, but we still would only execute a move when canMove=YES, this meant when we were unable to relocate shards from a shutting down node to a not-preferred allocation.

This PR fixes the logic to execute the move when canMove is YES or NOT_PREFERRED

DiannaHohensee

didn't look at test, but I think we should fix the other call-site?

DiannaHohensee · 2026-01-06T02:49:22Z

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

                final var shardRouting = storedShardMovement.shardRouting();
                final var index = projectIndex(shardRouting);
                final var moveDecision = refreshDecisionIfRequired(index, storedShardMovement, shardMoved);
                if (moveDecision.isDecisionTaken() && moveDecision.cannotRemainAndCanMove()) {


We've got to change this, too, I believe?

refreshDecisionIfRequired seems to wind down to the same decideMove method, and even if not-preferred didn't happen now, it could easily happen in future.

Fixed in cb607f1

DiannaHohensee · 2026-01-06T02:52:18Z

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


-                if (moveDecision.isDecisionTaken() && moveDecision.cannotRemainAndCanMove()) {
+                if (moveDecision.isDecisionTaken()
+                    && (moveDecision.cannotRemainAndCanMove() || moveDecision.cannotRemainAndNotPreferredMove())) {


cannotRemainAndCanMove caught my eye a while ago, and I wanted to make it something like cannotRemainAndCanMoveYes. However, now, I wonder if we should just keep cannotRemainAndCanMove and implement that single method as

cannotRemain() && (canMoveDecision == AllocationDecision.YES || canMoveDecision == AllocationDecision.NOT_PREFERRED); ?

Both production callers of cannotRemainAndCanMove now need to check both, I think.

Yeah there will need to be some consideration of how to do this because it's used in other places too. I'm not sure what the tidiest approach is

These two callers in BalancedShardsAllocator, MoveDecision printing. So it's pretty contained.

My preference would be to combine both under cannotRemainAndCanMove, or rename cannotRemainAndCanMove to include '-Yes-' someplace -- so it's too obvious to make this mistake again. But not a commit blocker to me if you strongly prefer something else.

Yep good call I think it makes more sense that way too. Changed in cb607f1

DiannaHohensee

testing looks fine with a couple superficial nits.

...down/src/internalClusterTest/java/org/elasticsearch/xpack/shutdown/ShutdownEvacuationIT.java

DiannaHohensee

Approving, straightforward feedback. Pls fix the commit title before merging :)

elasticsearchmachine · 2026-01-06T05:14:33Z

Hi @nicktindall, I've created a changelog YAML for you.

elasticsearchmachine · 2026-01-06T05:14:33Z

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

ywangd

LGTM

Nit: Maybe the PR title can more directly say something like "allow relocation to NOT_PREFERRED node for evacuating shards"?

henningandersen

LGTM.

nicktindall · 2026-01-06T11:46:00Z

@elasticsearchmachine test this please

…#140197)

elasticsearchmachine · 2026-01-06T12:52:26Z

💚 Backport successful

Status	Branch	Result
✅	9.3

#140208)

Fix canMove regression (hack)

8e4d326

nicktindall requested a review from DiannaHohensee January 6, 2026 02:45

elasticsearchmachine added the v9.4.0 label Jan 6, 2026

The test

6e9ad80

DiannaHohensee reviewed Jan 6, 2026

View reviewed changes

...down/src/internalClusterTest/java/org/elasticsearch/xpack/shutdown/ShutdownEvacuationIT.java Outdated Show resolved Hide resolved

...down/src/internalClusterTest/java/org/elasticsearch/xpack/shutdown/ShutdownEvacuationIT.java Outdated Show resolved Hide resolved

nicktindall marked this pull request as ready for review January 6, 2026 03:13

elasticsearchmachine added the needs:triage Requires assignment of a team area label label Jan 6, 2026

DiannaHohensee approved these changes Jan 6, 2026

View reviewed changes

elastic deleted a comment from elasticmachine Jan 6, 2026

nicktindall added >bug :Distributed/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) labels Jan 6, 2026

elasticsearchmachine added Team:Distributed Coordination (obsolete) Meta label for Distributed Coordination team. Obsolete. Please do not use. and removed needs:triage Requires assignment of a team area label labels Jan 6, 2026

Update docs/changelog/140197.yaml

9424e14

nicktindall changed the title ~~Fix canMove regression (hack)~~ Handle moveDecision returning NOT_PREFERRED correctly Jan 6, 2026

nicktindall added 2 commits January 6, 2026 16:15

Fix changelog message

5aacbae

Tidy

cb607f1

nicktindall added auto-backport Automatically create backport pull requests when merged v9.3.1 labels Jan 6, 2026

nicktindall requested a review from ywangd January 6, 2026 06:14

ywangd approved these changes Jan 6, 2026

View reviewed changes

ywangd mentioned this pull request Jan 6, 2026

Fix Decision.Type serialization BWC #140199

Merged

nicktindall changed the title ~~Handle moveDecision returning NOT_PREFERRED correctly~~ Allow relocation to NOT_PREFERRED node for evacuating shards Jan 6, 2026

nicktindall added 2 commits January 6, 2026 17:54

Handle NOT_PREFERRED/NOT_PREFERRED case

4bdf49f

Update changelog summary

0ff4103

henningandersen approved these changes Jan 6, 2026

View reviewed changes

Merge branch 'main' into fix_canMove_regression

e201f6b

nicktindall merged commit 6630582 into elastic:main Jan 6, 2026
35 checks passed

nicktindall deleted the fix_canMove_regression branch January 6, 2026 12:51

nicktindall added a commit to nicktindall/elasticsearch that referenced this pull request Jan 6, 2026

Allow relocation to NOT_PREFERRED node for evacuating shards (elastic…

726e336

…#140197)

nicktindall mentioned this pull request Jan 6, 2026

[9.3] Allow relocation to NOT_PREFERRED node for evacuating shards (#140197) #140208

Merged

elasticsearchmachine pushed a commit that referenced this pull request Jan 6, 2026

Allow relocation to NOT_PREFERRED node for evacuating shards (#140197) (

fc553ac

#140208)

Comments

Conversation

nicktindall commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DiannaHohensee left a comment

Choose a reason for hiding this comment

Uh oh!

DiannaHohensee Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

nicktindall Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

DiannaHohensee Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

nicktindall Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

DiannaHohensee Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

nicktindall Jan 6, 2026

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!

DiannaHohensee left a comment

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Jan 6, 2026

Uh oh!

elasticsearchmachine commented Jan 6, 2026

Uh oh!

ywangd left a comment

Choose a reason for hiding this comment

Uh oh!

henningandersen left a comment

Choose a reason for hiding this comment

Uh oh!

nicktindall commented Jan 6, 2026

Uh oh!

Uh oh!

elasticsearchmachine commented Jan 6, 2026

💚 Backport successful

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nicktindall commented Jan 6, 2026 •

edited

Loading