Repository navigation
Migration engine: Copy licensing throughput history to SQL - #6063
warwickschroeder wants to merge 2 commits into
Conversation
| - The dry run lists each pair by asking SQL Server how the column compares. For unusual characters its list can differ from what the copy does. | ||
| - **On SQL Server, licensing endpoints whose names differ only in trailing spaces become one row.** SQL Server ignores trailing spaces when it compares the key. PostgreSQL keeps both. | ||
| - The copy keeps the endpoint whose RavenDB document id sorts first. | ||
| - A day that both endpoints counted keeps the count of the endpoint whose id sorts later. The two counts are not added. |
There was a problem hiding this comment.
Yes. We need to keep these endpoints separate. Will look into a fix
| public override string CategoryId => MigrationCategoryIds.LicensingEndpoints; | ||
|
|
||
| public override IAsyncEnumerable<MigrationBatch> Read(string? resumeAfter, int batchSize, CancellationToken cancellationToken = default) => | ||
| RavenDocumentStream.ByPrefix<EndpointDocument>(Lifecycle, CategoryId, Lifecycle.Settings.ThroughputDatabaseName, string.Empty, resumeAfter, batchSize, RavenDocumentStream.MaxDocumentsWalkedPerBatch, ThroughputDocuments.Endpoint, cancellationToken); |
There was a problem hiding this comment.
The endpoint readers do not honor their resume cursors
(also applies to LicensingThroughputReader)
-
What this is: Both readers pass string.Empty to RavenDocumentStream.ByPrefix, expecting document-ID ordering and startAfter support.
-
Problem: RavenDB treats an empty prefix differently: it streams in reverse etag order and ignores startAfter. I reproduced this against the bundled RavenDB 6.2.18 server:
Insert: A/Broker, Z/Broker Initial stream: Z/Broker, A/Broker Resume after Z/Broker: Z/Broker, A/Broker -
Fix: Use a read-only scan with guaranteed ID ordering and a cursor it honors. Add reader-level tests that deliberately separate insertion order from ID order and resume after a committed
batch. -
If skipped: A crash resume rereads committed endpoints and days, inflating checkpoint totals and repeating work. Throughput collision winners also follow etag order rather than the
documented ID order.
There was a problem hiding this comment.
I'll add this curser fix as it would help with migration time and yes it would give inflated reporting numbers at the end of the migration, but throughput values are SET, not added or inserted. So migrating the same endpoints again wont inflate numbers, at least
| - **On SQL Server, licensing endpoints whose names differ only in trailing spaces become one row.** SQL Server ignores trailing spaces when it compares the key. PostgreSQL keeps both. | ||
| - The copy keeps the endpoint whose RavenDB document id sorts first. | ||
| - A day that both endpoints counted keeps the count of the endpoint whose id sorts later. The two counts are not added. | ||
| - This is not counted as a merge. The second endpoint and its shared days count as already present, the copy logs no warning, and the dry run does not list them. So on a category copied once, `--migration-verify` shows them as already-present rows that merges do not explain. |
There was a problem hiding this comment.
Updating a shared day returns a written row, which increments Copied, not AlreadyPresent. Two endpoints sharing one day therefore produce Copied=2, AlreadyPresent=0, and one SQL row. The persistence test explicitly asserts this.
What this adds
The migration now copies the licensing data in RavenDB's second database,
throughput, to SQL Server or PostgreSQL while ServiceControl is closed: the endpoint list, up to fourteen months of daily message counts, the report masks and the uploaded licence file. A copied day counts as already collected, so the throughput collectors never add to it.LicensingEndpointsReaderandLicensingEndpointsWritercopy endpoint documents, keyed by the product's own normalised name and throughput source.LicensingThroughputReaderandLicensingThroughputWritercopy each day after its endpoint, and set the count rather than add to it, so a re-run or resume never doubles one.Settingsrow the product reads, and a row already there is left alone.EndpointDocumentscollection are read as endpoints.MigrationInsertreads returned keys as their stored type, so adatekey matches on SQL Server.Normalizebecomeinternalfor the migration to use.ServiceControl.PersistencereferencesParticular.LicensingComponent.Contractsfor the shared day row.Tests
ServiceControl.Persistence.Tests/EFCore/Migration: the four writers, the mappers, set-not-add across batches, a day batch bigger than one SQL statement, value length skips and the date key read back.ServiceControl.Persistence.Tests.RavenDB/DataMigration: reading endpoints, counts and both settings documents from the throughput database, skipping other collections, and resuming.ServiceControl.Migration.AcceptanceTests: an end-to-end copy on both providers, an unchanged re-run, trailing-space endpoint pairs, counts blocked behind a skipped endpoint, and the broker collector not counting a copied day twice.