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
endgeo_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)
endSo 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)
end2. 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
endNote: 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
ProjectRepositoryrecord publishes acreatedevent. - Destroying a
ProjectRepositoryrecord publishes adeletedevent. - These are gated by
should_publish_replication_event?returningtrueonly 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
endgeo_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::RecordNotFound—model.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)
endStep 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
endStep 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
endFor 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)
end6. 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
-
Update
ProjectRepositoryReplicator#should_publish_replication_event?(Change 1)- Modify the method to suppress
Project-based events when V2 is enabled Projectevents should only publish when V2 is disabled (V1 mode)- Add unit tests asserting no events are published for
Projectrecords when V2 is enabled
- Modify the method to suppress
-
Override
geo_handle_after_createandgeo_handle_after_destroy(Change 2)- Add overrides in
ProjectRepositoryReplicatorto no-op forProjectrecords when V2 is enabled - Add unit tests for both methods under V1 and V2 scenarios
- Add overrides in
Phase 2: ProjectRepository lifecycle events
-
Handle
ProjectRepositorydestroy viaProjects::DestroyService(Change 3)- Modify
ee/app/services/ee/projects/destroy_service.rbto captureproject_repositorybefore destruction - Explicitly call
geo_handle_after_destroyon the staleProjectRepositoryafter project destruction - Add integration tests verifying
ProjectRepositorydelete events are published with correct IDs
- Modify
-
Verify
ProjectRepositorycreate events (Change 3)- Add/update specs confirming
createdevents fire forProjectRepositoryrecords under V2 - Ensure events are correctly gated by
should_publish_replication_event?
- Add/update specs confirming
Phase 3: FF toggle race condition handling
-
Store model class in event payload (Change 4, Step 1)
- Override
event_paramsinProjectRepositoryReplicatorto includemodel_class - Add specs verifying the payload includes the correct model class
- Override
-
Capture stored model class on consume (Change 4, Step 2)
- Override
consumemethod to extract@stored_model_classfrom event data - Add specs for the consume override
- Override
-
Implement
model_recordtranslation logic (Change 4, Step 3)- Override
model_recordto detect type mismatches andRecordNotFounderrors - Implement
translate_model_recordhelper to navigate betweenProjectandProjectRepository - 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_classin payload
- Override
Phase 4: Registry safety net
- Update
ProjectRepositoryRegistry.for_model_record_id(Change 5)- Add fallback logic to find by
project_idwhenproject_repository_idlookup fails under V2 - Add specs for the fallback behavior with both V1 and V2 scenarios
- Add fallback logic to find by
Phase 5: Verification and testing
-
Verify
project_stateswrites are unaffected (Change 6)- Add/update specs confirming state records are written regardless of FF state
- Ensure event suppression does not impact
Geo::VerifiableModelbehavior
-
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
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.