Fix multipart integrity and reduce encrypted read overhead - #156
Open
ServerSideHannes wants to merge 6 commits into
Open
Fix multipart integrity and reduce encrypted read overhead#156ServerSideHannes wants to merge 6 commits into
ServerSideHannes wants to merge 6 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Multipart replacements could reuse GCM nonces or overwrite accepted bytes before signature validation. Completion could report success for the wrong upload, and stale or unavailable sidecars could select the wrong read path. This change binds publication and reads to an object generation and keeps each verified part attempt immutable.
Validation: the CI unit selection passed 779 tests locally (including 110 mock integration tests); the separate full unit-directory run passed 670 tests including its slow case. 27 real MinIO/HTTP compatibility tests passed (including the optional Redis scenario), 11 additional copy/concurrency tests passed, and all nine native-copy tests passed, including 1,280 MiB objects and concurrent copies. Ruff lint and format checks pass. Tests cover hash tampering below/at/above 8 MiB, rejected replacement, uncertain state publication, retry after failed completion, key selection, out-of-order assembly, resource cleanup, range recovery, and real backend preconditions.
A local ten-sample 32 MiB GET comparison against d45732d measured median latency of 317.12 → 289.26 ms and sampled RSS of 179.23 → 175.06 MiB. A separate 50-GET run stayed below 176 MiB sampled RSS. These are local measurements, not production forecasts; staging adds storage/copy work to ordinary multipart writes.
Deployment: this is a new write format. Drain legacy active uploads and switch the fleet together; old readers cannot read v3 generations. Persistent Redis is required to resume active uploads across restarts. Configure orphan-attempt lifecycle expiry longer than the supported upload/retry window. Generation manifests are retained because copies and versions may reference them. Unsupported checksum-trailer formats fail explicitly. No deployment or bucket-policy changes are included.
See docs/GENERATION_FORMAT.md for the commit protocol, migration requirements, cleanup policy, limitations and benchmark results; docs/CODE_REVIEW.md retains the original findings.
Final CI status: all 11 pull-request checks passed for commit
2c68c39672badc849ac621ae82c4c8fe7e378cec, including both unit jobs, all seven Linux integration shards, Ruff and Helm lint. Test run.