Arbitrary bytes in blob store register#96019
Arbitrary bytes in blob store register#96019elasticsearchmachine merged 6 commits intoelastic:mainfrom
Conversation
Today the blob store register supports recording only a `long`, represented as an 8-byte blob. We need to store a little more data in the register, so this commit generalises things to work with a `BytesReference` directly.
|
Pinging @elastic/es-distributed (Team:Distributed) |
fcofdez
left a comment
There was a problem hiding this comment.
Looks good, but I left a question regarding MAX_REGISTER_CONTENT_LENGTH
| // no instances | ||
| } | ||
|
|
||
| public static final int MAX_REGISTER_CONTENT_LENGTH = Long.BYTES; |
There was a problem hiding this comment.
I think this should be greater than Long.BYTES?
There was a problem hiding this comment.
I was intending to do that in a follow-up, to avoid changing too many things here.
| slices.add(new BytesArray(randomByteArrayOfLength(sliceLen))); | ||
| remaining -= sliceLen; | ||
| } | ||
| return CompositeBytesReference.of(slices.toArray(BytesReference[]::new)); |
There was a problem hiding this comment.
Maybe we should generate the other BytesReference variants too?
There was a problem hiding this comment.
IMO this is enough - it means that we make #iterator() yield a fairly random selection of slice sizes, which is where I suspect the most bugs lurk. We'll also occasionally return a BytesArray in the case that there's only one slice.
| if (bytesReference.length() == 0) { | ||
| return 0L; | ||
| } else if (bytesReference.length() == Long.BYTES) { | ||
| try (var baos = new ByteArrayOutputStream(Long.BYTES)) { |
There was a problem hiding this comment.
Don't we have some library functions to deal with this directly?
There was a problem hiding this comment.
Apparently not, or at least I couldn't find any.
fcofdez
left a comment
There was a problem hiding this comment.
LGTM, thanks for the clarifications 👍
|
Well this is frustrating. https://gradle-enterprise.elastic.co/s/jd6eqec3nbvfe looks like a related failure, but I've been running that test in a loop for ages without being able to reproduce it. |
|
I have been running this test in a loop for 20+ hours without another failure. I'm going to ignore it in the name of progress. |
Relates elastic#96019 Closes elastic#96162
Today the blob store register supports recording only a
long, represented as an 8-byte blob. We need to store a little more data in the register, so this commit generalises things to work with aBytesReferencedirectly.