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

Reduce copying when creating scroll/PIT ids #99219

Merged

Conversation

DaveCTurner
Copy link
Contributor

These IDs can be pretty large at high shard counts. This commit removes
an unnecessary copy while base64-encoding/decoding, specifies ISO_8859_1
so that the unavoidable copy operations work directly on the bytes, and
also skips the temporary creation of an O(#shards)-sized map in favour
of just writing the entries directly and then reading them into a map at
the other end.

These IDs can be pretty large at high shard counts. This commit removes
an unnecessary copy while base64-encoding/decoding, specifies ISO_8859_1
so that the unavoidable copy operations work directly on the bytes, and
also skips the temporary creation of an O(#shards)-sized map in favour
of just writing the entries directly and then reading them into a map at
the other end.
@DaveCTurner DaveCTurner added >enhancement :Search/Search Search-related issues that do not fall into other categories v8.11.0 labels Sep 6, 2023
@elasticsearchmachine elasticsearchmachine added the Team:Search Meta label for search team label Sep 6, 2023
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Member

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

LGTM, I wonder if this is hot enough to justify even more effort. We could go one step further here and avoid one more round of copying by using something like org.apache.commons.io.output.WriterOutputStream but might not be necessary, this is a nice improvement already thanks!

@DaveCTurner DaveCTurner merged commit 4d614b2 into elastic:main Sep 6, 2023
@DaveCTurner DaveCTurner deleted the 2023/09/06/search-context-ids-copying branch September 6, 2023 15:41
@DaveCTurner
Copy link
Contributor Author

I suspect fundamentally we should reconsider the whole concept of IDs that can grow to MiBs in size...

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

Successfully merging this pull request may close these issues.

3 participants