Skip to content

Conversation

romseygeek
Copy link
Contributor

Lucene Query objects can hang around for longer than a single search, for example as
the key in a Query cache. This means that they should not hold references to anything
that is only expected to exist for the duration of a single search request.

SourceConfirmedTextQuery holds a lambda to load data during query execution. This
commit changes one of the constructor calls to avoid capturing unnecessary references
to the SearchExecutionContext, an object which is expected to be short-lived.

@elasticsearchmachine
Copy link
Collaborator

Hi @romseygeek, I've created a changelog YAML for you.

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

@romseygeek
Copy link
Contributor Author

I'm not really sure how to test this one, other than running a bunch of queries against a match_only_text field and then checking a heap dump on the server - suggestions welcome!

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Agreed, this is difficult to test. The other way to make this unlikely to happen again is to not pass down the context to getValueFetcherProvider(...). But it is used there and that would require a bit more work.

@romseygeek romseygeek added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport Automatically create backport pull requests when merged labels Sep 17, 2025
@elasticsearchmachine elasticsearchmachine merged commit dd4b8d9 into elastic:main Sep 17, 2025
34 checks passed
@romseygeek romseygeek deleted the bug/sctq-execution-context-leak branch September 17, 2025 14:57
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.19
9.1

romseygeek added a commit to romseygeek/elasticsearch that referenced this pull request Sep 17, 2025
…TextQuery (elastic#134887)

Lucene Query objects can hang around for longer than a single search,
for example as the key in a Query cache.  This means that they should
not hold references to anything that is only expected to exist for the
duration of a single search request.

SourceConfirmedTextQuery holds a lambda to load data during query
execution.  This commit changes one of the constructor calls to avoid
capturing unnecessary references to the SearchExecutionContext, an
object which is expected to be short-lived.
elasticsearchmachine pushed a commit that referenced this pull request Sep 17, 2025
…TextQuery (#134887) (#134913)

Lucene Query objects can hang around for longer than a single search,
for example as the key in a Query cache.  This means that they should
not hold references to anything that is only expected to exist for the
duration of a single search request.

SourceConfirmedTextQuery holds a lambda to load data during query
execution.  This commit changes one of the constructor calls to avoid
capturing unnecessary references to the SearchExecutionContext, an
object which is expected to be short-lived.
elasticsearchmachine pushed a commit that referenced this pull request Sep 17, 2025
…TextQuery (#134887) (#134914)

Lucene Query objects can hang around for longer than a single search,
for example as the key in a Query cache.  This means that they should
not hold references to anything that is only expected to exist for the
duration of a single search request.

SourceConfirmedTextQuery holds a lambda to load data during query
execution.  This commit changes one of the constructor calls to avoid
capturing unnecessary references to the SearchExecutionContext, an
object which is expected to be short-lived.
gmjehovich pushed a commit to gmjehovich/elasticsearch that referenced this pull request Sep 18, 2025
…TextQuery (elastic#134887)

Lucene Query objects can hang around for longer than a single search,
for example as the key in a Query cache.  This means that they should
not hold references to anything that is only expected to exist for the
duration of a single search request.

SourceConfirmedTextQuery holds a lambda to load data during query
execution.  This commit changes one of the constructor calls to avoid
capturing unnecessary references to the SearchExecutionContext, an
object which is expected to be short-lived.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >bug :StorageEngine/Mapping The storage related side of mappings Team:StorageEngine v8.19.5 v9.1.5 v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants