Fix S3 suffix byte-range requests when reading sharded chunks#80
Conversation
726dea6 to
4b3311d
Compare
|
@konstibob thanks for opening this. Is this fix also addressing the bug described in #79 ? #78 will need to be reviewed separately especially given the backwards-incompatible implications for downstream libraries consuming |
4b3311d to
deb1de1
Compare
|
@sbesson Yes it this PR fixes #79 only! this fixes exactly that. Regarding keeping it atomic: Agreed. I've restructured this PR so it now contains only the checksum fix on top of main, with the module-extraction refactoring removed entirely. The fix doesn't depend on that refactor, so this PR is now fully standalone and safe to merge on its own. |
|
Thanks @konstibob but deb1de1 still contains the whole module refactoring history |
deb1de1 to
726dea6
Compare
|
fixed it! |
|
Move testReadSuffix to OnlineS3StoreTest; it only applies to S3Store's suffix-range read, not every store. |
normanrz
left a comment
There was a problem hiding this comment.
Looks good, just minor comments.
Co-authored-by: Norman Rzepka <code@normanrz.com>
Reading a single chunk from a sharded array stored on S3 failed with a checksum
error. To locate a chunk inside a shard, the sharding codec first reads the shard
index at the end of the shard object via a suffix read
(
StoreHandle.read(-indexByteLength)).S3Store.get(long start, ...)always formatted the HTTP range header asbytes=<start>-. For a negative start this produced an invalid header likebytes=-16389-, which S3 does not accept as a suffix range, causing the requestto fail.
Fix
When
startis negative, emit the correct suffix-range form:bytes=-<N>instead of
bytes=<start>-.Testing
Added
testReadSuffixtoStoreTest, verifying that a suffix read returnsexactly the last 10 bytes of the object.