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

Nullify sourceAsMap once a search hit is processed #119734

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

iverase
Copy link
Contributor

@iverase iverase commented Jan 8, 2025

The sourceAsMap can use a significant amount of memory, still it is only hold for caching. Therefore it might make sense to nullify it once a search hit is processed so we free that memory.

@iverase iverase added >non-issue :Search/Search Search-related issues that do not fall into other categories v9.0.0 v8.18.0 labels Jan 8, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Search Meta label for search team label Jan 8, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@iverase iverase added the auto-backport Automatically create backport pull requests when merged label Jan 8, 2025
@iverase iverase merged commit ab77144 into elastic:main Jan 8, 2025
16 checks passed
@iverase iverase deleted the resetMapOfMaps branch January 8, 2025 10:38
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.x

iverase added a commit to iverase/elasticsearch that referenced this pull request Jan 8, 2025
he sourceAsMap can use a significant amount of memory, still it is only hold for caching. 
Therefore it might make sense to nullify it once a search hit is processed so we free that memory.
@javanna
Copy link
Member

javanna commented Jan 8, 2025

Would it be possible to have a test that verifies the new behavior introduced, or perhaps assertions that make sure that sourceMap is reset when needed?

iverase added a commit to iverase/elasticsearch that referenced this pull request Jan 8, 2025
@iverase iverase removed the v8.18.0 label Jan 8, 2025
@iverase
Copy link
Contributor Author

iverase commented Jan 8, 2025

@javanna I sam reverting this change as I think you are right, this change needs some test and assertions. More over there are places where we are calling getSourceAsMap after generating the hits so we need to check if this is affecting those cases.

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 >non-issue :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants