Geo: Handle toggling between legacy and v2 of project repo replication

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

Overview

Once #546175 is complete, ProjectRepository becomes the replicable model under V2. However, Project records still fire Geo events via geo_handle_after_update / geo_handle_after_create / geo_handle_after_destroy, because Project still includes Geo::ReplicableModel and Geo::VerifiableModel. Under V2, these events are redundant and wasteful: the ProjectRepositoryReplicator#should_publish_replication_event? guard already drops them, but the calls still happen on every repository write.

This issue ensures that, when geo_project_repository_replication_v2 is enabled, Geo events no longer fire for Project records, and that toggling the FF in either direction is seamless — with no data loss and without requiring re-replication of already-synced repositories.


Context: how event hooks work today

The consolidation work from #437929 is largely complete. EE::Repository#log_geo_updated_event is now the single source of truth for geo_handle_after_update calls:

# ee/app/models/ee/repository.rb
def log_geo_updated_event
  return unless ::Gitlab::Geo.primary?

  case container
  when Project, DesignManagement::Repository
    container.geo_handle_after_update
  when ProjectWiki
    project.wiki_repository&.geo_handle_after_update
  # ...
  end
end

geo_handle_after_update delegates to ProjectRepositoryReplicator#geo_handle_after_update, which calls publish(:updated, ...). The replicator already has a guard:

# ee/app/replicators/geo/project_repository_replicator.rb
def should_publish_replication_event?
  return false unless super
  return true if model_record.is_a?(::Project)  # always fires under V1

  self.class.geo_project_repository_replication_v2_enabled? && model_record.is_a?(::ProjectRepository)
end

So under V2, Project-based events are published but then silently dropped. The goal of this issue is to stop them from being published at all.


What needs to be done

1. Gate Project event publishing under V2

In should_publish_replication_event?, flip the logic so that Project-based events are only published when V2 is disabled (i.e. V1):

def should_publish_replication_event?
  return false unless super

  if model_record.is_a?(::Project)
    return !self.class.geo_project_repository_replication_v2_enabled?
  end

  self.class.geo_project_repository_replication_v2_enabled? && model_record.is_a?(::ProjectRepository)
end

2. Suppress Project create/destroy hooks under V2

geo_handle_after_create and geo_handle_after_destroy are triggered by after_create_commit / after_destroy callbacks in Geo::ReplicableModel (which Project includes). Override these in the replicator to no-op when V2 is enabled and model_record is a Project:

File: ee/app/replicators/geo/project_repository_replicator.rb (preferred) or ee/app/models/ee/project.rb

def geo_handle_after_create
  return if model_record.is_a?(::Project) &&
    self.class.geo_project_repository_replication_v2_enabled?

  super
end

def geo_handle_after_destroy
  return if model_record.is_a?(::Project) &&
    self.class.geo_project_repository_replication_v2_enabled?

  super
end

Note: Change 3 below handles the ProjectRepository destroy case separately and explicitly; these overrides only suppress the Project path.

3. Ensure ProjectRepository events fire correctly on create/destroy

Under V2, ProjectRepository records are the source of truth. Their after_create_commit and after_destroy callbacks (from Geo::ReplicableModel, included via EE::ProjectRepository) must fire correctly. Verify that:

  • Creating a ProjectRepository record publishes a created event.
  • Destroying a ProjectRepository record publishes a deleted event.
  • These are gated by should_publish_replication_event? returning true only under V2.

When a project is destroyed, ProjectRepository is cleaned up asynchronously via the loose FK mechanism (SQL DELETE, bypasses ActiveRecord callbacks), so ProjectRepository#after_destroy never fires.

To fix this, we capture a reference to project_repository before destruction and explicitly call geo_handle_after_destroy on it once the project is confirmed destroyed, mirroring the same pattern used for stale_secrets_manager. This ensures the Geo delete event is published with the correct model_record_id, allowing secondaries to clean up the registry record correctly.

In ee/app/services/ee/projects/destroy_service.rb:

  def execute                                                                                                                                                                   
    stale_secrets_manager = project.secrets_manager                                                                                                                             
    stale_project_repository = project.project_repository                                                                                                                       
    super.tap do                                                                                                                                                                
      if project&.destroyed?
        mirror_cleanup(project)                                                                                                                                                 
        deprovision_secrets_manager(stale_secrets_manager)
        stale_project_repository&.geo_handle_after_destroy <= ensures 
      end                                                                                                                                                                       
    end
  end

geo_handle_after_destroy on a ProjectRepository calls the replicator with model_record = project_repository, so the event is published with project_repository.id. This is the correct ID for the secondary to look up and clean up the ProjectRepositoryRegistry record.

This is only needed when V2 is ON (Change 2 suppresses the Project path). The geo_handle_after_destroy call on ProjectRepository is already gated by should_publish_replication_event?, which requires V2 to be enabled for ProjectRepository events.

4. ProjectRepositoryReplicator#model_record override (FF race condition)

If the FF state changes between event publish and consume (secondary can lag ~1 minute), two failure modes exist:

  • ActiveRecord::RecordNotFoundmodel.find(id) fails because the ID doesn't exist in the current model's table
  • Silent wrong record — the ID coincidentally exists in the current model's table but belongs to a different entity (Project IDs and ProjectRepository IDs are independent sequences and can collide)

Catching RecordNotFound alone is insufficient. We need to know what model class the ID was generated from.

The following steps are a proposition from Claude:

Step 1: Store the model class in the event payload by overriding event_params:

def event_params
  super.merge("model_class" => model_record.class.name)
end

Step 2: Override consume to capture the stored model class hint before model_record is lazily loaded:

def consume(event_name, **event_data)
  @stored_model_class = event_data["model_class"]
  super
end

Step 3: Override model_record to detect both failure modes and translate:

def model_record
  record = super

  # If we know what class was stored, check for a type mismatch (wrong record found)
  return record if @stored_model_class.nil? || record.class.name == @stored_model_class

  translate_model_record(record.id)
rescue ActiveRecord::RecordNotFound
  # ID doesn't exist in the current model's table — also translate
  # Might only be worth translating when we're dealing with a Project or ProjectRepo
  translate_model_record(model_record_id)
end

def translate_model_record(id)
  if self.class.geo_project_repository_replication_v2_enabled?
    # V2 ON but received a Project ID — navigate Project → ProjectRepository
    ::Project.find_by(id: id)&.project_repository
  else
    # V2 OFF but received a ProjectRepository ID — navigate ProjectRepository → Project
    ::ProjectRepository.find_by(id: id)&.project
  end
end

For events predating this change (no model_class in payload), @stored_model_class is nil and only the RecordNotFound path applies — imperfect but no worse than before.

Updating @model_record_id ensures subsequent registry lookups (via registry_class.for_model_record_id(model_record_id)) also use the translated ID.

5. ProjectRepositoryRegistry.for_model_record_id safety net

Defense-in-depth for events already in the Sidekiq queue when fixes are deployed (published with a Project ID, consumed after V2 is ON). project_id is always populated and unique in the registry, making it a reliable fallback.

File: ee/app/models/geo/project_repository_registry.rb

def self.for_model_record_id(id)
  return find_or_initialize_by(project_id: id) unless
    ProjectRepositoryReplicator.geo_project_repository_replication_v2_enabled?

  find_by(project_repository_id: id) || find_or_initialize_by(project_id: id)
end

6. Verify state records still propagate under both versions**

Project still includes Geo::VerifiableModel, so project_states records are written on save regardless of the FF. This must remain true under V2 for backward compatibility (dual-write is handled in #591309). Confirm that suppressing replication events does not accidentally suppress state record writes.


Implementation plan

Phase 1: Core event suppression

  1. Update ProjectRepositoryReplicator#should_publish_replication_event? (Change 1)

    • Modify the method to suppress Project-based events when V2 is enabled
    • Project events should only publish when V2 is disabled (V1 mode)
    • Add unit tests asserting no events are published for Project records when V2 is enabled
  2. Override geo_handle_after_create and geo_handle_after_destroy (Change 2)

    • Add overrides in ProjectRepositoryReplicator to no-op for Project records when V2 is enabled
    • Add unit tests for both methods under V1 and V2 scenarios

Phase 2: ProjectRepository lifecycle events

  1. Handle ProjectRepository destroy via Projects::DestroyService (Change 3)

    • Modify ee/app/services/ee/projects/destroy_service.rb to capture project_repository before destruction
    • Explicitly call geo_handle_after_destroy on the stale ProjectRepository after project destruction
    • Add integration tests verifying ProjectRepository delete events are published with correct IDs
  2. Verify ProjectRepository create events (Change 3)

    • Add/update specs confirming created events fire for ProjectRepository records under V2
    • Ensure events are correctly gated by should_publish_replication_event?

Phase 3: FF toggle race condition handling

  1. Store model class in event payload (Change 4, Step 1)

    • Override event_params in ProjectRepositoryReplicator to include model_class
    • Add specs verifying the payload includes the correct model class
  2. Capture stored model class on consume (Change 4, Step 2)

    • Override consume method to extract @stored_model_class from event data
    • Add specs for the consume override
  3. Implement model_record translation logic (Change 4, Step 3)

    • Override model_record to detect type mismatches and RecordNotFound errors
    • Implement translate_model_record helper to navigate between Project and ProjectRepository
    • Add comprehensive specs for:
      • V2 ON receiving a Project ID (translate to ProjectRepository)
      • V2 OFF receiving a ProjectRepository ID (translate to Project)
      • Legacy events without model_class in payload

Phase 4: Registry safety net

  1. Update ProjectRepositoryRegistry.for_model_record_id (Change 5)
    • Add fallback logic to find by project_id when project_repository_id lookup fails under V2
    • Add specs for the fallback behavior with both V1 and V2 scenarios

Phase 5: Verification and testing

  1. Verify project_states writes are unaffected (Change 6)

    • Add/update specs confirming state records are written regardless of FF state
    • Ensure event suppression does not impact Geo::VerifiableModel behavior
  2. Add FF toggle integration tests

    • Test V1 → V2 toggle: no duplicate events, existing synced repos remain synced
    • Test V2 → V1 toggle: no missing events, graceful handling of in-flight events
    • Test events published under one FF state and consumed under another

Follow-up

⚠️ When the FF is removed and V2 becomes the only version, the Project-based event paths, the project_states table, and the associated DB triggers should be cleaned up. A separate issue should be created for this at that point.

Edited by 🤖 GitLab Bot 🤖