Defer add_to_outbox to async processing during post save #2761
Conversation
This improves publishing performance by scheduling the add_to_outbox call via WP Cron instead of running it synchronously during post save. This prevents the Save button from being blocked by potentially slow operations like fetching remote objects for reply posts. - Rename schedule_post_activity to triage for clarity - Add async handler via activitypub_add_to_outbox action - Falls back to synchronous execution when scheduling fails (tests/cron disabled) - Apply same pattern to attachment transitions Fixes #2567.
There was a problem hiding this comment.
Pull request overview
This PR improves publishing performance by deferring add_to_outbox() processing to asynchronous execution via WP Cron instead of running synchronously during post save operations. This prevents the Save button from being blocked by slow operations such as fetching remote objects for reply posts.
Changes:
- Renamed
schedule_post_activitytotriagefor better clarity about its role in determining activity types - Added
add_to_outboxas an async handler method triggered by theactivitypub_add_to_outboxaction - Applied the same async pattern to attachment status transitions
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
includes/scheduler/class-post.php |
Implements async outbox processing with WP Cron scheduling and synchronous fallback for both posts and attachments |
tests/phpunit/tests/includes/scheduler/class-test-post.php |
Updates test method names and hook references to match renamed triage method |
tests/phpunit/tests/includes/class-test-shortcodes.php |
Updates hook reference to use renamed triage method |
tests/phpunit/tests/includes/class-test-scheduler.php |
Updates hook references to use renamed triage method |
tests/phpunit/tests/includes/class-test-migration.php |
Updates hook reference to use renamed triage method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This ensures all tests that create posts get synchronous outbox execution while not affecting other scheduler tests that need real scheduling.
|
Looks good. As far as I remember #1269 was not solved completely for some Event-Plugins, thats why I kept this workaround, so now I could remove it :) |
- Re-validate post eligibility in async handler (post could be disabled between scheduling and execution) - Add WP_Post instanceof check in attachment handler before accessing properties - Rename test method for consistency with other test names
|
@Menrath that is even better news! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( is_post_disabled( $post ) ) { | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
There's a potential race condition where a post or attachment could change state between when the event is scheduled and when it executes. While the async handler checks if the post exists and validates it's not disabled, it doesn't verify that the activity type is still appropriate for the current post state. For example, if a post is scheduled for a Create activity but is then trashed before the async handler runs, the handler would still execute a Create activity for a trashed post (since is_post_disabled doesn't check for trash status). Consider adding validation to ensure the post is still in a publishable state, or re-determining the appropriate activity type based on the current post state.
| // Ensure the Activity type is still appropriate for the current post state. | |
| $current_type = get_wp_object_state( $post ); | |
| // If we cannot determine a current type, do not emit an Activity. | |
| if ( ! $current_type ) { | |
| return; | |
| } | |
| // If the scheduled type no longer matches the current state, prefer the current state. | |
| if ( $current_type !== $type ) { | |
| if ( \in_array( $current_type, array( 'Create', 'Update', 'Delete' ), true ) ) { | |
| $type = $current_type; | |
| } else { | |
| // Unknown or unsupported type for the current state; abort. | |
| return; | |
| } | |
| } |
jeherve
left a comment
jeherve
left a comment
There was a problem hiding this comment.
While this works well, I do worry about the potential impact it may have on sites with a problematic WordPress' cron implementation ; WordPress' cron can be fragile and the performance gains of this PR may not outweigh the potential cron failures that would cause federation to be delayed or to fail.
I don't have a lot of context around #2567. Is this a performance issue that's been reported by some users already, something that's problematic already, or is this something you'd like to address before it becomes a problem?
All that said, I don't want to block this PR, we can certainly give it a try, and see if we see more reported federation failures in the future.
Oh yes, missed to add the wordpress.org discussion. Will add it asap. |
|
@jeherve we can't get rid of CRONs, because (as you already referenced in your comment), the preparing task already slows down publishing. If we think about a follower count above 10, we can not federate in a synchronous way without slowing down the process by minutes or even run in a timeout. I am happy to revise the whole process, to check if we can improve it, but from my perspective, there is no way to have the plugin run without CRONs. |
In this case, let's go with this approach for now, see how it works for everyone in the wild! |
Fixes #2567
Proposed changes:
add_to_outbox()to async processing via WP Cron instead of running synchronously during post saveschedule_post_activitytotriagefor clarityadd_to_outboxmethod as async handler viaactivitypub_add_to_outboxactionOther information:
Testing instructions:
activitypub_add_to_outboxaction was scheduledChangelog entry
Changelog Entry Details
Significance
Type
Message
Defer outbox processing to async execution to improve publishing performance.