feat(storage): Add full object checksum validation for appendable uploads#16110
feat(storage): Add full object checksum validation for appendable uploads#16110v-pratap wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements dynamic CRC32C validation for appendable object uploads by updating the connection and writer logic to initialize hash functions from RPC response metadata. It also adds a stateful constructor to the CRC32C hash function. Feedback highlights a critical regression where a large suite of existing integration tests was deleted and replaced with experimental code. Additional suggestions include adding defensive checks for CRC32C field presence, utilizing existing library thread management instead of a custom thread pool in tests, and using environment variables instead of hardcoded strings for test configuration.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16110 +/- ##
==========================================
- Coverage 92.71% 92.71% -0.01%
==========================================
Files 2353 2353
Lines 218986 219098 +112
==========================================
+ Hits 203031 203129 +98
- Misses 15955 15969 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| persisted_state_ = response->persisted_size(); | ||
|
|
||
| if (response->has_persisted_data_checksums()) { |
There was a problem hiding this comment.
Is it possible that: if a connection retry is triggered during the session, and the server's reported persisted_size causes a resend that overlaps with (but is not exactly aligned to) the client's previous chunk boundaries, the request fails?
There was a problem hiding this comment.
Yes, the request will fail if we try to send overlapping data.
I think the server strictly requires the next write to start exactly at the persisted_size it has saved.
18389a0 to
58d4b1d
Compare
No description provided.