Skip to content

Allow Elastic migrations to be skipped

Madelein van Niekerk requested to merge 451431-skippable-elastic-migrations into master

What does this MR do and why?

Introduces a new trait for Elastic migrations to allow a migration to be skipped when a condition is not met, for example, if Elastic is not on a supported version.

A migration can define a skip condition by:

class Test < Elastic::Migration
  skip_if ->() { true|false }
end

When the migration worker runs (every 5 minutes), we will try to find the next migration to execute with the Elastic::MigrationRecord.current_migration method. This method has been updated to exclude migrations where migration.skip? evaluates to true. The pending_migrations? and pending_migrations also exclude skipped migrations since these are not required to be completed for an upgrade to continue since the migration could be skipped indefinitely.

Implementation notes

I initially added a skipped true|false field to the migration state but then removed it because (1) determining when to set it to skipped is tricky (is it when a migration was supposed to be run but was skipped or the first time it was checked to be skipped? and (2) if we add a skipped state, we also create the migration record in Elasticsearch which messes with the way we check for completed migrations and I thought it's not worth adding custom logic just to have that state. If the reviewers feel it would be useful to add a skipped state, please let me know.

Next up

  1. Add developer documentation to show how to add a skippable migration
  2. Show skipped Elastic migrations to users (#455295)

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Migration created, skip_if is true

{"severity":"INFO","time":"2024-04-10T12:15:44.899Z","class":"Elastic::MigrationWorker","message":"MigrationWorker: no migration available","job_status":"running","queue":"default","jid":null}

skip_if changes to false

{"severity":"INFO","time":"2024-04-10T12:16:04.481Z","class":"Elastic::MigrationWorker","message":"MigrationWorker: migration[Test] executing migrate method","job_status":"running","queue":"default","jid":null}
{"severity":"INFO","time":"2024-04-10T12:16:04.481Z","class":"Elastic::MigrationWorker","message":"MigrationWorker: migration[Test] updating with completed: true","job_status":"running","queue":"default","jid":null}
{"severity":"INFO","time":"2024-04-10T12:16:13.001Z","class":"Elastic::MigrationWorker","message":"MigrationWorker: no migration available","job_status":"running","queue":"default","jid":null}

How to set up and validate locally

  1. Create a migration file with a skip_if -> { ... } option defined. Make the condition evaluate to true at first.
  2. Execute the migration worker: Elastic::MigrationWorker.new.perform
  3. See that the migration wasn't executed by checking the logs
  4. Verify that the migration isn't shown as pending: bundle exec rake gitlab:elastic:list_pending_migrations
  5. Change the condition to evaluate to false and execute the worker again.
  6. Verify that the migration now executes and is no longer skipped

Related to #454763 (closed)

Edited by Madelein van Niekerk

Merge request reports