Combine per-part CRCs for whole-object download checksum validation - #663
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #663 +/- ##
==========================================
- Coverage 89.14% 89.11% -0.03%
==========================================
Files 23 23
Lines 8044 8160 +116
==========================================
+ Hits 7171 7272 +101
- Misses 873 888 +15
🚀 New features to boost your workflow:
|
TingDaoK
changed the title
| * Folds the digest of one data block into `head`, so that `head` becomes the checksum of its own data | ||
| * followed by that block, without re-scanning either: | ||
| * | ||
| * head = checksum(block_head) (still running, not finalized) |
There was a problem hiding this comment.
nit: this threw me in for a loop. technically this works because our crc impl is just always in finalized state and the finalize call in s3 code does not do anything.
in general in would not be valid to combine finalized and non-finalized crcs as it would produce wrong result
| * | ||
| * Only worth calling for multipart downloads; with a single request there are no parts to combine. */ | ||
| AWS_S3_API | ||
| int aws_s3_meta_request_setup_checksum_combine_synced( |
There was a problem hiding this comment.
do we know the timings on this? if needed we can probably speed it up further
There was a problem hiding this comment.
[checksum-combine] 9363 combines in 3.293 ms file(s) remaining
[checksum-combine] 65536 combines in 21.310 msile(s) remaining
~350ns per combine operations. So, for the case of small part size on large object, it took 20ms, but not i'd say to be a concern here.
|
|
||
| /* The CRC finalizers write digests big-endian, so read them back the same way. */ | ||
| uint64_t tail_value = 0; | ||
| if (head->digest_size == AWS_CRC32_LEN) { |
There was a problem hiding this comment.
potential optimization idea for future: but for gets we never need to finalize to be and then back. thats needed purely for puts
There was a problem hiding this comment.
yeah, mostly because our current aws_s3_checksum interface is a pointer tied with the request itself, so I want to keep the parts level checksum independent from the request without affecting the lifetime of the request itself to keep it simple.
And the interface only has the checksum_finalize that provides the encoded checksum. and I keep a list of the finalized one with the meta request.
But, the extra encode/decode should be trivial, we can optimize it if we need to.
3 participants
Problem
When a multipart download validates a whole-object checksum, that checksum could only be
built by feeding the object's bytes into a single running checksum in object order. The work
happened in the delivery loop on the meta request's
io_event_loopthread, so when the deliveryis the bottleneck, this added extra cost to the critical path and impacts the overall performance.
Change
CRCs compose: given the digests of two adjacent blocks and the length of the second, the
digest of their concatenation follows in O(1). So for the CRC algorithms each part now
digests its own body on its own connection thread, and the whole-object checksum is assembled
from those per-part digests when the meta request finishes.
Non-combinable algorithms (SHA*, XXHASH*) are unchanged — they still accumulate byte-wise in
the delivery loop, which is what guarantees object order for them.
Moved away from ubuntu 18 for gcc 13, it keeps failing with an unknown crash in libc. I have no idea what is causing the crash. But, since we know that there are fixes for later version of gcc that is not port back to old version of ubuntu, #479, so I decided to try move ubuntu 22 for gcc 13. And it seems to work. :-)
TODO
This can be used for the upload path to provide the full object level checksum to s3.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.