Add stateless `ReindexRelocationNodePicker` impl by PeteGillinElastic · Pull Request #140267 · elastic/elasticsearch

PeteGillinElastic · 2026-01-07T13:48:13Z

This adds an implmentation of the helper which picks a node to relocate a reindex task to on shutdown to be used in stateless.

This changes the mechanism for choosing the correct implementation that was added in #139081. The previous mechanism assumed that we'd want serverless to behave differently, and so it defined a default implementation and allowed for that to be swapped out via another plugin. But we actually want stateless to behave differently, so we can take the simpler approach of doing everything inside the reindex plugin and just pick the correct implementation based on settings. The implementation added previously has been renamed, because it is now explicitly used in stateful rather than just being used by default.

Closes elastic/elasticsearch-team#2093

This adds an implmentation of the helper which picks a node to relocate a reindex task to on shutdown to be used in stateless. This changes the mechanism for choosing the correct implementation that was added in elastic#139081. The previous mechanism assumed that we'd want *serverless* to behave differently, and so it defined a default implementation and allowed for that to be swapped out via another plugin. But we actually want *stateless* to behave differently, so we can take the simpler approach of doing everything inside the `reindex` plugin and just pick the correct implementation based on settings. The implementation added previously has been renamed, because it is now explicitly used in stateful rather than just being used by default.

elasticsearchmachine · 2026-01-07T17:26:15Z

Pinging @elastic/es-distributed-indexing (Team:Distributed Indexing)

…node-picker-in-stateless

szybia

few considerations, nothing to block on

fwiw if you agree with some suggestions things should be changed in both implementations of ReindexRelocationNodePicker

szybia · 2026-01-19T19:51:02Z

...es/reindex/src/main/java/org/elasticsearch/reindex/StatelessReindexRelocationNodePicker.java

+    public String pickNode(DiscoveryNodes nodes) {
+        String currentNodeId = nodes.getLocalNodeId();
+        if (currentNodeId == null) {
+            logger.debug(


should this be a higher-priority log? (ik you're doing the same as in StatefulNodePicker)

but sounds like a very wrong situation to be in

and also sounds useful to get this in logs if it ever happens and we're investigating why a relocation didn't happen? 🤔

Yeah, I think you're probably right. FWIW, DiscoveryNodes.isLocalNodeElectedMaster() handles the case where localNodeId == null and there's a comment saying we don't know yet the local node id, which implies that it is expected under some circumstances. However, I would imagine that this is something that happens early in the discovery process, and it shouldn't have got as far as running real user work (such as reindex tasks) without assigning this.

I'll bump it up to warn, in both impls.

szybia · 2026-01-19T19:59:25Z

...es/reindex/src/main/java/org/elasticsearch/reindex/StatelessReindexRelocationNodePicker.java

+            .toList();
+        if (eligibleDedicatedCoordinatingNodes.isEmpty() == false) {
+            String newNodeId = randomNodeId(eligibleDedicatedCoordinatingNodes);
+            logger.debug("Chose dedicated coordinating node ID {} for relocating a reindex task from node {}", newNodeId, currentNodeId);


doesn't need to be here but starting conversation

i'm thinking we'll want to have some logging to say i'm relocating this task to this node

if so, i'm imagining we'll have it higher up in the call stack and not here?

Yeah, anything like that would be higher up the stack. It's not really the concern of this thing to know anything much about the context it's being used in.

szybia · 2026-01-19T20:12:58Z

modules/reindex/src/main/java/org/elasticsearch/reindex/ReindexPlugin.java

            new UpdateByQueryMetrics(services.telemetryProvider().getMeterRegistry()),
            new DeleteByQueryMetrics(services.telemetryProvider().getMeterRegistry()),
-            new PluginComponentBinding<>(ReindexRelocationNodePicker.class, relocationNodePicker.get())
+            new PluginComponentBinding<>(


i think potential for this to mess anything up is very small, but maybe a will sleep 1% better at night improvement

should we put this being returned in the list behind feature flag so "truly" none of our changes can affect release build?

I think that would be tricky. The binding will be used in the @Inject constructor of TransportReindexAction, and injection will fail if there isn't anything bound to this key.

I guess that we could maybe mark it as @Nullable in that constructor, and bind it to null if the feature flag isn't set. However, once we remove the feature flag, we won't really want it to be nullable. And I worry that we'd forget to chase down the places we'd made it nullable (we'll be passing it into Reindexer, I think) and change that. I'd be more worried about that than I would about the fact that we're instantiating this thing in production without using it.

szybia · 2026-01-19T20:25:38Z

...index/src/test/java/org/elasticsearch/reindex/StatelessReindexRelocationNodePickerTests.java

+    private static final Set<DiscoveryNodeRole> SEARCH_NODE_ROLES = Set.of(
+        DiscoveryNodeRole.SEARCH_ROLE,
+        DiscoveryNodeRole.TRANSFORM_ROLE,
+        DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE
+    );


will probably scan for it myself, but if you get here before i do

where are these defined (for learning and for double-checking)?

(might be private, therefore slack?)

If you're asking how I know that these are the roles assigned to each node type in serverless: I just did GET /_nodes on a running instance. (As it happens, you prompted me to go poking around in the code for the controller, which is where this stuff is set up. That raised one curious thing, there's a thread in slack if you're interested.)

...es/reindex/src/main/java/org/elasticsearch/reindex/StatelessReindexRelocationNodePicker.java

szybia · 2026-01-19T20:38:53Z

...es/reindex/src/main/java/org/elasticsearch/reindex/StatelessReindexRelocationNodePicker.java

+            "Trying to pick a node to relocate a reindex task to, but there are no dedicated coordinating or indexing nodes "
+                + "(perhaps excluding the current node): the relocation attempt will be aborted"
+        );
+        return null;


fwiw, imo would be nice for interface to be Optional<String>

not a fan of @Nullable

Yeah, that's probably an improvement. Done.

Co-authored-by: Szymon Bialkowski <szybia@tuta.io>

…node-picker-in-stateless

PeteGillinElastic · 2026-01-20T13:04:35Z

few considerations, nothing to block on

fwiw if you agree with some suggestions things should be changed in both implementations of ReindexRelocationNodePicker

Thanks! All done, except for one where I've pushed back for reasons.

samxbr · 2026-01-20T20:38:34Z

...es/reindex/src/main/java/org/elasticsearch/reindex/StatelessReindexRelocationNodePicker.java

+import java.util.Optional;
+import java.util.Random;
+
+class StatelessReindexRelocationNodePicker implements ReindexRelocationNodePicker {


I wonder if we should avoid picking a node that's marked for shutdown in NodesShutdownMetadata, since relocating to a to-be-shutdown node will just lead to a second relocation? I think a node is removed from DiscoveryNodes by NodeLeftExecutor when it's already shutdown, so it seems a to-be-shutdown node can still exists in DiscoveryNodes.

The NodeShutdownAllocationDecider has something similar to avoid nodes marked for shutdown. In our case, the logic to exclude shutdown nodes could be shared by stateless and stateful?

That's a really good idea, thanks for pointing it out.

This PR has been in CI purgatory for ages and finally has a green light (if you'll forgive the mixed metaphors) so I'm going to merge it and do this (for both cases) as a follow-up.

This adds an implmentation of the helper which picks a node to relocate a reindex task to on shutdown to be used in stateless. This changes the mechanism for choosing the correct implementation that was added in elastic#139081. The previous mechanism assumed that we'd want *serverless* to behave differently, and so it defined a default implementation and allowed for that to be swapped out via another plugin. But we actually want *stateless* to behave differently, so we can take the simpler approach of doing everything inside the `reindex` plugin and just pick the correct implementation based on settings. The implementation added previously has been renamed, because it is now explicitly used in stateful rather than just being used by default. Co-authored-by: Szymon Bialkowski <szybia@tuta.io> Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>

elasticsearchmachine added the v9.4.0 label Jan 7, 2026

PeteGillinElastic added >non-issue :Distributed/Reindex Issues relating to reindex that are not caused by issues further down labels Jan 7, 2026

PeteGillinElastic force-pushed the reindex-relocation-node-picker-in-stateless branch from 4476680 to 81c4a42 Compare January 7, 2026 14:04

PeteGillinElastic force-pushed the reindex-relocation-node-picker-in-stateless branch from 81c4a42 to d6afa2f Compare January 7, 2026 15:20

PeteGillinElastic marked this pull request as ready for review January 7, 2026 17:25

elasticsearchmachine added the Team:Distributed Indexing (obsolete) Meta label for Distributed Indexing team. Obsolete. Please do not use. label Jan 7, 2026

PeteGillinElastic added 5 commits January 14, 2026 17:14

Merge remote-tracking branch 'upstream/main' into reindex-relocation-…

0a0147f

…node-picker-in-stateless

Merge remote-tracking branch 'upstream/main' into reindex-relocation-…

3de8e8e

…node-picker-in-stateless

use dedicated coordinating nodes if available

80de247

Merge remote-tracking branch 'upstream/main' into reindex-relocation-…

ddc00aa

…node-picker-in-stateless

Merge remote-tracking branch 'upstream/main' into reindex-relocation-…

6377551

…node-picker-in-stateless

szybia approved these changes Jan 19, 2026

View reviewed changes

PeteGillinElastic and others added 5 commits January 20, 2026 12:45

Rename method

49e8182

Co-authored-by: Szymon Bialkowski <szybia@tuta.io>

bump logging when we do not know local node ID up to WARN

b1bd705

finish renaming method

8e68ac3

switch to returning optional

c274320

Merge remote-tracking branch 'upstream/main' into reindex-relocation-…

856c702

…node-picker-in-stateless

elasticsearchmachine and others added 3 commits January 20, 2026 13:11

[CI] Auto commit changes from spotless

b1a805d

Merge branch 'main' into reindex-relocation-node-picker-in-stateless

4183fb5

Merge branch 'main' into reindex-relocation-node-picker-in-stateless

7b04686

samxbr reviewed Jan 20, 2026

View reviewed changes

PeteGillinElastic merged commit 1fdea95 into elastic:main Jan 21, 2026
35 checks passed

Conversation

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

Uh oh!

Uh oh!

elasticsearchmachine commented Jan 7, 2026

Uh oh!

szybia 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!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PeteGillinElastic commented Jan 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

PeteGillinElastic commented Jan 7, 2026 •

edited

Loading