Skip to content

[ILM]: Fix TSDS unfollow timing with WaitUntilTimeSeriesEndTimePassesStep #128361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Jun 17, 2025
Merged

[ILM]: Fix TSDS unfollow timing with WaitUntilTimeSeriesEndTimePassesStep #128361

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e45b2ef
[ILM] Fix TSDS unfollow timing with WaitUntilTimeSeriesEndTimePassesStep
happysubin May 23, 2025
731d62b
Merge branch 'main' into fix/ilm-unfollow-delay
samxbr May 28, 2025
e759e97
[ILM] Move WaitUntilTimeSeriesEndTimePassesStep before WaitForFollowS…
happysubin May 29, 2025
4634388
style: apply code format to UnfollowAction.java
happysubin Jun 7, 2025
99d6ecf
test: Add integration test for TSDB rollover and CCR sync during ILM …
happysubin Jun 7, 2025
4f2845a
Merge branch 'main' into fix/ilm-unfollow-delay
samxbr Jun 9, 2025
cb5e908
fix: remove println(), line length warning supress
happysubin Jun 9, 2025
08fd178
Add changelog
gmarouli Jun 10, 2025
47f2255
refactor: improve test code quality and readability
happysubin Jun 10, 2025
b053052
Merge branch 'main' into fix/ilm-unfollow-delay
gmarouli Jun 10, 2025
feab346
fix: increase assertBusy timeout
happysubin Jun 11, 2025
9598115
fix: Address flakiness in follower cluster rollover test
happysubin Jun 12, 2025
808af43
Merge branch 'main' into fix/ilm-unfollow-delay
gmarouli Jun 12, 2025
18bcd3d
docs: explain why certain implementations were chosen
happysubin Jun 13, 2025
1ad5d31
Improve ILM rollover comment clarity
happysubin Jun 16, 2025
c0cc30e
Merge branch 'main' into fix/ilm-unfollow-delay
samxbr Jun 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/UnfollowAction.java
View file
Open in desktop
Copy link
Contributor

@samxbr samxbr May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add new integration test in the CCRIndexLifecycleIT test suite to cover this scenario. You can refer to this test as an example for verifying the WaitUntilTimeSeriesEndTimePassesStep. Essentially we would want to verify that after leader index rollovers, the follower index goes into the WaitUntilTimeSeriesEndTimePassesStep, and new documents to the leader index are synced to the follower index until end_time has passed.

Copy link
Contributor Author

@happysubin happysubin May 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the code review.
I'll review the existing test cases and work on adding new ones.I'm going to add test code!

Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.xpack.core.ilm.Step.StepKey;

import java.io.IOException;
import java.time.Instant;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -45,6 +46,7 @@ public List<Step> toSteps(Client client, String phase, StepKey nextStepKey) {
StepKey preUnfollowKey = new StepKey(phase, NAME, CONDITIONAL_UNFOLLOW_STEP);
StepKey indexingComplete = new StepKey(phase, NAME, WaitForIndexingCompleteStep.NAME);
StepKey waitForFollowShardTasks = new StepKey(phase, NAME, WaitForFollowShardTasksStep.NAME);
StepKey waitUntilTimeSeriesEndTimePassesStep = new StepKey(phase, NAME, WaitUntilTimeSeriesEndTimePassesStep.NAME);
StepKey pauseFollowerIndex = new StepKey(phase, NAME, PauseFollowerIndexStep.NAME);
StepKey closeFollowerIndex = new StepKey(phase, NAME, CloseFollowerIndexStep.NAME);
StepKey unfollowFollowerIndex = new StepKey(phase, NAME, UnfollowFollowerIndexStep.NAME);
Expand All @@ -60,13 +62,22 @@ public List<Step> toSteps(Client client, String phase, StepKey nextStepKey) {
return customIndexMetadata == null;
});
WaitForIndexingCompleteStep step1 = new WaitForIndexingCompleteStep(indexingComplete, waitForFollowShardTasks);
WaitForFollowShardTasksStep step2 = new WaitForFollowShardTasksStep(waitForFollowShardTasks, pauseFollowerIndex, client);
PauseFollowerIndexStep step3 = new PauseFollowerIndexStep(pauseFollowerIndex, closeFollowerIndex, client);
CloseFollowerIndexStep step4 = new CloseFollowerIndexStep(closeFollowerIndex, unfollowFollowerIndex, client);
UnfollowFollowerIndexStep step5 = new UnfollowFollowerIndexStep(unfollowFollowerIndex, openFollowerIndex, client);
OpenIndexStep step6 = new OpenIndexStep(openFollowerIndex, waitForYellowStep, client);
WaitForIndexColorStep step7 = new WaitForIndexColorStep(waitForYellowStep, nextStepKey, ClusterHealthStatus.YELLOW);
return List.of(conditionalSkipUnfollowStep, step1, step2, step3, step4, step5, step6, step7);
WaitForFollowShardTasksStep step2 = new WaitForFollowShardTasksStep(
waitForFollowShardTasks,
waitUntilTimeSeriesEndTimePassesStep,
client
);
WaitUntilTimeSeriesEndTimePassesStep step3 = new WaitUntilTimeSeriesEndTimePassesStep(
waitUntilTimeSeriesEndTimePassesStep,
pauseFollowerIndex,
Instant::now
);
Copy link
Contributor

@samxbr samxbr May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WaitUntilTimeSeriesEndTimePassesStep should be prior to WaitForFollowShardTasksStep, because the follower index can sync with the leader index one last time after the end_time has passed, to make sure there's no new docs coming in to the leader index.

PauseFollowerIndexStep step4 = new PauseFollowerIndexStep(pauseFollowerIndex, closeFollowerIndex, client);
CloseFollowerIndexStep step5 = new CloseFollowerIndexStep(closeFollowerIndex, unfollowFollowerIndex, client);
UnfollowFollowerIndexStep step6 = new UnfollowFollowerIndexStep(unfollowFollowerIndex, openFollowerIndex, client);
OpenIndexStep step7 = new OpenIndexStep(openFollowerIndex, waitForYellowStep, client);
WaitForIndexColorStep step8 = new WaitForIndexColorStep(waitForYellowStep, nextStepKey, ClusterHealthStatus.YELLOW);
return List.of(conditionalSkipUnfollowStep, step1, step2, step3, step4, step5, step6, step7, step8);
}

@Override
Expand Down
31 changes: 18 additions & 13 deletions x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/UnfollowActionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,17 @@ public void testToSteps() {
);
List<Step> steps = action.toSteps(null, phase, nextStepKey);
assertThat(steps, notNullValue());
assertThat(steps.size(), equalTo(8));
assertThat(steps.size(), equalTo(9));

StepKey expectedFirstStepKey = new StepKey(phase, UnfollowAction.NAME, UnfollowAction.CONDITIONAL_UNFOLLOW_STEP);
StepKey expectedSecondStepKey = new StepKey(phase, UnfollowAction.NAME, WaitForIndexingCompleteStep.NAME);
StepKey expectedThirdStepKey = new StepKey(phase, UnfollowAction.NAME, WaitForFollowShardTasksStep.NAME);
StepKey expectedFourthStepKey = new StepKey(phase, UnfollowAction.NAME, PauseFollowerIndexStep.NAME);
StepKey expectedFifthStepKey = new StepKey(phase, UnfollowAction.NAME, CloseFollowerIndexStep.NAME);
StepKey expectedSixthStepKey = new StepKey(phase, UnfollowAction.NAME, UnfollowFollowerIndexStep.NAME);
StepKey expectedSeventhStepKey = new StepKey(phase, UnfollowAction.NAME, OPEN_FOLLOWER_INDEX_STEP_NAME);
StepKey expectedEighthStepKey = new StepKey(phase, UnfollowAction.NAME, WaitForIndexColorStep.NAME);
StepKey expectedFourthStepKey = new StepKey(phase, UnfollowAction.NAME, WaitUntilTimeSeriesEndTimePassesStep.NAME);
StepKey expectedFifthStepKey = new StepKey(phase, UnfollowAction.NAME, PauseFollowerIndexStep.NAME);
StepKey expectedSixthStepKey = new StepKey(phase, UnfollowAction.NAME, CloseFollowerIndexStep.NAME);
StepKey expectedSeventhStepKey = new StepKey(phase, UnfollowAction.NAME, UnfollowFollowerIndexStep.NAME);
StepKey expectedEighthStepKey = new StepKey(phase, UnfollowAction.NAME, OPEN_FOLLOWER_INDEX_STEP_NAME);
StepKey expectedNinthStepKey = new StepKey(phase, UnfollowAction.NAME, WaitForIndexColorStep.NAME);

BranchingStep firstStep = (BranchingStep) steps.get(0);
assertThat(firstStep.getKey(), equalTo(expectedFirstStepKey));
Expand All @@ -73,26 +74,30 @@ public void testToSteps() {
assertThat(thirdStep.getKey(), equalTo(expectedThirdStepKey));
assertThat(thirdStep.getNextStepKey(), equalTo(expectedFourthStepKey));

PauseFollowerIndexStep fourthStep = (PauseFollowerIndexStep) steps.get(3);
WaitUntilTimeSeriesEndTimePassesStep fourthStep = (WaitUntilTimeSeriesEndTimePassesStep) steps.get(3);
assertThat(fourthStep.getKey(), equalTo(expectedFourthStepKey));
assertThat(fourthStep.getNextStepKey(), equalTo(expectedFifthStepKey));

CloseFollowerIndexStep fifthStep = (CloseFollowerIndexStep) steps.get(4);
PauseFollowerIndexStep fifthStep = (PauseFollowerIndexStep) steps.get(4);
assertThat(fifthStep.getKey(), equalTo(expectedFifthStepKey));
assertThat(fifthStep.getNextStepKey(), equalTo(expectedSixthStepKey));

UnfollowFollowerIndexStep sixthStep = (UnfollowFollowerIndexStep) steps.get(5);
CloseFollowerIndexStep sixthStep = (CloseFollowerIndexStep) steps.get(5);
assertThat(sixthStep.getKey(), equalTo(expectedSixthStepKey));
assertThat(sixthStep.getNextStepKey(), equalTo(expectedSeventhStepKey));

OpenIndexStep seventhStep = (OpenIndexStep) steps.get(6);
UnfollowFollowerIndexStep seventhStep = (UnfollowFollowerIndexStep) steps.get(6);
assertThat(seventhStep.getKey(), equalTo(expectedSeventhStepKey));
assertThat(seventhStep.getNextStepKey(), equalTo(expectedEighthStepKey));

WaitForIndexColorStep eighthStep = (WaitForIndexColorStep) steps.get(7);
assertThat(eighthStep.getColor(), is(ClusterHealthStatus.YELLOW));
OpenIndexStep eighthStep = (OpenIndexStep) steps.get(7);
assertThat(eighthStep.getKey(), equalTo(expectedEighthStepKey));
assertThat(eighthStep.getNextStepKey(), equalTo(nextStepKey));
assertThat(eighthStep.getNextStepKey(), equalTo(expectedNinthStepKey));

WaitForIndexColorStep ninth = (WaitForIndexColorStep) steps.get(8);
assertThat(ninth.getColor(), is(ClusterHealthStatus.YELLOW));
assertThat(ninth.getKey(), equalTo(expectedNinthStepKey));
assertThat(ninth.getNextStepKey(), equalTo(nextStepKey));
}

@Override
Expand Down