Skip to content

Fix IndexStatsIT#testThrottleStats #128049

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix IndexStatsIT#testThrottleStats #128049

wants to merge 2 commits into from
+18 −4

Conversation

arteam
Copy link
Contributor

@arteam arteam commented May 13, 2025

Engine.PauseLock#throttle can be called when the lock is being throttled, so we can't guarantee that all permits are available before throttling.

Resolve #126359
See #127173

@arteam
Fix IndexStatsIT#testThrottleStats
47130ac 
`Engine.PauseLock#throttle` can be called when the lock is being throttled,
so we can't guarantee that all permits are available before throttling.

Resolve elastic#126359
See elastic#127173
@arteam arteam added >test Issues or PRs that are addressing/adding tests :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. labels May 13, 2025
@arteam arteam marked this pull request as ready for review May 13, 2025 19:16
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Indexing Meta label for Distributed Indexing team label May 13, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@ankikuma
Copy link
Contributor

ankikuma commented May 20, 2025 •
edited
Loading

Hi Artem, sorry for taking so long to get to this. I am not convinced this is the right fix. We do not call throttle and unthrottle directly on the lock. We go through InternalEngine#activateThrottling and InternalEngine#deactivateThrottling instead, which update a counter to make sure that the throttle and unthrottle calls are matched.
Basically, the callers are expected to follow the semantics that for each activateThrottling call there should be a matching deactivateThrottling call and we throw asserts in the code if this is not the case.
I think the problem might be in the way the ThreadPoolMergeScheduler is calling activateThrottling and deactivateThrottling.

But I am not sure yet where the mismatch might be caused.

@ankikuma ankikuma requested a review from albertzaharovits May 20, 2025 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ankikuma ankikuma Awaiting requested review from ankikuma

@henningandersen henningandersen Awaiting requested review from henningandersen

@albertzaharovits albertzaharovits Awaiting requested review from albertzaharovits

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
: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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CI] IndexStatsIT testThrottleStats failing
3 participants
@arteam @elasticsearchmachine @ankikuma