Add clone step to DLM by seanzatzdev · Pull Request #141638 · elastic/elasticsearch

seanzatzdev · 2026-02-02T06:04:35Z

Adds clone step to DLM

closes https://github.com/orgs/elastic/projects/2139/views/1?pane=issue&itemId=141184511&issue=elastic%7Celasticsearch-team%7C2111

# Conflicts: # modules/data-streams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java # modules/data-streams/src/test/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStepTests.java

Copilot

Pull request overview

This PR adds a clone step to the Data Lifecycle Management (DLM) system. The clone step creates a copy of an index with zero replicas to prepare it for force merging, optimizing resource usage during the force merge operation.

Changes:

Implements CloneStep that clones indices with 0 replicas or marks indices that already have 0 replicas for force merging
Adds MarkIndexToBeForceMergedAction to update cluster state metadata marking which index should be force merged
Adds comprehensive test coverage for the clone step functionality

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
CloneStep.java	Implements the core clone step logic including index cloning, metadata marking, and cleanup operations
MarkIndexToBeForceMergedAction.java	Defines the action and request/response structures for marking indices to be force merged
TransportMarkIndexToBeForceMergedAction.java	Implements the transport layer for the mark index action
CloneStepTests.java	Provides comprehensive unit tests for the clone step functionality
DataStreamsPlugin.java	Registers the new mark index action handler

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

elasticsearchmachine · 2026-02-03T09:41:12Z

Pinging @elastic/es-storage-engine (Team:StorageEngine)

lukewhiting · 2026-02-03T09:43:30Z

@elasticsearchmachine test this

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

lukewhiting · 2026-02-03T10:23:54Z

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

+        String cloneIndexName = generateCloneIndexName(indexName);
+        if (projectMetadata.indices().containsKey(cloneIndexName)) {
+            logger.info("DLM cleaning up clone index [{}] for index [{}] as it already exists.", cloneIndexName, indexName);
+            deleteCloneIndexIfExists(stepContext);


Could this cause an infinite loop?

If this step runs again while the clone is in progress (Which may happen as we keep re-running execute(..) until stepComplete(...) is true, relying on the action duplicators to prevent us actually making changes twice) won't it delete the currently cloning index then try and clone it again next cycle?

Good catch, the deduplicator alone wouldn't fix this though if i understand correctly? Struggling to think of a solution here that allows us to clean up without a potential conflict

I also just realized i forgot to add the deduplicators on the other requests... let me fix that now!

Leaving a note here that we discussed this offline and will be following up on this problem to discuss workarounds

I added the timeout we discussed, figured the best thing to do if the timeout hasn't been reached is just return and let the next DLM run check again rather than have the step's thread waiting around and checking periodically or something

lukewhiting · 2026-02-03T10:27:43Z

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

+        public void onResponse(CreateIndexResponse createIndexResponse) {
+            logger.debug("DLM successfully cloned index [{}] to index [{}]", sourceIndexName, targetIndexName);
+            // on success, write the cloned index name to the custom metadata of the index metadata of original index
+            markIndexToBeForceMerged(sourceIndexName, targetIndexName, stepContext, listener);


I believe all actions need to be executed via the deduplicator. I'm also not sure what effect calling an action inside a listener vs callback would have on thread usage... Perhaps @dakrone can weigh in on this pattern?

My mistake, i totally forgot to wrap the other actions in the deduplicator... will fix that

I'm also not sure what effect calling an action inside a listener vs callback would have on thread usage

True, I can move this to a callback pattern

We do have a SubscribableListener which can be used for this, where you can chain actions with the .andThen(…) method. I've seen it used multiple places, though I don't claim to be an expert in its use.

I ended up adapting the way some other places in the codebase do this, e.g. TransportReindexAction, if I understand correctly the execute's should be async? The SubscribableListener didn't seem as common to me so I was a little apprehensive to use it. Maybe I need some more pointers on this though if there's problems with my approach, I'm finding elasticsearch's listeners to be a little confusing to work with to say the least...

dakrone

I haven't looked at the test cases yet, but I left some comments about the implementation so far, thanks for working on this Sean!

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

dakrone · 2026-02-03T00:18:49Z

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

+        if (indexRoutingTable == null) {
+            return false;
+        }
+        return indexRoutingTable.allPrimaryShardsActive();


Do we have index metadata that links the two indices? Should we add a check for that here if we do?

The first conditional in the chain (getIndexToBeForceMerged) should check for that, it checks whether there has been an index marked for force merge for the source index.

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

...g/elasticsearch/datastreams/lifecycle/transitions/steps/MarkIndexForDLMForceMergeAction.java

...rg/elasticsearch/datastreams/lifecycle/transitions/steps/MarkIndexToBeForceMergedAction.java

lukewhiting

Solid progress :-) Just a few bugs left to iron out.

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

lukewhiting · 2026-02-11T09:52:13Z

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

+        String cloneIndexName = getCloneIndexName(indexName);
+        IndexMetadata cloneIndexMetadata = stepContext.projectState().metadata().index(cloneIndexName);
+        long cloneCreationTime = cloneIndexMetadata.getCreationDate();
+        long currentTime = System.currentTimeMillis();


Generally I would avoid using System.currentTimeMillis and use a Clock instead as it will make testing these timeouts much easier.

We should probably add a Clock to the step context then, so it's available to every step.

We should probably add a Clock to the step context then, so it's available to every step.

Clock.systemUTC().millis() is a static method, would we still want to add a clock to the context?

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

.../src/test/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStepTests.java

dakrone

I'm only partway through, but I wanted to leave some overall feedback to unblock you from continuing to work on this while I continue the rest of the review.

...search/datastreams/lifecycle/transitions/steps/TransportMarkIndexForDLMForceMergeAction.java

dakrone · 2026-02-11T18:18:56Z

...g/elasticsearch/datastreams/lifecycle/transitions/steps/MarkIndexForDLMForceMergeAction.java

+     * Request to mark an index to be force merged.
+     */
+    public static class Request extends MasterNodeRequest<Request> {
+        private final ProjectId projectId;


When I see ProjectId here, it fires a mental alarm that we're passing through too much state. I mentioned elsewhere that the service should be the one that handles cluster state updates, but here we're passing through too much info. I think moving the logic into the service would be better.

Is there an easier way to do this without moving transport logic to the service class as you mentioned?Since the transport action will need that projectId to do the block check.

If we move the logic from the transport action to the service it'd be easy to avoid this kind of awkwardness, but if we don't rely on the transport action, my impression was that it would be difficult to do the throttling/deduplication. If that's the case, do you think it's warranted to just do this in a hacky way?

...g/elasticsearch/datastreams/lifecycle/transitions/steps/MarkIndexForDLMForceMergeAction.java

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

dakrone

I left more comments, and I'm happy to discuss if it helps!

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

dakrone · 2026-02-11T21:01:59Z

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

+        stepContext.executeDeduplicatedRequest(
+            MarkIndexForDLMForceMergeAction.TYPE.name(),
+            markIndexForForceMergeRequest,
+            Strings.format(
+                "DLM service encountered an error when trying to mark index [%s] to be force merged for source index [%s]",
+                indexToBeForceMerged,
+                sourceIndex
+            ),
+            (req, unused) -> markIndexToBeForceMergedCallback(markIndexForForceMergeRequest, stepContext, listener)
+        );


You know, now that I think about this more, I'm not sure we actually need to make the cluster state update a transport action. We're really only making it a separate transport action so that we can use the deduplicator (and thus avoid issuing multiple cluster state updates). Really what we want is something that does something like:

stepContext.executeOnce( <some-object-that-we-use-as-a-key>, "error message because failure", (unused, unused2) -> taskQueue.submitTask("mark-" + indexName + "-ready-for-merge", new MarkIndexAsForceMergeReady(indexName)));

I wonder if we could/should add something to the stepContext that could allow us to do these without having to add the transport bits. Is it going to happen enough as a pattern (where we really want to execute something that is not a transport action, like a CS update) that we'd want to do it?

What do you think @seanzatzdev @lukewhiting ?

Although, we'd also have to thread some kind of task queue through the context so that we could submit things to it from steps.

I think we will have to do a fair bit of cluster state updates in the latter steps so adding transport actions for everything could get tedious/messy. Curious to hear Luke's thoughts

Maybe I've just been looking at this too long, but do we actually need this marker in the cluster state? Since the index for force merge will either be a deterministically named clone or the original (if it has 0 replicas already) is that something we can just write a function to check for in later steps and avoid this extra write to cluster state?

I wonder if we could/should add something to the stepContext that could allow us to do these without having to add the transport bits. Is it going to happen enough as a pattern (where we really want to execute something that is not a transport action, like a CS update) that we'd want to do it?

Without the transport action serving as a wrapper (and making use of the transport action deduplicator), is there an easy way to do the throttling? My impression from our previous meetings was that it'd be difficult to implement the throttling without the transport action deduplicator.

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

...g/elasticsearch/datastreams/lifecycle/transitions/steps/MarkIndexForDLMForceMergeAction.java

...search/datastreams/lifecycle/transitions/steps/TransportMarkIndexForDLMForceMergeAction.java

…on in one place

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...g/elasticsearch/datastreams/lifecycle/transitions/steps/MarkIndexForDLMForceMergeAction.java

...treams/src/main/java/org/elasticsearch/datastreams/lifecycle/DataStreamLifecycleService.java

...g/elasticsearch/datastreams/lifecycle/transitions/steps/MarkIndexForDLMForceMergeAction.java

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...treams/src/main/java/org/elasticsearch/datastreams/lifecycle/DataStreamLifecycleService.java

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

...g/elasticsearch/datastreams/lifecycle/transitions/steps/MarkIndexForDLMForceMergeAction.java

.../test/java/org/elasticsearch/datastreams/lifecycle/MarkIndexForDLMForceMergeActionTests.java

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

...g/elasticsearch/datastreams/lifecycle/transitions/steps/MarkIndexForDLMForceMergeAction.java

...search/datastreams/lifecycle/transitions/steps/TransportMarkIndexForDLMForceMergeAction.java

...reams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java

Add clone step to DLM

8ee2c86

elasticsearchmachine added the v9.4.0 label Feb 2, 2026

elasticsearchmachine and others added 9 commits February 2, 2026 06:12

[CI] Auto commit changes from spotless

0183020

Add clone step to DLM

88ceed6

# Conflicts: # modules/data-streams/src/main/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStep.java # modules/data-streams/src/test/java/org/elasticsearch/datastreams/lifecycle/transitions/steps/CloneStepTests.java

[CI] Auto commit changes from spotless

8975131

fix failing tests

c81bcfb

formatting

666f179

Update custom metadata with index for force merge asynchronously

99e6e63

Merge branch 'main' into dlm-clone-step

41f6830

Add additional null checks

342c1b7

clean up test file

f54f575

seanzatzdev marked this pull request as ready for review February 2, 2026 23:23

elasticsearchmachine added the needs:triage Requires assignment of a team area label label Feb 2, 2026

seanzatzdev requested review from Copilot, dakrone and lukewhiting February 2, 2026 23:23

Copilot AI reviewed Feb 2, 2026

View reviewed changes

lukewhiting added :StorageEngine/Data streams Data streams and their lifecycles >non-issue labels Feb 3, 2026

elasticsearchmachine added the Team:StorageEngine label Feb 3, 2026

elasticsearchmachine removed the needs:triage Requires assignment of a team area label label Feb 3, 2026

lukewhiting reviewed Feb 3, 2026

View reviewed changes

seanzatzdev added 2 commits February 3, 2026 10:43

Merge branch 'main' into dlm-clone-step

c3e7e7f

Merge branch 'main' into dlm-clone-step

d4dee8b

dakrone requested changes Feb 3, 2026

View reviewed changes

seanzatzdev added 3 commits February 4, 2026 11:13

Export dlm steps to server module to fix DI/give Guice access

fb81e8c

Add mark_to_force_merge action to non_operator_actions

76cf7dd

remove unnecessary force merge step

bdcf9d7

seanzatzdev and others added 2 commits February 10, 2026 19:35

add tests for marking action

716ee73

[CI] Auto commit changes from spotless

20d6702

lukewhiting requested changes Feb 11, 2026

View reviewed changes

dakrone requested changes Feb 11, 2026

View reviewed changes

seanzatzdev and others added 9 commits February 12, 2026 03:46

address PR feedback, truncate clone names, keep clone request formati…

21720e1

…on in one place

[CI] Auto commit changes from spotless

0642435

pr feedback

ffd9f9d

renaming functions

f5d23e1

wait for active shards on resize req

a85656b

format time logs

6c3bd98

Improved error handling, renames, refactor tests to simplify

d7e92d0

[CI] Auto commit changes from spotless

241b5d7

Merge branch 'main' into dlm-clone-step

b23456d

seanzatzdev requested a review from Copilot February 17, 2026 21:32

Copilot started reviewing on behalf of seanzatzdev February 17, 2026 21:33 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

small fixes

c004bbb

seanzatzdev requested a review from Copilot February 20, 2026 21:48

Copilot started reviewing on behalf of seanzatzdev February 20, 2026 21:49 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

seanzatzdev added 2 commits February 20, 2026 17:44

pr feedback

754cfd9

Merge branch 'main' into dlm-clone-step

4d72f77

seanzatzdev requested a review from Copilot February 20, 2026 22:46

Copilot started reviewing on behalf of seanzatzdev February 20, 2026 22:47 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

seanzatzdev added 2 commits February 20, 2026 18:47

remove truncated prefix from clone index name, nit

29e212f

Merge branch 'main' into dlm-clone-step

176cead

seanzatzdev requested review from dakrone and lukewhiting February 21, 2026 00:09

Comments

Conversation

seanzatzdev commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

elasticsearchmachine commented Feb 3, 2026

Uh oh!

lukewhiting commented Feb 3, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seanzatzdev Feb 3, 2026 • 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!

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!

dakrone left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seanzatzdev Feb 5, 2026 • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lukewhiting left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dakrone left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seanzatzdev Feb 14, 2026 • 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!

Uh oh!

seanzatzdev commented Feb 2, 2026 •

edited

Loading

seanzatzdev Feb 3, 2026 •

edited

Loading

seanzatzdev Feb 5, 2026 •

edited

Loading

seanzatzdev Feb 14, 2026 •

edited

Loading