Split notify_about_push into separate worker
What does this MR do and why?
This MR extracts the notify_about_push functionality from MergeRequests::RefreshService into a dedicated worker and service, following the established pattern used by PipelineWorker and WebHooksWorker.
By executing notification logic asynchronously, this change improves the performance of the refresh service by offloading notification work to background jobs, reducing the time spent in the synchronous push processing path.
Changes
- Created
MergeRequests::Refresh::NotifyAboutPushServiceto handle notification logic - Created
MergeRequests::Refresh::NotifyAboutPushWorkerto execute the service asynchronously - Added
NotificationService#push_to_merge_request_with_datamethod to accept pre-computed commit data, avoiding large Sidekiq payloads - Updated
MergeRequests::RefreshServiceto:- Skip synchronous
notify_about_pushwhen the feature flag is enabled - Enqueue the new worker in
execute_async_workerswhen the feature flag is enabled
- Skip synchronous
- Controlled by the
split_refresh_worker_notify_about_pushfeature flag (beta, disabled by default) - Added comprehensive test coverage for both the worker and service
- Fixed invalid commit short_id test data in
notification_service_spec.rb(changed non-hexadecimal values like'old111'to valid hex'01d111')
Implementation Details
The implementation follows the same pattern as existing async workers:
RefreshService captures the MR's commit state at push time and passes it to the worker in execute_async_workers to avoid race conditions where subsequent pushes change the MR's commit SHAs.
NotifyAboutPushWorker receives commit SHAs and the MR's captured commit state, loads commit objects from the repository, and delegates to the service.
NotifyAboutPushService accepts commits and the captured MR commit state, then handles the notification logic:
- Partitions commits into new and existing based on the captured commit SHAs (not the current MR state)
- Creates system notes via
SystemNoteService.add_commits - Sends push notifications via
notification_service.push_to_merge_request
NotificationService#push_to_merge_request_with_data is a new method that accepts pre-computed commit data (arrays of hashes with short_id and title) instead of commit objects. This allows the worker to:
- Compute commit data before enqueueing
- Pass only the minimal required data to Sidekiq
- Avoid serializing large commit objects
The feature flag controls whether to execute synchronously (disabled, old behavior) or asynchronously (enabled, new behavior).
Race Condition Handling
To prevent race conditions where the MR's commit SHAs change between when the worker is queued and when it's processed, the implementation captures the MR's commit state at push time. This captured state is passed to the worker and used to partition commits, ensuring consistent behavior regardless of subsequent pushes.
How to test
-
Enable the feature flag in a Rails console:
Feature.enable(:split_refresh_worker_notify_about_push) -
Push commits to a branch with an open merge request:
git checkout -b test-notify-pushecho "test" > test.txtgit add test.txtgit commit -m "Test commit"git push origin test-notify-push -
Create a merge request for the branch if one doesn't exist
-
Push additional commits to the same branch:
echo "test2" >> test.txtgit add test.txtgit commit -m "Another test commit"git push origin test-notify-push -
Verify that:
- System notes are created on the merge request showing the new commits
- Push notifications are sent to merge request participants
- The
MergeRequests::Refresh::NotifyAboutPushWorkerjob appears in Sidekiq
-
Disable the feature flag and repeat steps 4-5 to verify the old synchronous behavior still works:
Feature.disable(:split_refresh_worker_notify_about_push)
Related to #576798 (closed)