Upload Media: Enable concurrent sideload uploads#75888
Merged
Merged
Conversation
|
Size Change: -425 B (-0.01%) Total Size: 7.75 MB 📦 View Changed
ℹ️ View Unchanged
|
4512159 to
4d5f668
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Member
Author
|
Needs testing! |
|
Flaky tests detected in 16d9cfe. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24642003849
|
The per-attachment serial constraint on thumbnail sideloads added unnecessary latency during bulk uploads. The existing maxConcurrentUploads limit already bounds total concurrency, and image processing naturally staggers upload start times.
- Add CHANGELOG entry for concurrent sideload behavior change - Remove pauseItem from ActionCreators type (no thunk calls it) - Simplify first concurrent sideload test to use fake parentId like the other tests in the same describe block
4d5f668 to
e847348
Compare
Member
Author
This works well in my manual testing. |
Member
|
Aren't there any race conditions trying to update post meta simultaneously? |
Member
Author
Just saw your comment on the issue and replied there. I'll look into how we can address that. |
Use MySQL advisory locks (GET_LOCK/RELEASE_LOCK) to serialize the read-modify-write cycle for attachment metadata during concurrent sideloads. Without this, parallel sideloads for the same attachment can overwrite each other's metadata changes. The lock is scoped per-attachment and held only during the metadata update (not during file upload). Object cache is invalidated before reading to ensure fresh data from the database.
Delete the sideloaded file from disk when the advisory lock cannot be acquired, preventing orphaned files that would never be linked to attachment metadata.
Replace GET_LOCK/RELEASE_LOCK with WordPress transient locks, matching the pattern used by wp_maybe_generate_attachment_metadata() in core. This aligns with established WordPress conventions and avoids introducing an unprecedented locking mechanism. The transient lock spins with 50ms pauses until available or a 5-second timeout is reached. The transient auto-expires as a safety net against stale locks from crashed processes.
Member
Author
@swissspidy Good point there could be! I added transient based locking similar to what we do elsewhere in core, what do you think? |
Member
Author
I will work on the core backport once we revert removing the feature. |
westonruter
reviewed
Apr 2, 2026
Eliminate the race condition in concurrent sideloads by no longer writing attachment metadata in the sideload endpoint. Instead, sideload returns lightweight sub-size data (dimensions, filename, filesize) and the client accumulates it. The finalize endpoint receives all sub-size metadata and writes it in a single operation. This removes the transient-based lock (acquire_metadata_lock) which had TOCTOU issues, and aligns more closely with how WordPress core handles sub-size generation (single metadata write after all sizes are created).
The sideload endpoint now returns lightweight sub-size data instead of a full attachment response. Update sideloadToServer to return SubSizeData directly and change sideloadMedia to use an onSuccess callback with SubSizeData instead of onFileChange with Attachment.
Sideload responses now return SubSizeData instead of full attachments. The client accumulates these on the parent queue item via a new AccumulateSubSize reducer action, then passes all collected sub-sizes to the finalize endpoint which writes metadata in a single operation. Remove onChange callbacks from sideload items since they no longer return attachment data for intermediate editor updates.
Update mediaFinalize tests to verify sub_sizes are sent in the request body. Update finalizeItem tests to verify sub-sizes are passed from the queue item. Update sideload tests to use onSuccess callback with SubSizeData instead of onFileChange with Attachment.
Update existing sideload tests to match the new response shape (sub-size data instead of full attachment). Add new tests for the finalize endpoint: - test_finalize_writes_regular_sub_sizes: verifies thumbnail and medium metadata are written correctly - test_finalize_writes_scaled_metadata: verifies scaled image metadata including original_image, dimensions, and file path - test_finalize_writes_original_metadata: verifies original_image is set for rotated image sideloads - test_finalize_with_empty_sub_sizes: verifies the wp_generate_attachment_metadata filter still fires - test_finalize_preserves_image_meta: verifies EXIF data is not overwritten when adding sub-sizes Update the full flow integration test to sideload then finalize, verifying the complete upload pipeline works end-to-end.
Fix equals sign alignment in controller and test file. Fix test_sideload_item to use generate_sub_sizes=false upload so the attachment starts without pre-existing sub-sizes. Fix test_finalize_writes_original_metadata to actually sideload the file before finalizing so it exists on disk.
Use a unique filename (sideload-test.jpg) to avoid collision with canola.jpg that may exist in uploads from other tests in the same CI run.
westonruter
reviewed
Apr 3, 2026
westonruter
reviewed
Apr 3, 2026
westonruter
reviewed
Apr 3, 2026
westonruter
reviewed
Apr 3, 2026
westonruter
reviewed
Apr 3, 2026
Add minimum constraints for width, height, and filesize (must be >= 1) and a pattern constraint for mime_type (must start with 'image/'). Props westonruter.
swissspidy
approved these changes
Apr 16, 2026
Resolve conflicts in packages/upload-media keeping our concurrent sideload uploads approach over trunk's batch-resize refactor. Preserve trunk's 0.29.0 release heading in the CHANGELOG alongside our Unreleased entry, and keep the 0.29.0 package.json version bump.
Member
Author
|
Noting the changes in |
adamsilverstein
added a commit
that referenced
this pull request
Apr 20, 2026
Resolve conflicts in lib/media/class-gutenberg-rest-attachments-controller.php and packages/upload-media/src/store/private-actions.ts by keeping the PR's HEIC-handling approach. Trunk's #75888 sideload refactor introduced an incompatible sub-size accumulation pattern; reconciling with the PR's 'original-heic' branch and scaled-sideload flow is deferred as follow-up.
10 tasks
adamsilverstein
added a commit
that referenced
this pull request
Apr 20, 2026
Adopt the sub_size_data accumulation pattern introduced in #75888 on trunk and fold the HEIC companion handling into it: PHP (class-gutenberg-rest-attachments-controller.php) - finalize route: add sub_sizes schema argument - finalize_item(): apply accumulated sub_sizes to $metadata, including a new 'original-heic' branch that writes to $metadata['original'] - sideload_item(): return a $sub_size_data shape; the 'original-heic' case returns file=wp_basename($path) for finalize to pick up TS (upload-media/src/store/private-actions.ts) - Remove shouldPauseForSideload and resumeItemByPostId; trunk gates concurrency via getActiveUploadCount / maxConcurrentUploads - sideloadItem(): replace onFileChange with onSuccess(subSize) that dispatches AccumulateSubSize against item.parentId - generateThumbnails(): drop onChange callbacks from thumbnail and scaled addSideloadItem dispatches; sideloads no longer return a full attachment - finalizeItem(): pass item.subSizes to mediaFinalize
ciampo
pushed a commit
that referenced
this pull request
Apr 21, 2026
* Remove sideload upload serialization The per-attachment serial constraint on thumbnail sideloads added unnecessary latency during bulk uploads. The existing maxConcurrentUploads limit already bounds total concurrency, and image processing naturally staggers upload start times. * Polish concurrent sideload PR - Add CHANGELOG entry for concurrent sideload behavior change - Remove pauseItem from ActionCreators type (no thunk calls it) - Simplify first concurrent sideload test to use fake parentId like the other tests in the same describe block * Fix race condition in concurrent sideload metadata updates Use MySQL advisory locks (GET_LOCK/RELEASE_LOCK) to serialize the read-modify-write cycle for attachment metadata during concurrent sideloads. Without this, parallel sideloads for the same attachment can overwrite each other's metadata changes. The lock is scoped per-attachment and held only during the metadata update (not during file upload). Object cache is invalidated before reading to ensure fresh data from the database. * Clean up uploaded file on metadata lock timeout Delete the sideloaded file from disk when the advisory lock cannot be acquired, preventing orphaned files that would never be linked to attachment metadata. * Switch from MySQL advisory locks to transient-based locks Replace GET_LOCK/RELEASE_LOCK with WordPress transient locks, matching the pattern used by wp_maybe_generate_attachment_metadata() in core. This aligns with established WordPress conventions and avoids introducing an unprecedented locking mechanism. The transient lock spins with 50ms pauses until available or a 5-second timeout is reached. The transient auto-expires as a safety net against stale locks from crashed processes. * Move metadata saving from sideload to finalize endpoint Eliminate the race condition in concurrent sideloads by no longer writing attachment metadata in the sideload endpoint. Instead, sideload returns lightweight sub-size data (dimensions, filename, filesize) and the client accumulates it. The finalize endpoint receives all sub-size metadata and writes it in a single operation. This removes the transient-based lock (acquire_metadata_lock) which had TOCTOU issues, and aligns more closely with how WordPress core handles sub-size generation (single metadata write after all sizes are created). * Update media-utils sideload to return SubSizeData instead of Attachment The sideload endpoint now returns lightweight sub-size data instead of a full attachment response. Update sideloadToServer to return SubSizeData directly and change sideloadMedia to use an onSuccess callback with SubSizeData instead of onFileChange with Attachment. * Accumulate sub-size data client-side and pass to finalize Sideload responses now return SubSizeData instead of full attachments. The client accumulates these on the parent queue item via a new AccumulateSubSize reducer action, then passes all collected sub-sizes to the finalize endpoint which writes metadata in a single operation. Remove onChange callbacks from sideload items since they no longer return attachment data for intermediate editor updates. * Update tests for sub-size data accumulation in finalize Update mediaFinalize tests to verify sub_sizes are sent in the request body. Update finalizeItem tests to verify sub-sizes are passed from the queue item. Update sideload tests to use onSuccess callback with SubSizeData instead of onFileChange with Attachment. * Update PHP tests for finalize metadata writing Update existing sideload tests to match the new response shape (sub-size data instead of full attachment). Add new tests for the finalize endpoint: - test_finalize_writes_regular_sub_sizes: verifies thumbnail and medium metadata are written correctly - test_finalize_writes_scaled_metadata: verifies scaled image metadata including original_image, dimensions, and file path - test_finalize_writes_original_metadata: verifies original_image is set for rotated image sideloads - test_finalize_with_empty_sub_sizes: verifies the wp_generate_attachment_metadata filter still fires - test_finalize_preserves_image_meta: verifies EXIF data is not overwritten when adding sub-sizes Update the full flow integration test to sideload then finalize, verifying the complete upload pipeline works end-to-end. * Fix PHPCS alignment warnings and PHP test failures Fix equals sign alignment in controller and test file. Fix test_sideload_item to use generate_sub_sizes=false upload so the attachment starts without pre-existing sub-sizes. Fix test_finalize_writes_original_metadata to actually sideload the file before finalizing so it exists on disk. * Fix test_sideload_item filename collision in CI Use a unique filename (sideload-test.jpg) to avoid collision with canola.jpg that may exist in uploads from other tests in the same CI run. * Add schema validation for finalize endpoint sub-size fields Add minimum constraints for width, height, and filesize (must be >= 1) and a pattern constraint for mime_type (must start with 'image/'). Props westonruter.
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
3 participants
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.
Summary
Why
For a single image with 5 missing thumbnail sizes + a scaled version, the old behavior created 6 sequential HTTP uploads per attachment. During bulk uploads (e.g., Gallery block), this compounds significantly and adds latency without meaningfully reducing race condition risk.
Test plan
Verifying concurrency in the Network panel
Open DevTools → Network tab, then enable throttling (e.g. 3g or 4g) so uploads are slow enough to observe overlap.
Before this PR: Upload a large image (one that generates several thumbnail sizes). In the Network panel you will see the sideload requests arrive one after another — each
/wp/v2/media/<id>PUT request starts only after the previous one finishes. The waterfall looks like a staircase.After this PR: The same upload produces multiple sideload PUT requests that start at roughly the same time and run in parallel. In the waterfall view the bars overlap horizontally. With the default
maxConcurrentUploads: 5you should see up to 5 requests in flight simultaneously instead of 1.To count in-flight requests at a glance: filter the Network panel to
XHR/Fetch, look for/wp/v2/media/PUT requests, and observe how many show a "pending" state at the same time.Closes #75257