Pin Hyrax version for chunk upload fix - #359
Merged
Merged
Conversation
ShanaLMoore
force-pushed
the
chunk-upload-fix
branch
from
July 10, 2025 19:11
df05851 to
8f78050
Compare
ShanaLMoore
commented
Jul 10, 2025
In multi-pod environments with a shared file system, chunked uploads were failing due to a race condition. The file size was being read from a cache, which could be stale if another pod had recently written a chunk. This resulted in an incorrect size check, causing parts of the file to be overwritten. This commit fixes the issue by changing how the file size is retrieved. It now opens the file for reading to get the current size directly from the file handle, bypassing any filesystem attribute caching. This ensures an accurate size check and prevents file corruption during chunked uploads.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR pins the Hyrax gem to a specific commit and applies a fix for chunked uploads by decorating the UploadsController.
- Switches injection of
bundler.dfrom an environment variable to a symlink in the Dockerfile. - Adds a Bundler override to pin
hyraxto a specific Git ref inbundler.d/example.rb. - Implements a controller decorator to read file size directly and prevent stale cache corruption during chunked uploads.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docker-compose.yml | Removes BUNDLE_BUNDLER_INJECT__GEM_PATH env var and updates volumes |
| Dockerfile | Replaces copying bundler.d with a symlink to /app/.bundler.d |
| bundler.d/example.rb | Pins hyrax gem to commit d6330a1c048bd498da852325a502f8dba0467c11 |
| app/controllers/hyrax/uploads_controller_decorator.rb | Adds decorator to fix a race condition in chunked uploads |
Comments suppressed due to low confidence (3)
app/controllers/hyrax/uploads_controller_decorator.rb:9
- The new chunk upload logic should be covered by unit or integration tests to ensure correct behavior when
begin_of_chunkmatchescurrent_sizeand when it doesn't.
def handle_chunk(content_range, chunk)
docker-compose.yml:16
- Since the
BUNDLE_BUNDLER_INJECT__GEM_PATHenvironment variable is removed, consider mounting thebundler.ddirectory as a volume so that local gem overrides are applied in development.
- BUNDLE_DISABLE_LOCAL_BRANCH_CHECK=true
Dockerfile:5
- [nitpick] Consider adding a comment explaining why a symlink is used instead of copying
bundler.d, so future maintainers understand the reasoning.
RUN ln -s /app/samvera/bundler.d /app/.bundler.d
Refactors the chunked upload process to prevent file corruption in multi-pod environments. - Gets the file size by opening the file (File.open + size) to bypass stale filesystem attribute caches (e.g., on NFS). - Replaces the system sync command with the more precise and idiomatic IO#fsync to ensure each chunk is flushed to disk before the next operation proceeds.
ShanaLMoore
commented
Jul 11, 2025
| ensure_gem "sentry-ruby" | ||
| ensure_gem "sentry-rails" | ||
| ensure_gem "cancancan", "~> 3.0" # cancancan is bundling to v1.17.0 but we need at least 3.0 | ||
|
|
Contributor
Author
There was a problem hiding this comment.
This approach keeps the submodule’s Hyku version pinned to 315925a4 (the version currently deployed to production) while still incorporating the chunk upload fix by specifying the Hyrax version in bundler.d/example.
The decorator code is in hyrax now, sha ab4d108114289822b186579cd73a53d68c145a73
kirkkwang
approved these changes
Jul 11, 2025
Contributor
Author
|
Contributor
Author
Manual
Bulkrax
|
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.
Story
Expected Behavior Before Changes
Unable to upload a file larger than 20MB
Expected Behavior After Changes
The file fully uploads