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

Add a checkCancelled public method to ContextIndexSearcher #121349

Open
scampi opened this issue Jan 30, 2025 · 5 comments
Open

Add a checkCancelled public method to ContextIndexSearcher #121349

scampi opened this issue Jan 30, 2025 · 5 comments
Labels
>enhancement help wanted adoptme :Search Foundations/Search Catch all for Search Foundations Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch

Comments

@scampi
Copy link
Contributor

scampi commented Jan 30, 2025

Description

The ContextIndexSearcher has some API to deal with cancelled tasks, see for example hasCancellations() below. Acting on cancelled tasks can be useful in ValueFetchers (which can be implemented by plugins via custom runtime fields), where resources can be released in such a case.
Although the method hasCancellations() is public and accessible to plugins, there is however no access point to its counterpart checkCancelled() (see below): that method would be needed to check if the search's task got cancelled (and act upon it).

  • Would it be possible to add a public access method to checkCancelled(), in order to allow plugins to handle cancellations in custom runtime fields ?
  • Related question: are IndexSearchers used in Elasticsearch all ContextIndexSearcher instances ?

public abstract ValueFetcher valueFetcher(SearchExecutionContext context, @Nullable String format);

@scampi scampi added >enhancement needs:triage Requires assignment of a team area label labels Jan 30, 2025
@scampi
Copy link
Contributor Author

scampi commented Jan 30, 2025

Maybe an easier access would be to have an access point to SearchContext#isCancelled from SearchExecutionContext ?

@pxsalehi pxsalehi added :Search Foundations/Search Catch all for Search Foundations and removed needs:triage Requires assignment of a team area label labels Feb 10, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch label Feb 10, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-foundations (Team:Search Foundations)

@javanna
Copy link
Member

javanna commented Feb 12, 2025

Hey @scampi the fact that we expose ContextIndexSearcher#hasCancellations is a bit of a coincidence. As far as I can tell we use it only in tests. I would suggest relying instead on SearchContext#getCancellationChecks. We don't really have a super convenient isCancelled method that you can call, but you can reliably loop over the runnable that getCancellationChecks returns and handle potential exceptions that may be thrown by them to signal cancellation or timeout. Would that work for you?

@scampi
Copy link
Contributor Author

scampi commented Mar 15, 2025

sorry for the late reply @javanna

I would suggest relying instead on SearchContext#getCancellationChecks

This would be nice, however SearchContext is not accessible from a SearchExecutionContext.

  • Within the MappedFieldType#valueFetcher we can only access the SearchExecutionContext.
  • A bit similar for MappedFieldType#fielddataBuilder where the FieldDataContext doesn't give access to the SearchContext in order to react upon cancellation.

@javanna
Copy link
Member

javanna commented Mar 17, 2025

Thanks for clarifying @scampi , that makes sense to me.

@javanna javanna added help wanted adoptme and removed feedback_needed labels Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement help wanted adoptme :Search Foundations/Search Catch all for Search Foundations Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch
Projects
None yet
Development

No branches or pull requests

4 participants