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::NotifyAboutPushService to handle notification logic
  • Created MergeRequests::Refresh::NotifyAboutPushWorker to execute the service asynchronously
  • Added NotificationService#push_to_merge_request_with_data method to accept pre-computed commit data, avoiding large Sidekiq payloads
  • Updated MergeRequests::RefreshService to:
    • Skip synchronous notify_about_push when the feature flag is enabled
    • Enqueue the new worker in execute_async_workers when the feature flag is enabled
  • Controlled by the split_refresh_worker_notify_about_push feature 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

  1. Enable the feature flag in a Rails console:

    Feature.enable(:split_refresh_worker_notify_about_push)

  2. Push commits to a branch with an open merge request:

    git checkout -b test-notify-push echo "test" > test.txt git add test.txt git commit -m "Test commit" git push origin test-notify-push

  3. Create a merge request for the branch if one doesn't exist

  4. Push additional commits to the same branch:

    echo "test2" >> test.txt git add test.txt git commit -m "Another test commit" git push origin test-notify-push

  5. 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::NotifyAboutPushWorker job appears in Sidekiq
  6. 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)

Edited by Marc Shaw

Merge request reports

Loading