-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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 bug with BigIntArray
serialization
#90142
Fix bug with BigIntArray
serialization
#90142
Conversation
Pinging @elastic/es-distributed (Team:Distributed) |
This fixed a bug with the `BigIntArray` serialization that I wrote in elastic#89668 where it'd skip the entire final block if we used all of it.
...r/src/test/java/org/elasticsearch/common/bytes/ReleasableBytesReferenceStreamInputTests.java
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/common/io/stream/AbstractStreamTests.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -54,11 +54,17 @@ public void writeTo(StreamOutput out) throws IOException { | |||
} | |||
int intSize = (int) size; | |||
out.writeVInt(intSize * 4); | |||
int lastPageEnd = intSize % INT_PAGE_SIZE; | |||
if (lastPageEnd == 0) { | |||
for (int i = 0; i < pages.length; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: I guess you could also do for (page : pages) {
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* main: (186 commits) [DOCS] Add 8.5 release notes and fix links (elastic#90201) Mute DownsampleActionSingleNodeTests.testCannotRollupWhileOtherRollupInProgress (elastic#90198) Bump to version 8.6.0 Increase the minimum size of the management pool to 2 (elastic#90193) Speed `getIntLE` from `BytesReference` (elastic#90147) Restrict nodes for testClusterPrimariesActive1 (elastic#90191) Fix bug with `BigIntArray` serialization (elastic#90142) Synthetic _source: test _source filtering (elastic#90138) Modernize cardinality agg tests (elastic#90114) Mute failing test (elastic#90186) Move assertion in ES85BloomFilterPostingsFormat to fix test (elastic#90150) Restrict nodes for testClusterPrimariesActive2 (elastic#90184) Batch index delete cluster state updates (elastic#90033) Register stable plugins in ActionModule (elastic#90067) Mute failing test (elastic#90180) [HealthAPI] Disk: Use _ for diagnosis id (elastic#90179) [HealtAPI] Disk: use shorter help URLs (elastic#90178) Fixing disk health indicator unit tests (elastic#90175) Enable the health node and the disk health indicator elastic#84811 (elastic#90085) Add missing Disk Indicator health api IDs (elastic#90174) ...
* main: (121 commits) [DOCS] Add 8.5 release notes and fix links (elastic#90201) Mute DownsampleActionSingleNodeTests.testCannotRollupWhileOtherRollupInProgress (elastic#90198) Bump to version 8.6.0 Increase the minimum size of the management pool to 2 (elastic#90193) Speed `getIntLE` from `BytesReference` (elastic#90147) Restrict nodes for testClusterPrimariesActive1 (elastic#90191) Fix bug with `BigIntArray` serialization (elastic#90142) Synthetic _source: test _source filtering (elastic#90138) Modernize cardinality agg tests (elastic#90114) Mute failing test (elastic#90186) Move assertion in ES85BloomFilterPostingsFormat to fix test (elastic#90150) Restrict nodes for testClusterPrimariesActive2 (elastic#90184) Batch index delete cluster state updates (elastic#90033) Register stable plugins in ActionModule (elastic#90067) Mute failing test (elastic#90180) [HealthAPI] Disk: Use _ for diagnosis id (elastic#90179) [HealtAPI] Disk: use shorter help URLs (elastic#90178) Fixing disk health indicator unit tests (elastic#90175) Enable the health node and the disk health indicator elastic#84811 (elastic#90085) Add missing Disk Indicator health api IDs (elastic#90174) ...
This fixed a bug with the
BigIntArray
serialization that I wrote in #89668 where it'd skip the entire final block if we used all of it.