Skip to content

Reduce copying when creating scroll/PIT ids#99219

Merged
DaveCTurner merged 2 commits intoelastic:mainfrom
DaveCTurner:2023/09/06/search-context-ids-copying
Sep 6, 2023
Merged

Reduce copying when creating scroll/PIT ids#99219
DaveCTurner merged 2 commits intoelastic:mainfrom
DaveCTurner:2023/09/06/search-context-ids-copying

Conversation

@DaveCTurner
Copy link
Copy Markdown
Member

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
Copy Markdown
Collaborator

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

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
Contributor

@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
Copy Markdown
Member 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