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

Mark TimeLimitingCollector as deprecated #13220

Merged
merged 1 commit into from
Mar 29, 2024
Merged

Mark TimeLimitingCollector as deprecated #13220

merged 1 commit into from
Mar 29, 2024

Conversation

kaivalnp
Copy link
Contributor

Description

Follow-up to #13202 (comment)

TimeLimitingCollector only seems to be used from comments and tests. Can we mark it as @Deprecated and remove from main in another PR?

@vigyasharma
Copy link
Contributor

As an alternate to deprecating this entirely, we could also change it to start using QueryTimeout, instead of its custom time limiting counters. I'd like to get more opinions from the community, on whether we think it's useful to keep a time limiting collector around.

@jpountz
Copy link
Contributor

jpountz commented Mar 26, 2024

It makes sense to me to deprecate it, IndexSearcher#setTimeout should be used instead.

@jpountz
Copy link
Contributor

jpountz commented Mar 27, 2024

@vigyasharma I'd like to double check with you if you're good with deprecating before merging, I'm not sure if your previous comment was candidly checking for consensus, or if you were implicitly raising concerns.

@vigyasharma
Copy link
Contributor

@jpountz I was checking for consensus. I'm aligned with deprecating.

@jpountz
Copy link
Contributor

jpountz commented Mar 29, 2024

Thanks for confirming, I'll merge.

@jpountz jpountz merged commit 878d233 into apache:main Mar 29, 2024
3 checks passed
jpountz pushed a commit that referenced this pull request Mar 29, 2024
Co-authored-by: Kaival Parikh <kaivalp2000@gmail.com>
@kaivalnp kaivalnp deleted the dep branch March 29, 2024 08:58
@kaivalnp
Copy link
Contributor Author

Created a follow-up to remove TimeLimitingCollector from main after deprecation: #13243

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants