Stop event purges from moving the stale scan cursor

Gregory Havengarequested to merge
ghavenga/633838-purge-cursor into master

What does this MR do and why?

This MR stops event purges from writing the stale purge cursor, and resets the cursor when a stale run drains.

Security::PurgeScansService stores a keyset cursor in Redis under LAST_PURGED_SCAN_TUPLE. The stale purge cron (Security::Scans::PurgeWorker) uses this cursor to continue from the last scan that it purged.

Two paths use the service:

  1. purge_stale_records purges stale scans in created_at, id order. This is the cron path.
  2. purge_by_build_ids purges the scans of specific builds. This is the event path.

Before this change, both paths wrote the same cursor. The scans of an event purge are not in order with the stale set. When an event purge ran, it moved the cursor to one of its own scans, which is usually newer than the old stale scans. The next cron run then started after that scan. When the run found no more stale scans after the cursor, it reported :drained. The old stale scans before the cursor stayed in the table and the cron did not purge them.

This MR makes these changes:

  1. execute and initialize get a persist_cursor: keyword. The default is false.
  2. Only purge_stale_records sets persist_cursor: true. purge_by_build_ids does not write the cursor.
  3. When a stale run ends with :drained, the service resets the cursor to an empty value. The next cron run starts again from the oldest stale scan.
  4. When a stale run ends with :work_remaining or :halted, the service keeps the cursor. The next run continues from it.

After the deploy, a manual Redis change is not necessary. The current cursor is probably past many stale scans. The next cron runs continue from it until they get to the end of the stale set. That run ends with :drained and resets the cursor. The next cron tick (every 3 hours) then starts again from the oldest stale scan.

On an instance with a backlog of skipped scans, that tick starts a catch-up chain. Each run still stops at MAX_RUNTIME or MAX_STALE_SCANS_SIZE, and the worker re-enqueues itself until the backlog is gone. To slow the catch-up when the database is under pressure, enable the ops flag security_scans_purge_db_health_check. It is off by default. To start the catch-up sooner, run Security::PurgeScansService.redis_cursor.commit({}) in a Rails console.

The reset costs little. The stale query uses the partial index index_security_scans_for_non_purged_records on (created_at, id) WHERE status <> 6. A purged scan leaves that index by the next vacuum of security_scans. Until then, the first run after a reset passes over the index entries of scans purged since the last vacuum. This happens at most once for each drained run.

This is a follow-up to https://gitlab.com/gitlab-com/gl-infra/production/-/issues/23192.

References

Screenshots or screen recordings

Not applicable. This change has no UI.

How to set up and validate locally

  1. Run the specs:

    bundle exec rspec \
  ee/spec/services/security/purge_scans_service_spec.rb \
  ee/spec/workers/security/scans/purge_worker_spec.rb

  2. Optional. In a Rails console, set a cursor, then purge the scans of one build:

    cursor = Security::PurgeScansService.redis_cursor
cursor.commit(created_at: "'2020-01-01 00:00:00'", id: 1)
Security::PurgeScansService.purge_by_build_ids([Security::Scan.last.build_id])
cursor.cursor

    The cursor does not change.

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.

Edited by Gregory Havenga

Merge request reports