romange / helio

A modern framework for backend development based on io_uring Linux interface
Apache License 2.0
435 stars 49 forks source link

feat(aws): remove crc32 checksum #152

Closed andydunstall closed 11 months ago

andydunstall commented 11 months ago

Using the CRC32 header works on MinIO, though running again on S3 before merging the dragonfly changes found S3 returning:

Checksum Type mismatch occurred, expected checksum Type: null, actual checksum Type: crc32

I don't understand why given it has headers:

x-amz-sdk-checksum-algorithm: CRC32\r\n
x-amz-checksum-crc32: uDr/9A==\r\n

So I don't know why it expects no checksum... I'm fairly certain it worked before, though maybe I was only testing MinIO (another argument for regression tests using S3 directly)

Will remove for now just to unblock merging the Dragonfly side (it was to improve resource usage rather than a functional requirement) - will look at next

andydunstall commented 11 months ago

(I tested on the AWS SDK with no patches and that seems to fail too...)

romange commented 11 months ago

weird. please see aws s3api put-object --help using awscli. I remember I used its verbosity/debugging printings to dive deep into the API. https://docs.aws.amazon.com/cli/latest/userguide/getting-started-install.html

andydunstall commented 11 months ago

Ah oops I misunderstood how checksums work - you have to include the CRC32 in the 'create upload' request - then each CRC32 part in the final completion request (so yeah must have been only testing against MinIO and forgot to try against S3 itself...)

(Still seeing ~25% CPU improvement over MD5)

codecov-commenter commented 11 months ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (51d32b0) 77.46% compared to head (16af4d0) 77.46%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #152 +/- ## ======================================= Coverage 77.46% 77.46% ======================================= Files 98 98 Lines 7166 7166 ======================================= Hits 5551 5551 Misses 1615 1615 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.