Skip to content

FunctionScoreQuery have a direct reference to QueryShardContext #73925

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

Closed
jimczi opened this issue Jun 8, 2021 · 3 comments · Fixed by #74060
Closed

FunctionScoreQuery have a direct reference to QueryShardContext #73925

jimczi opened this issue Jun 8, 2021 · 3 comments · Fixed by #74060
Labels
>bug >regression :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team

Comments

@jimczi
Copy link
Contributor

jimczi commented Jun 8, 2021

FunctionScoreQuery can be cached on a per-segment basis in the Lucene query cache. However a recent refactoring made them depend on the QueryShardContext so the query cache can now keep references to old index readers.
That should be considered as a leak since this memory is not taken into account in the cache size.

@jimczi jimczi added >bug >regression :Search/Search Search-related issues that do not fall into other categories labels Jun 8, 2021
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Jun 8, 2021
@elasticmachine
Copy link
Collaborator

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

@jimczi
Copy link
Contributor Author

jimczi commented Jun 8, 2021

One additional comment here is whether we should allow FunctionScoreQuery to be cached. They are meant to produce a score so seeing them in the query cache would be surprising.

@romseygeek
Copy link
Contributor

I think we have two options here:

  • add an isCacheable() method on score functions so that we can choose whether or not to cache a score function query depending on what function it's using
  • just disable cacheing entirely

Note that we already have some tests for whether or not this is cacheable at the request layer, but that checks for things like whether NOW has been accessed on the execution context so is at a different level.

jimczi added a commit to jimczi/elasticsearch that referenced this issue Jun 14, 2021
This commit disables the query cache for the `FunctionScoreQuery` and the
`ScriptScoreQuery`. These queries are not meant to be cached. If the score
is not needed, we'll now cache the sub-query and filters independently since
we don't want to keep an unused script in the cache.

Closes elastic#73925
jimczi added a commit that referenced this issue Jun 16, 2021
This commit disables the query cache for the `FunctionScoreQuery` and the
`ScriptScoreQuery`. These queries are not meant to be cached. If the score
is not needed, we'll now cache the sub-query and filters independently since
we don't want to keep an unused script in the cache.

Closes #73925
jimczi added a commit that referenced this issue Jun 16, 2021
This commit disables the query cache for the `FunctionScoreQuery` and the
`ScriptScoreQuery`. These queries are not meant to be cached. If the score
is not needed, we'll now cache the sub-query and filters independently since
we don't want to keep an unused script in the cache.

Closes #73925
jimczi added a commit that referenced this issue Jun 16, 2021
This commit disables the query cache for the `FunctionScoreQuery` and the
`ScriptScoreQuery`. These queries are not meant to be cached. If the score
is not needed, we'll now cache the sub-query and filters independently since
we don't want to keep an unused script in the cache.

Closes #73925
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug >regression :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants