fix(rst): multipart upload checksum

[swartzn]

• Add the missing checksum algorithm to CreateMultipartUpload input.
• Use CRC32C instead of SHA-256 for multipart upload checksums because CRC32C is,
  • designed for integrity and error detection.
  • commonly used for metadata checksums.
  • significantly faster than SHA-256.

What does this PR do / why do we need it?

Required for all PRs.

LocalStack which emulates AWS was failing the complete step for multipart uploads with the error:

error completing job: operation error S3: CompleteMultipartUpload, https response error StatusCode: 400, RequestID: b03d2203-3f7c-4eca-9d67-0796da241230, HostID: s9lzHYrFp76ZVxRcpX9+5cjAnEH2ROuNkd2BHfIa6UkFVdtjf5mKR3/eTPFvsiP/XV/VLi31234=, api error InvalidPart: One or more of the specified parts could not be found. The part may not have been uploaded, or the specified entity tag may not match the part's entity tag.

The parts were verified to be upload with the expected part number and entity tag. I also confirmed this behavior manually.

AWS documentation says the checksum algorithm must be the same for all parts and it match the checksum value supplied in the CreateMultipartUpload request. Adding ChecksumAlgorithm to the CreateMultipartUploadInput resolves this issue and inconsistency.

Related Issue(s)

Required when applicable.

Where should the reviewer(s) start reviewing this?

Only required for larger PRs when this may not be immediately obvious.

Are there any specific topics we should discuss before merging?

Not required.

What are the next steps after this PR?

Not required.

Checklist before merging:

Required for all PRs.

When creating a PR these are items to keep in mind that cannot be checked by GitHub actions:

• Documentation:
  • Does developer documentation (code comments, readme, etc.) need to be added or updated?
  • Does the user documentation need to be expanded or updated for this change?
• Testing:
  • Does this functionality require changing or adding new unit tests?
  • Does this functionality require changing or adding new integration tests?
• Git Hygiene:
  • Do commits adhere to Conventional Commits?
  • Is the commit history squashed down reasonably?

For more details refer to the Go coding standards and the pull request process.

[swartzn]

// Comparison of crc32c, crc64nvme, and sha256.
//   - crc32c is common for file consistency and integrity. It's used in metadata and journals (e.g.xfs and ext4)
//   - crc64nvme is AWS S3's CRC64 implementation and is there default hasher.
//   - sha256
//
// Example Benchmark
//
//	cpu: Intel(R) Core(TM) Ultra 5 135U
//	BenchmarkChecksumRelative/1MiB/crc32c----14         	   36085	     33094 ns/op	31684.54 MB/s	       0 B/op	       0 allocs/op
//	BenchmarkChecksumRelative/1MiB/crc64nvme-14         	    2599	    464879 ns/op	2255.59 MB/s	       0 B/op	       0 allocs/op
//	BenchmarkChecksumRelative/1MiB/sha256----14         	    2256	    495992 ns/op	2114.10 MB/s	       0 B/op	       0 allocs/op
//	BenchmarkChecksumRelative/8MiB/crc32c----14         	    3634	    333560 ns/op	25148.70 MB/s	       0 B/op	       0 allocs/op
//	BenchmarkChecksumRelative/8MiB/crc64nvme-14         	     316	   3849870 ns/op	2178.93 MB/s	       0 B/op	       0 allocs/op
//	BenchmarkChecksumRelative/8MiB/sha256----14         	     298	   3955709 ns/op	2120.63 MB/s	       0 B/op	       0 allocs/op
//	BenchmarkChecksumRelative/1024MiB/crc32c----14      	      19	  62318611 ns/op	17229.87 MB/s	       0 B/op	       0 allocs/op
//	BenchmarkChecksumRelative/1024MiB/crc64nvme-14      	       3	 490528340 ns/op	2188.95 MB/s	       0 B/op	       0 allocs/op
//	BenchmarkChecksumRelative/1024MiB/sha256----14      	       2	 511649848 ns/op	2098.59 MB/s	       0 B/op	       0 allocs/op
//
//	 7.11% crc32c vs crc64nvme @ 1MiB
//	 6.67% crc32c vs sha256 @ 1MiB
//	 8.66% crc32c vs crc64nvme @ 8MiB
//	 8.43% crc32c vs sha256 @ 8MiB
//	12.70% crc32c vs crc64nvme @ 1024MiB
//	12.21% crc32c vs sha256 @ 1024MiB
func BenchmarkChecksumRelative(b *testing.B) {
	sizes := []int{1 << 20, 8 << 20, 1 << 30}

	// Allocate once; slice per size.
	maxSize := sizes[len(sizes)-1]
	payload := make([]byte, maxSize)
	for i := range payload {
		payload[i] = byte(i)
	}

	crc32cTable := crc32.MakeTable(crc32.Castagnoli)
	crc64NvmeTable := crc64.MakeTable(0x9a6c9329ac4bc9b5)

	for _, size := range sizes {
		data := payload[:size]

		b.Run(fmt.Sprintf("%dMiB", size>>20), func(b *testing.B) {
			b.Run("crc32c---", func(b *testing.B) {
				h := crc32.New(crc32cTable)
				b.SetBytes(int64(size))
				b.ReportAllocs()
				b.ResetTimer()

				for i := 0; i < b.N; i++ {
					h.Reset()
					h.Write(data)
					h.Sum32()
				}
			})

			b.Run("crc64nvme", func(b *testing.B) {
				h := crc64.New(crc64NvmeTable)
				b.SetBytes(int64(size))
				b.ReportAllocs()
				b.ResetTimer()

				for i := 0; i < b.N; i++ {
					h.Reset()
					h.Write(data)
					h.Sum64()
				}
			})

			b.Run("sha256---", func(b *testing.B) {
				b.SetBytes(int64(size))
				b.ReportAllocs()
				b.ResetTimer()

				for i := 0; i < b.N; i++ {
					sha256.Sum256(data)
				}
			})
		})
	}
}
'''

[iamjoemccormick]

Discussed and we need to keep in mind upgrades/downgrades.

We need to use the same checksum type across the whole job so as part of the WorkRequest Remote needs to communicate what checksum it used when setting up the multi-part upload. That way upgrades where some parts already exist continue to use SHA256 checksums for the remaining parts.

We also need to test and understand what happens if we downgrade while there are jobs using CRC32C checksums in the system. Basically how do older Remote/Sync nodes handle the SHA256 checksum field just being empty? In theory they would just ignore this and it would depend on the S3 provider.