Skip EventWorker enqueue in the Geo Log Cursor when sync_object_storage is disabled on the current node
What does this MR do and why?
The Geo LogCursor enqueues every blob event even when the target secondary has sync_object_storage disabled and the worker will just discard the job. On secondaries with a lot of blob traffic this wastes ~99% of LogCursor throughput and can grow the event backlog to millions of events over days.
This MR adds a skip_enqueue? check to Gitlab::Geo::LogCursor::Events::Event#process, gated behind the geo_log_cursor_skip_object_storage_events feature flag to decide — before paying Sidekiq middleware cost — whether a given event's file is already reachable externally and can be skipped. It also closes a related crash risk: resolving an unrecognized replicable_name now safely falls back to skipping the event instead of letting NotImplementedError (a ScriptError, not StandardError) propagate uncaught and crash the Geo Log Cursor daemon.
Database concerns
This fix trades the avoided middleware cost for one new indexed DB query per eligible event: object_storage_scope(node).id_in(model_record_id).exists?. That read is a single indexed id_in (primary-key) lookup layered on top of object_storage_scope, so it's an O(1) point query, not a scan, and it's only reached after the cheap FF/event-type/node checks have already filtered out most events. Running the indexed query for a subset of the enqueued events seems like a good tradeoff against the IO cost of enqueueing >100 jobs per second.
Query plan
The query mentioned above creates the following plan, taken with the Upload blob model class:
SQL
SELECT 1 AS one FROM "uploads" WHERE "uploads"."id" = 1197913122 LIMIT 1Plan: https://postgres.ai/console/gitlab/gitlab-production-main/sessions/53812/commands/156133
The query takes ~13ms to resolve.
Follow-up
That per-event lookup measured at ~13-15ms risks becoming a bottleneck for LogCursor's single-threaded processing loop at >100 events/second. Investigation showed the cost is almost entirely Postgres planning time: p_ci_job_artifacts (partitioned by partition_id) and uploads (partitioned by model_type) can't be pruned by a bare id lookup, so the planner builds an Append across every partition before even running the (trivially fast) per-partition scan.
This issue will be addressed in a follow-up MR by modifying object_storage_scope to include the partition keys in the scope too.
References
Fixes Geo: Logcursor enqueues blob events through ful... (#593115)
How to set up and validate locally
| Scenario | Expected | Observed |
|---|---|---|
| Remote-stored blob | skip enqueue | "Skipped enqueue (not in storage scope for current secondary)", geo queue stayed at 0 |
| Local-stored blob | enqueue normally | No skip log; Geo::BlobDownloadService ran, registry state updated |
| Unrecognised replicable name | skip, no crash | One ERROR log + one skip log for the event; geo-cursor stayed on the same pid (2410) the entire test, uptime climbing continuously = no crash |
Setup
- Ensure you've got a GDK with a primary and secondary Geo site.
- Checkout this branch on both secondary and primary sites.
- On the primary, enable the flag and disable storage sync:
Feature.enable(:geo_log_cursor_skip_object_storage_events)
GeoNode.secondary_nodes.first.update!(sync_object_storage: false)- On the secondary, start (a fresh process avoids the known
current_nodememoization staleness tracked in #595353 ) the daemon and Sidekiq:
gdk start geo-cursor
gdk start rails-background-jobs- Note the current line count of the secondary's log/geo.log before each scenario, so you only inspect newly-appended lines:
wc -l < log/geo.log.
Scenario A — remote-stored blob should be skipped
On the primary, run:
pf = Packages::PackageFile.where(file_store: ObjectStorage::Store::LOCAL).first
pf.update_column(:file_store, ObjectStorage::Store::REMOTE)
replicator = Geo::PackageFileReplicator.new(model_record: pf)
replicator.publish(:created, **replicator.created_params)Expect on the secondary: a "Skipped enqueue (not in storage scope for current secondary)" line in geo.log for that event, and Sidekiq::Queue.new("geo").size unchanged.
Scenario B — locally-stored blob should still enqueue
On the primary, using a different local package file:
pf2 = Packages::PackageFile.where(file_store: ObjectStorage::Store::LOCAL).where.not(id: pf.id).first
replicator2 = Geo::PackageFileReplicator.new(model_record: pf2)
replicator2.publish(:created, **replicator2.created_params)Expect on the secondary: no skip line for this event; evidence the worker actually ran (e.g. a Geo::BlobDownloadService log entry, registry state change).
Scenario C — unrecognized replicable_name should skip, not crash the daemon
On the primary:
event = Geo::Event.create!(replicable_name: "totally_bogus_type", event_name: "created",
payload: { "model_record_id" => 1, "correlation_id" => SecureRandom.uuid })
Geo::EventLog.create!(geo_event: event)Expect on the secondary: one ERROR line ("Cannot find a Geo::Replicator for totally_bogus_type"), one skip line for that event, no new Sidekiq job, and gdk status still showing geo-cursor up on the same pid as before (confirms no crash).
Cleanup
On the primary:
GeoNode.secondary_nodes.first.update!(sync_object_storage: true) # or whatever the original value was
Feature.remove(:geo_log_cursor_skip_object_storage_events)
pf.update_column(:file_store, ObjectStorage::Store::LOCAL)
Geo::Event.find_by(replicable_name: "totally_bogus_type")&.destroy!MR acceptance checklist
Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.