Skip to content

[fix] retry with checksum result in failure#543

Merged
TingDaoK merged 9 commits intomainfrom
checksum-retry
Aug 12, 2025
Merged

[fix] retry with checksum result in failure#543
TingDaoK merged 9 commits intomainfrom
checksum-retry

Conversation

@TingDaoK
Copy link
Copy Markdown
Contributor

@TingDaoK TingDaoK commented Aug 8, 2025

Issue #, if available:

  • When retry happens in the middle of reading from the stream, the previous checksum stream will calculate the checksum with how far it read through, and since we now keep the checksum through retry, the next time, it will try to write to the same buffer and result in failure.

Description of changes:

  • The issue was uncovered by the reuse of checksum, but it's a race condition for this to happen before head.
  • Updated the checksum stream API to explicit to finalize the checksum, instead of reaching to EOS or stream destroyed.
  • The chunked stream will finalize the checksum when it read to the EOS,
  • Put a lock in the checksum context to make sure it works with different threads.
  • Avoid accessing the synced_data directly for checksum context to avoid possible dead lock with the new lock introduced.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 8, 2025

Codecov Report

❌ Patch coverage is 76.25000% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.60%. Comparing base (c27e829) to head (ab48d06).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
source/s3_checksum_stream.c 50.00% 12 Missing ⚠️
source/s3_checksum_context.c 78.12% 7 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #543      +/-   ##
==========================================
- Coverage   88.70%   88.60%   -0.11%     
==========================================
  Files          22       22              
  Lines        6727     6755      +28     
==========================================
+ Hits         5967     5985      +18     
- Misses        760      770      +10     
Files with missing lines Coverage Δ
source/s3_auto_ranged_put.c 92.10% <100.00%> (+0.02%) ⬆️
source/s3_chunk_stream.c 80.82% <100.00%> (ø)
source/s3_request_messages.c 73.70% <100.00%> (+0.15%) ⬆️
source/s3_checksum_context.c 88.23% <78.12%> (-6.85%) ⬇️
source/s3_checksum_stream.c 65.62% <50.00%> (-4.87%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@TingDaoK TingDaoK changed the title why we try to finalize the checksum on error? [fix] retry with checksum result in failure Aug 11, 2025
@TingDaoK TingDaoK marked this pull request as ready for review August 11, 2025 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants