Report an aged-out sync anchor as a distinguishable condition - #19
Merged
Conversation
A client whose anchor has fallen outside the store's change retention window cannot be served incremental changes, and no amount of retrying will change that: the history it asks for has been cleaned up. SqlServerCT already raised the typed SyncAnchorTooOldException for this, but the condition was still indistinguishable everywhere else - the remaining providers threw a bare InvalidOperationException, and over HTTP it reached the client as an unhandled exception that an application could only report as "unable to synchronize". - Throw SyncAnchorTooOldException from the SqlServer, PostgreSQL, Sqlite and MySql providers instead of InvalidOperationException. These stores keep one shared change journal rather than per-table history, so they get a constructor that carries no table name and a message that does not invent one. MinValidVersion is reported as MIN(ID) - 1, the oldest anchor that can still be resolved, which keeps the requested < min-valid invariant true. - Surface it over HTTP as 410 Gone with an X-CoreSync-Error: anchor-too-old header and a SyncAnchorTooOldError body. 410 is the honest code: the history existed once and is permanently gone, so unlike a 5xx there is nothing to retry. An endpoint filter applies this to every sync endpoint, covering both the download path and the upload guard. - Rebuild the exception client-side from that signal, so callers see the same typed error they would from a local provider, with the table and both versions intact. The header alone identifies the condition, so a body that cannot be parsed does not downgrade it back into a transport failure. - Stop the binary download retry loop from retrying it: three round trips and fifteen seconds to reach an answer that cannot change. This does not attempt automatic recovery - an aged-out client still has to be reinitialized from a fresh snapshot by hand. What changes is that both the server logs and the client can now say so, instead of reporting an anonymous failure.
The anchor validation runs ahead of the per-table loop that emits the ordinary progress traces, so a session that failed here recorded its opening "Begin GetChanges" line and nothing else. The reason was reachable, but only outside the session's own trace list. Log the condition at error level, with the table and both versions, at each point it is raised. A failed session now carries its own explanation.
Retention is reconciled as part of ApplyProvisionAsync, which is the right place for it during provisioning but leaves no way to act on the setting on its own. A host that lets an operator edit the retention needs one: storing the value changes nothing on a database that already has change tracking enabled, and a setting that appears saved but was never applied is how a database quietly keeps a week of history when it was configured for a month. Add SqlServerCTProvider.ApplyChangeRetentionAsync, which issues the same reconciliation and then reads the settings back from sys.change_tracking_databases so the caller can report what the database actually holds rather than what was asked for. Making that return value useful means ChangeTrackingDatabaseOptions becomes public.
SyncAnchorTooOldException was rethrown unwrapped so callers could branch on it
without digging through inner exceptions. That reads well until you count who it
breaks: callers have caught SynchronizationException around SynchronizeAsync
since before the typed exception existed, and an escaping subclass gives them
nothing to catch - at runtime, with no compile-time warning.
When only SqlServerCT could raise it the blast radius was small. Now that every
provider does, it covers Sqlite, which is what most clients run locally.
Wrap it like everything else and let callers branch on the inner exception:
catch (SynchronizationException ex)
when (ex.InnerException is SyncAnchorTooOldException tooOld)
Documented on SynchronizeAsync, and pinned by a test so it cannot regress
quietly. The provider-level rethrow stays: it keeps the typed exception out of
SyncErrorException so it still arrives intact as the inner exception.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A client whose anchor has fallen outside the store's change retention window cannot be served incremental changes, and no amount of retrying will change that — the history it asks for has been cleaned up.
SqlServerCTalready raised the typedSyncAnchorTooOldExceptionfor this (#17), but everywhere else the condition was still indistinguishable: the other providers threw a bareInvalidOperationException, and over HTTP it reached the client as an unhandled exception that an application could only report as "unable to synchronize".This is the dominant cause of a real field failure — devices idle longer than the retention window sync forever without ever being told why, and the only cure users find is a destructive local reset.
What changed
SqlServer,PostgreSQL,SqliteandMySqlnow throwSyncAnchorTooOldException. These stores keep one shared change journal rather than per-table history, so they get a constructor that carries no table name and a message that does not invent one.MinValidVersionis reported asMIN(ID) - 1, the oldest anchor that can still be resolved, which keeps therequested < min-validinvariant true.410 Gonewith anX-CoreSync-Error: anchor-too-oldheader and aSyncAnchorTooOldErrorbody. 410 is the honest code: the history existed once and is permanently gone, so unlike a 5xx there is nothing to retry. An endpoint filter applies it to every sync endpoint, covering both the download path and the upload guard.Begin GetChangesline and nothing else.ApplyChangeRetentionAsynconSqlServerCTProviderapplies the configured retention and reads it back fromsys.change_tracking_databases, for hosts that let an operator edit retention on its own. Storing a value changes nothing on a database that already has change tracking enabled, and a setting that appears saved but was never applied is how a database quietly keeps a week of history when it was configured for a month.Compatibility
Nothing here is binary-breaking, and no existing public signature changed.
SynchronizeAsyncstill wraps every failure inSynchronizationException, this one included. It briefly did not — #17 rethrew it unwrapped so callers could branch without digging through inner exceptions — but callers have caughtSynchronizationExceptionaroundSynchronizeAsyncsince before the typed exception existed, and an escaping subclass gives them nothing to catch, at runtime, with no compile-time warning. That was tolerable when onlySqlServerCTcould raise it; now that every provider does, it coversSqlite, which is what most clients run locally. Callers branch on the inner exception:Worth calling out in release notes, none of them runtime-silent:
SyncAnchorTooOldException.TableNameis nowstring?— it is genuinely absent for the single-journal providers. Binary-compatible; a nullable warning for consumers, a build break only underTreatWarningsAsErrors.EnsureSuccessStatusCode()and getHttpRequestExceptioneither way, so mixed-version deployments are no worse.CoreSync.Http.ClientthrowsSyncAnchorTooOldExceptioninstead ofHttpRequestExceptionfor this case — only for direct provider callers, and only against an upgraded server.Suggested release: 1.1.0.
Not in scope
No automatic recovery. An aged-out client still has to be reinitialized from a fresh snapshot by hand — deliberately, since re-seeding safely needs a guarantee that no unsynced local rows are discarded. What changes is that both sides can now say why, instead of reporting an anonymous failure.
Tests
AnchorTooOldTestscovers the aged-out path end to end: the typed error and its versions from a provider, the boundary case that must still succeed, the 410 wire format, the client rebuilding the exception from it (download path, upload path, and store-wide form), an unrelated failure not being misread as this one, and theSynchronizationExceptionwrapping contract so it cannot regress quietly.43 tests pass locally (the SQL Server, PostgreSQL and MySQL suites need the CI service containers).