Skip to content

Fix RepositoryUpdateRemoteMirrorWorker retry scheduled_time arg

Kyle Russell requested to merge bkylerussell/gitlab:retry into master

What does this MR do and why?

Fix RepositoryUpdateRemoteMirrorWorker retry scheduled_time arg

If the first sync attempt (tries == 0) fails very quickly and we immediately schedule a retry (tries == 1), arg scheduled_time was not adjusted for the mirror's backoff_delay. When sidekiq serializes the original Time.current value to JSON, some precision is lost. If the new scheduled_time is close enough to the previous attempt (last_updated_started_at), then the remote_mirror.updated_since? check thinks we've already performed the update and never schedules any successive retries (tries > 1).

"time":"2024-03-05T19:06:18.374Z","timestamp":"2024-03-05 19:06:18 UTC","last_update_started_at":"2024-03-05T18:41:53.049Z","updated_since":false
"time":"2024-03-05T19:11:21.366Z","timestamp":"2024-03-05 19:06:18 UTC","last_update_started_at":"2024-03-05T19:06:18.646Z","updated_since":true

When we don't exceed MAX_TRIES, the mirror sync may never be marked as failed which breaks the failure notification and :remote_mirrors_failed metric. (RemoteMirror.send_failure_notifications() is never called.)

Sidekiq warns about passing Time values as arguments, but we can preserve the updated_since? behavior by adjusting the scheduled_time for each successive retry (as long as backoff_delay has a resolution greater than 1.second, which is the most precision we get from sidekiq).

Don't pass symbols, named parameters, keyword arguments or complex Ruby objects (like Date or Time!)

(https://github.com/sidekiq/sidekiq/wiki/Best-Practices)

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

Merge request reports