Skip to content
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.

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

Removing invalid assertion from SnapshotLifecycleServiceTests #107736

Merged
merged 1 commit into from Apr 23, 2024

Conversation

masseyke
Copy link
Member

SnapshotLifecycleServiceTests.testPolicyCRUD() gets the number of snapshot tasks that have executed, updates the cluster state to cancel any pending snapshot tasks, and then asserts that the number of tasks that has executed is still the same. The problem is that since the scheduler is running in another thread, in between the time that the number of tasks is grabbed and the time that the cluster state update is complete, another task could have begun executing. So it is possible that when we check the number of tasks again after that, it is higher than it was before. This happens rarely from the command line, but I can reproduce it 100% of the time in the debugger by just putting a breakpoint on the line that captures the number of tasks in the currentCount2 variable. I think it is the assertion that is broken rather than the behavior. This PR removes the bad assertion.
Closes #107506

@masseyke masseyke added >test Issues or PRs that are addressing/adding tests :Data Management/ILM+SLM Index and Snapshot lifecycle management v8.15.0 labels Apr 22, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Data Management Meta label for data/management team label Apr 22, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

Copy link
Contributor

@andreidan andreidan left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing this Keith

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM too

@masseyke masseyke merged commit a5970da into elastic:main Apr 23, 2024
14 checks passed
@masseyke masseyke deleted the fix-SnapshotLifecycleServiceTests branch April 23, 2024 20:27
dnhatn pushed a commit to dnhatn/elasticsearch that referenced this pull request Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/ILM+SLM Index and Snapshot lifecycle management Team:Data Management Meta label for data/management team >test Issues or PRs that are addressing/adding tests v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] SnapshotLifecycleServiceTests testPolicyCRUD failing
4 participants