Advanced search migration review guidelines
This page is specific to advanced search migration reviews. Refer to our code review guide for broader advice and best practices for code review in general.
When a review is needed
An advanced search migration review is required for:
- Changes to files in
ee/elastic/migrate/ - Changes to index mappings or settings
- Changes to document serialization in
Search::Elastic::References::<IndexedData>or legacyElastic::Latest::<IndexedData>InstanceProxyclasses - Changes that affect indexing behavior or data consistency
Roles
Authors must provide the required artifacts before requesting review. Reviewers ensure artifacts are complete and perform a first-pass review. Maintainers perform the final review and merge when approved.
Required artifacts
You must provide the following when requesting a review. If your merge request description does not include these items, the review is reassigned back to the author.
All migrations
- Migration purpose: What the migration does and why it’s needed.
- Affected document type: Which index and document type is modified (for example,
WorkItem,MergeRequest,Commit). - Backwards compatibility: How the migration handles multi-version compatibility.
- Testing: Run the migration locally and confirm
completed?returns true after execution. For batched migrations, verify batching works as expected.
Mapping or settings changes
- Before and after: Show exact changes to index configuration.
- Impact analysis: How the change affects existing documents, search behavior, and performance.
Backfill or reindex migrations
- Data volume estimate: Approximate number of documents affected.
- Runtime estimate: Expected execution time on GitLab.com using the calculation formula.
- Batch size justification: Why the chosen batch size and throttle delay are appropriate.
- Query plan (if applicable): The Elasticsearch query being executed.
Data modification migrations
- Reversibility: Confirm the migration is reversible or explain why it cannot be.
- Data safety: Safeguards against data loss or corruption.
- Validation: How to verify the migration completed successfully.
Review checklist
Basics
- Migration follows the style guide format.
- Merge request has a changelog.
- Appropriate migration helpers are used.
- Spec file exists with adequate test coverage using shared examples.
- YAML documentation file exists in
ee/elastic/docs/with required fields:description- what the migration doesintroduced_by_url- link to the merge request that introduced the migration- For skippable migrations:
skippable: trueandskip_condition - For obsolete migrations:
marked_obsolete_by_urlandmarked_obsolete_in_milestone
Backwards compatibility
- Migration handles the case where pre-migration application code is still running. See multi-version compatibility.
- Destructive operations (deletions, field removals) are deferred to a later release if needed.
schema_versionis incremented appropriately for document changes. The format isYYVV(two-digit year and rolling version counter within that year, for example24_46).- Code that depends on the migration uses
Elastic::DataMigrationService.migration_has_finished?to check if the migration is complete before using new fields or behavior.
Data safety
- Migration doesn’t introduce orphaned or inconsistent data.
- For document modifications:
as_indexed_jsonmethod is updated to match. - Authorization fields (
visibility_level,traversal_ids, etc.) are handled correctly. - Migration doesn’t conflict with ongoing indexing from
Elastic::ProcessBookkeepingService. - Custom Elasticsearch queries are reviewed for correctness.
Performance
- Runtime estimate is reasonable using the calculation formula.
- Batch size is not greater than
10_000documents. - Throttle delay is appropriate for batch size and operation complexity.
- Large migrations use
batched!with appropriate delays. - For migrations:
preload_indexing_datamethod efficiently loads associated data before indexing. - For search results: Model includes
preload_search_datascope to preload associations when returning results.
Index mappings
- Mapping changes are applied to
Search::Elastic::Types::<IndexedData>or legacyElastic::Latest::<IndexedData>Config. - New fields have appropriate types and analyzers.
- Follow-up backfill migration is included if needed.
How to apply for becoming an advanced search migration reviewer
To apply, you should be familiar with:
- The advanced search migration style guide.
- The review checklist on this page.
- The scope defined in when a review is needed.
You should also have prior experience authoring or reviewing advanced search migrations.
Team members are encouraged to self-identify as advanced search migration domain experts, by adding it to your profile YAML file:
Make a merge request following the project maintainer process for
gitlab-advanced-search-migration.Add your advanced search migration expertise to your YAML file:
projects: gitlab: - reviewer advanced_search_migrationsAssign the merge request to an advanced search migration maintainer or the Global Search Engineering Manager.
After the team.yml update is merged, the Reviewer roulette
may recommend you as an advanced search migration reviewer.