Skip to content

[Migration Engine Part 3] Implement the RavenDB to SQL migration machinery - #5911

Merged
warwickschroeder merged 9 commits into
warwick/migration-engine-2from
warwick/migration-engine-3
Oct 9, 2026
Merged

warwickschroeder merged 9 commits into
warwick/migration-engine-2from
warwick/migration-engine-3

Conversation

@warwickschroeder

@warwickschroeder warwickschroeder commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #5897 (warwick/migration-engine-2). Review against that branch, not master.

What this adds

The machinery that copies an instance's data from RavenDB into SQL Server or PostgreSQL during startup, before the host opens. Only known endpoints and endpoint settings can be copied, so a real instance with Migration/Enabled set still refuses to start; message bodies are not read, and Migration/AllowIncompleteExit does nothing yet.

  • The copy runs during startup, and the host does not open until every category has finished.
  • Each batch commits its rows and checkpoint together, so a restart resumes from the last batch.
  • Only categories both the source and the target support are attempted.
  • Startup checks refuse before anything is copied, including one that blocks any build unable to copy every required category.
  • A stalled or failing copy stops with an error that names the category, what to fix and how to resume.
  • The moment the host first opens on the target is recorded, and every refusal says how to go back to RavenDB before then.
  • An ingestion-only worker refuses to start while a copy into its database is unfinished.
  • Docs gain a system design, and the overview and instructions are updated.

Tests

  • ServiceControl.UnitTests/Migration: the engine, each startup check, the stall watchdog and the refusal messages, against fakes and a fake clock.
  • ServiceControl.Persistence.Tests/EFCore/Migration: the target and its two writers on SQL Server and PostgreSQL, and a check that fails when an entity has no migration decision.
  • ServiceControl.Persistence.Tests.RavenDB/DataMigration: the readers, the source and its data version against an embedded server.
  • ServiceControl.Persistence.Tests.SqlServer: endpoint settings keys that differ only in case, and settings wiring.
  • ServiceControl.Migration.AcceptanceTests (new): a killed copy, a restart, each refusal and the host opening on the target, on both providers.
  • ServiceControl.Migration.Tests: every category with a reader has a writer, and the reverse.

@warwickschroeder warwickschroeder self-assigned this Sep 21, 2026
@warwickschroeder
warwickschroeder added this pull request to stack #5898 September 21, 2026 04:08
@warwickschroeder
warwickschroeder force-pushed the warwick/migration-engine-2 branch from 7dd5105 to a2f9ce8 Compare September 30, 2026 07:25
Adds the RavenDB source and the EF Core target for the KnownEndpoints and EndpointSettings categories, the startup checks that refuse an unsupported or unready migration, and the stall watchdog that stops a copy committing nothing for 30 minutes. Documents the migration contracts.
Adds unit tests for the startup checks, the stall watchdog and the refusals, target and reader tests for both persisters, a SQL Server collation test for the endpoint settings key, and acceptance tests for a copy that is killed, restarted and opened on. Approves the two new migration settings.
@warwickschroeder
warwickschroeder force-pushed the warwick/migration-engine-3 branch from 3fc7d09 to 58d4114 Compare September 30, 2026 08:23
Comment thread docs/migration/ravendb-to-sql-migration-instructions.md
Comment thread docs/migration/ravendb-to-sql-migration-overview.md Outdated
- **Processing attempt history collapses to the newest attempt.** The SQL model has no attempts table. This affects every failed message that failed more than once, whether it is unresolved, archived or resolved. A message that failed five times arrives showing one attempt, and the other four are gone.
- **Subscriptions that differ only in message-type version merge onto one row**, because the target key carries the type name without the version.
- **Endpoint settings for two endpoint names that differ only in case merge onto one row on SQL Server**, because SQL Server's default collation compares names without case, so one of the two settings is kept. PostgreSQL keeps both, and so does a SQL Server database created with a case-sensitive collation. The dry run counts this one too, by asking SQL Server how the name column compares, though for unusual characters its count can differ from what the copy does.
- **Endpoint settings for two endpoint names that differ only in case merge onto one row on SQL Server**, because the collation of the name column decides the comparison and the default collation compares names without case, so one of the two settings is kept. It is the column's own collation that decides, not the database default, so a case-sensitive database whose name column was given a case-insensitive collation still merges. PostgreSQL keeps both. The dry run counts this one too, by asking SQL Server how the name column compares, though for unusual characters its count can differ from what the copy does.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a functional change that could impact licensing, is it actually ok?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically on licensing - licensing never goes through the endpoint settings table. So merging of endpoint settings here doesnt affect that. Licensing is already case-blind on both stores. EF lowercases the licensing key (LicensingDataStore.cs:331, Normalize uses ToLowerInvariant), and RavenDB keys licensing documents on {Name}/{ThroughputSource} (EndpointExtensions.cs:9), and RavenDB ids ignore case. So the migration changes no licensing counts.

Known endpoints dont merge either. They are keyed on a GUID. The only thing that merges is the settings for the endpoints, which is only 1 TrackInstances flag.

SQL Server persistence already does this. EndpointSettingsStore.UpdateEndpointSettings upserts on that case-insensitive key, so a live SQL Server instance can't hold Sales and sales apart either. RavenDB and PostgreSQL keep both.

The question is, whats the impact of this. Its either fine, or its a product bug on SQL Server that the migration just inherited. @johnsimons - thoughts?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like a defect we need to resolve..

  1. The settings sync runs 20 seconds after start and then every 6 hours. It reads the stored names ({"Sales"}), compares them case-sensitively with the known endpoints ({"Sales", "sales"}), and decides sales has no setting.
  2. It then upserts sales with the default value. The key ignores case, so that upsert overwrites the shared Sales row.
  3. Result: both endpoints are reset to the instance-wide default at startup and every 6 hours after. Any choice an operator makes for either one in ServicePulse is lost by the next sync.
  4. Old-instance cleanup only ever matches the stored spelling, so dead instances of sales are never cleaned up when tracking is off.

Comment thread docs/migration/ravendb-to-sql-migration-overview.md Outdated
Comment thread src/ServiceControl.Persistence.RavenDB/DataMigration/RavenMigrationSource.cs Outdated
Comment thread src/ServiceControl.UnitTests/Migration/MigrationEngineHaltTests.cs Outdated
@warwickschroeder
warwickschroeder removed this pull request from stack #5898 October 9, 2026 01:45
@warwickschroeder
warwickschroeder marked this pull request as ready for review October 9, 2026 06:24
@warwickschroeder
warwickschroeder merged commit 1d18223 into warwick/migration-engine-2 Oct 9, 2026
105 of 110 checks passed
@warwickschroeder
warwickschroeder deleted the warwick/migration-engine-3 branch October 9, 2026 06:25
warwickschroeder added a commit that referenced this pull request Oct 9, 2026
…ver and PostgreSQL (#5897)

* feat: Add Migration Checkpoints to EF Core

- Introduced a new table `MigrationCheckpoints` to track migration states and progress.
- Implemented `MigrationCheckpointEntity` to represent the checkpoint data structure.
- Added `EFMigrationCheckpointStore` for managing checkpoint data with methods for reading and upserting checkpoints.
- Created `MigrationCheckpointConfiguration` for configuring the entity in the DbContext.
- Enhanced `ServiceControlDbContext` to include the new `MigrationCheckpoints` DbSet.
- Updated `EfCoreExtensions` to provide an `UpsertCheckpoint` method for managing checkpoint persistence.
- Added unit tests for `EFMigrationCheckpointStore` and `MigrationCheckpointTable` to ensure correct functionality.
- Implemented transaction handling in `MigrationCheckpointTransactionTests` to verify checkpoint behavior during commits and rollbacks.
- Updated RavenDB integration to support embedded server readiness checks.

* feat: Enhance migration engine with improved error handling and configuration options

* feat: Refactor migration checkpoint handling and add UpsertCheckpoint method

* docs: Update migration documentation for clarity on RavenDB to SQL migration process

* [Migration Engine Part 3] Implement the RavenDB to SQL migration machinery (#5911)

* Update migration documentation

* feat: run the required migration copy at host start

Adds the RavenDB source and the EF Core target for the KnownEndpoints and EndpointSettings categories, the startup checks that refuse an unsupported or unready migration, and the stall watchdog that stops a copy committing nothing for 30 minutes. Documents the migration contracts.

* test: cover the RavenDB to SQL migration end to end

Adds unit tests for the startup checks, the stall watchdog and the refusals, target and reader tests for both persisters, a SQL Server collation test for the endpoint settings key, and acceptance tests for a copy that is killed, restarted and opened on. Approves the two new migration settings.

* tests: ensure DB schema is setup before the host starts

* refactor: update BatchSizeFor method to return Task<int> for async compatibility across migration targets

* docs: update migration instructions and system design to clarify settings and behavior

* Refactor migration checks and improve error handling

* Refactor migration commands and error handling

* PR review

* Fix flakey test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants