Skip to content

[CI] Fix IndexShardTests.testReentrantEngineReadLockAcquisitionInRefreshListener #126685

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

Merged
merged 4 commits into from
Apr 11, 2025

Conversation

tlrx
Copy link
Member

@tlrx tlrx commented Apr 11, 2025

I suspect the test resets/closes the reference manager between the
refresh and the retrieval of the segment generation after the refresh.

By executing segmentGenerationAfterRefresh while holding the engine
reset lock we make sure there are no concurrent engine resets meanwhile.

In the future, we should also ensure that IndexShard.refresh() uses withEngine.

Closes #126628

…eshListener

I suspect the test resets/closes the reference manager
between the refresh and the retrieval of the segment
generation after the refresh.

By executing segmentGenerationAfterRefresh while
holding the engine reset lock we make sure there
are no concurrent engine resets meanwhile.

In the future, we should also ensure that
IndexShard.refresh() uses withEngine.

Closes elastic#126628
@tlrx tlrx added >test Issues or PRs that are addressing/adding tests :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. v9.1.0 labels Apr 11, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Distributed Indexing Meta label for Distributed Indexing team label Apr 11, 2025
@tlrx tlrx requested review from fcofdez and arteam April 11, 2025 13:26
Copy link
Contributor

@arteam arteam left a comment

Choose a reason for hiding this comment

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

I couldn't reproduce the failure locally, but it makes to me to call segmentGenerationAfterRefresh while holding the reset lock

Copy link
Contributor

@fcofdez fcofdez left a comment

Choose a reason for hiding this comment

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

LGTM

@tlrx tlrx added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Apr 11, 2025
@tlrx tlrx merged commit f57be54 into elastic:main Apr 11, 2025
16 of 17 checks passed
@tlrx tlrx deleted the 2025/04/11/fix-126628 branch April 11, 2025 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. Team:Distributed Indexing Meta label for Distributed Indexing team >test Issues or PRs that are addressing/adding tests v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] IndexShardTests testReentrantEngineReadLockAcquisitionInRefreshListener failing
4 participants