Skip to content

fix: bypass blob get notifications to avoid filesystem link check in migration mode

João Pereira requested to merge 437-bypass-on-mirroring-disable into master

Related to #437.

According to the plan (link), we'll need to disable filesystem metadata mirroring before starting phase 2 of the registry migration on .com.

However, doing this causes the problem reported in #437. The proper fix is not trivial or low risk, which is why this MR proposes temporary mitigation to improve the situation. We could as well leave it, but by doing so we would:

  1. Be making a stat request against the storage backend for every blob HEAD/GET request when we know it's going to fail (because there are no blob links there when metadata mirroring is disabled, they are only on the DB);
  2. Have an error log entry for each blob request, which is noisy.

The shortest mitigation for the two problems listed above is to bypass the notification listener actions when in migration mode with mirroring disabled.

This comes with a side effect though: there will be no async notifications emission for blob HEAD/GET requests. However, this is already true if mirroring is disabled, as described in #437. Also, we don't consume any pull notifications for GitLab.com, we're only consuming push and delete notifications (for metrics purposes). Notifications are not enabled for self-managed instances at all either, and even if they were, the bypass, as implemented in this MR, only comes into effect when in migration mode and mirroring disabled, none of which is available for self-managed yet (they will come later, after the .com migration).

Considering the above, this imperfect (but quick and low risk) fix has no impact on users and will allow us to spare a stat request (which is one of the slowest) per HEAD/GET request during the migration phase 2.

Followup issue to ensure that we implement a better solution (with no compromises) later on: #622 (closed)

Edited by João Pereira

Merge request reports