Skip to content

Fix bug tracking snippet shard name

Francisco Javier López (ex-Gitlab) requested to merge fj-212399-fix-bug-tracking-snippet-shard-name into master

What does this MR do?

The bug happens when there are more than one shard configured (like in staging and production), involving the following methods:

  def repository_storage
    snippet_repository&.shard_name || self.class.pick_repository_storage
  end

  def create_repository
    return if repository_exists? && snippet_repository

    repository.create_if_not_exists
    track_snippet_repository
  end

  def track_snippet_repository
    repository = snippet_repository || build_snippet_repository
    repository.update!(shard_name: repository_storage, disk_path: disk_path)
  end

Let's take a look at what happens when we create a repository:

  • Method create_repository is called
  • We initialize the repository object
    • The shard used will be picked from the available set of shards because snippet_repository does not exist yet. Let's call it foo_shard
  • Creates the repository in the shard
    • The Gitaly client uses repository.storage as the shard to use (foo_shard)
  • Method track_snippet_repository is called to create the snippet_repository object
    • We initialize the snippet_repository object
    • We create the object in the database using as shard_name the value returned by repository_storage
      • Since snippet_repository hasn't been created yet, we pick a random shard from the set. And here is where we can find the bug, since the shard returned might not be the same as the one used to initialize the repository (let's call it bar_shard)
  • At this point, the repository has been created in foo_shard but we're storing that it was created in bar_shard.
  • Some Gitaly operation performed on the repo will use the repository.storage, so no error will be raised here.
  • Nevertheless, when we perform any operation that needs to call the internal API we call the method gitaly_payload 
  def gitaly_payload(action)
    return unless %w[git-receive-pack git-upload-pack git-upload-archive].include?(action)

    {
      repository: repository.gitaly_repository,
      address: Gitlab::GitalyClient.address(container.repository_storage),
      token: Gitlab::GitalyClient.token(container.repository_storage),
      features: Feature::Gitaly.server_feature_flags
    }
  end
  • In address we store the value of container.repository_storage and, because the snippet_repository object exists, we return the shard stored on the record, which is bar_shard.
  • Basically, any further operation will use bar_shard as the shard to look for the repository but, however, the repo was stored in foo_bar
  • This ends up with an error from Gitaly saying that the dir is not a git project

Closes #212399 (closed)

Does this MR meet the acceptance criteria?

Conformity

Edited by 🤖 GitLab Bot 🤖

Merge request reports