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

Fix incorrect assumptions about BytesReference in StateToProcessWriterHelper #103929

Merged

Conversation

original-brownbear
Copy link
Member

The logic here only works accidentally if the source is backed by a single byte[] with offset 0.
Fixed by using the writeTo API and just working on the bytes reference via its index handling to find the zero byte offset.

Non issue since this isn't causing trouble right now but broke when I tried using this with sliced buffers for #102030

…rHelper

The logic here only works accidentally if the `source` is backed by a
single `byte[]` with offset `0`.
Fixed by using the `writeTo` API and just working on the bytes reference
via its index handling to find the zero byte offset.
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

@elasticsearchmachine elasticsearchmachine added Team:ML Meta label for the ML team v8.13.0 labels Jan 4, 2024
Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for fixing this Armin.

@original-brownbear
Copy link
Member Author

npnp + thanks for the speedy review David! :)

@original-brownbear original-brownbear added the auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jan 4, 2024
@elasticsearchmachine elasticsearchmachine merged commit 0bd8ffe into elastic:main Jan 4, 2024
15 checks passed
@original-brownbear original-brownbear deleted the fix-ml-bytes-ref branch January 4, 2024 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :ml Machine learning >non-issue Team:ML Meta label for the ML team v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants