Skip to content

Add opt-in import state cache (CacheImportState, default off) - #1016

Merged
KevinJump merged 1 commit into
v18/mainfrom
v18/feature/import-state-cache
Aug 5, 2026
Merged

Add opt-in import state cache (CacheImportState, default off)#1016
KevinJump merged 1 commit into
v18/mainfrom
v18/feature/import-state-cache

Conversation

@KevinJump

Copy link
Copy Markdown
Owner

Why

Someone passed on an AI analysis of v18/main focused on import performance on large sites (thousands of content and dictionary files). Its main finding checks out:

SyncSerializerRoot.DeserializeAsync calls IsCurrentAsync for every item, and IsCurrentAsync does a database lookup, a full re-serialize of the Umbraco item, CleanseNode on both sides, then two hashes. The report path pays the same through SyncHandlerRoot.IsItemCurrentAsync.

So an import where nothing has changed costs about as much as a full export. Reading the files is not the bottleneck — that is already nicely parallelised in GetFolderItemsAsync — the per-item work afterwards is, and at a few thousand files it is the whole run time.

Two of the analysis's supporting claims are real but smaller than presented, and are not addressed here (they are cheap independent fixes, deliberately not entangled with a feature flag):

  • DictionaryItemSerializer.CleanseNode really does deep-clone via XElement.Parse(node.ToString()), for both sides of every comparison, and GetLevelAsync walks the parent chain with an uncached service lookup per level.
  • The "hash the raw file bytes instead of parsing the XML" idea does not save the parse — GetFolderItemsAsync has to parse every file anyway for key/level/path. The saving is in skipping the database lookup, the re-serialize and the second hash. So this hashes the in-memory XElement we already have.

What this does

Adds uSync:Settings:CacheImportState (default false). With it on, uSync remembers the hash of each file it has confirmed matches Umbraco, and skips those items on the next run with no database lookup and no serialize. Import cost goes from O(all items) of database and serialization work to O(changed items).

Only confirmed matches are recorded

An entry is written in exactly two cases: the full check ran and returned NoChange, or we have just exported the item (so the file was written from the database and the two match by construction).

It deliberately does not record after a successful update or create. Tempting, but some items do not round-trip exactly — that is what uSync's "XML is different - but properties may not have changed" message is telling you — and recording those would silence a real difference for good.

Consequence worth being clear about: the first run after enabling is no faster than before. The benefit arrives on the second run. An export warms it too.

How it hooks in

Through uSync's existing per-item notifications. The report one already carried a comment saying it exists for precisely this — "this lets us intercept a report and shortcut the checking (sometimes)".

Notification What happens
uSyncImportingItemNotification / uSyncReportingItemNotification cancel the item if we already know the file matches
uSyncImportedItemNotification / uSyncReportedItemNotification record the hash if the answer was NoChange
uSyncExportedItemNotification record the hash — we just wrote the file from the database
uSync*Starting / uSync*Completed load / save the cache file

SyncSerializerRoot, every serializer, and the handler import/report methods are untouched. No constructor signatures changed either, which would have been source-breaking for uSync.Complete and community handlers/serializers. The only change to the pipeline is 19 lines in SyncHandlerRoot, all additive.

Storage and invalidation

  • {LocalTempPath}/uSync/cache/state-{identity}.jsonnever in the uSync folder. The cache describes this site's database, not the source of truth, so it must not travel between environments or show up in a diff. (Same precedent as uSync.History.)
  • Identity = a GUID stamp kept in Umbraco's key/value table (same pattern as SyncTrackerService) + the uSync version + a fingerprint of the settings that affect serialization. Any mismatch and the file is discarded. If the stamp can't be read, the cache isn't used at all.
  • SyncStateCacheInvalidator listens to Umbraco. Three levels: the item (saved/deleted/published), the whole type (moved — a move rewrites the path of every descendant), and everything (doc type, data type, template, language, container — these get embedded in other items' xml, so changing one silently changes items whose own rows never moved).
  • A force import ignores the cache entirely.

Two decisions reviewers should look at

No IsPaused guard on invalidation. The original design had one. Working through the ordering, it is both unnecessary and less safe: an item uSync writes gets invalidated and then simply isn't re-recorded (we only record confirmed NoChange), and dropping the guard also catches items Umbraco saves as a side effect of an import, which a paused check would miss. Reasoning is in the class docs.

Invalidations are journalled to disk. This turned out to be load-bearing, not a nicety. Removing an entry from memory does nothing about the copy in the file, and the file is what the next restart trusts — so an item saved between a run and a restart would be wrongly skipped. That is exactly the ImportAtStartup case, i.e. a main reason to want this at all. A tiny append per save, folded back in and cleared on the next persist, both halves under one lock so nothing can be lost between "manifest written" and "journal cleared".

Also: the identity fingerprint is built by hand rather than by serializing the settings object, because dictionaries bound from configuration enumerate in provider order. If that wobbled the identity would change on restart and the cache would silently never survive one — the only symptom being "the feature does nothing". There is a regression test for it.

Limitations (why it's off by default)

Full list in docs/perf/state-cache.md. The headline: with the cache on, uSync's change detection stops being self-verifying and starts trusting its own bookkeeping. It cannot see a database change made by something that raises no Umbraco notification (raw SQL, a row-level restore). The database stamp catches a whole-database swap; it cannot catch a targeted edit.

The pitfall I'd watch hardest is not raw SQL though — it's cross-item dependencies. The known ones are handled, but any serializer that pulls data in from another item is a new hole. That's called out in the docs for third-party serializer authors, and the policy lives in one set in SyncStateCache so it's a one-line extension.

Also documented: report is no longer an independent check (force import is the way to get a guaranteed full one); dictionary items whose file key differs from the database key; multiple handler sets with differing serialization settings; and LocalTempPath not surviving on Azure App Service / containers / scale-out, where the cache may rarely be warm and the feature quietly does little.

Deliberately not in this PR

UpdateDate verification. Storing each item's UpdateDate and validating against one IEntityService.GetAll(objectType) query per handler would give existence plus a timestamp for every item in a single query, at no per-item cost, closing the raw-SQL gap for real Umbraco node types. That's the obvious hardening step if notification-based invalidation proves too leaky; the manifest has a format version so it can be added without a migration.

Testing

  • dotnet build uSync.slnx — clean, 0 warnings, 0 errors.
  • 197 tests pass, including 20 new ones in uSync.Tests/Cache/SyncStateCacheTests.cs. They run against a real temp folder rather than a mocked file service, because the bits most likely to break involve real files. Notably: the hash surviving a save/reload round trip (the whole export warm-up depends on it), identity stability across differing dictionary order, invalidations surviving a reload, and the three invalidation levels.

Not yet verified against a real site. The acceptance test still to do: enable it on a site with a few thousand content and dictionary items, run a report twice, and confirm run 2 is dramatically faster and produces an identical action list to run 1. Any difference between the two is a bug. Also worth walking through: edit one item (only that item re-checked), rename a doc type (everything re-checked), move a node (descendants re-checked), force import (cache ignored), hand-edit a .config (change detected), change a handler setting (cache discarded), and setting the flag back to false (timings return to baseline).

🤖 Generated with Claude Code

uSync decides whether an item has changed by loading it from Umbraco,
serializing the whole thing and comparing hashes - for every item, every
run. SyncSerializerRoot.DeserializeAsync calls IsCurrentAsync for each
item, and that does a database lookup, a full re-serialize, CleanseNode
on both sides and two hashes. The report path pays the same through
SyncHandlerRoot.IsItemCurrentAsync.

The result is that an import where nothing has changed costs about as
much as a full export. Reading the files is not the bottleneck - that is
already parallelised in GetFolderItemsAsync - the per-item work
afterwards is, and on sites with thousands of content and dictionary
files that is the whole run time.

With CacheImportState on, uSync remembers the hash of each file it has
confirmed matches and skips those items next run with no database lookup
and no serialize, taking import cost from O(all items) of database and
serialization work to O(changed items).

Implemented entirely through uSync's existing per-item notifications -
the report one already carried a comment saying it exists for exactly
this ("this lets us intercept a report and shortcut the checking"). So
SyncSerializerRoot, every serializer, and the handler import and report
methods are untouched, and no constructor signatures changed (which
would have been source-breaking for uSync.Complete and community
handlers).

Only confirmed matches are recorded: either the full check ran and
returned NoChange, or we have just exported the item so the file was
written from the database. It never assumes that because an import
succeeded the two sides now agree - some items do not round-trip
exactly, and recording those would silence a real difference for good.
The cost of that choice is that the first run after enabling is no
faster than before; the benefit arrives on the second run.

The cache lives in the site's temp folder, never in the uSync folder,
and carries an identity (a stamp kept in Umbraco's key/value table, the
uSync version, and a fingerprint of the settings that affect
serialization) so it is discarded whenever it cannot be proven to still
apply. Items are forgotten when Umbraco reports a save, delete, move or
publish; changing a doc type, data type, template, language or container
clears the whole cache, because those get embedded in other items' xml.
A force import ignores the cache entirely.

Two details worth calling out:

- Invalidations are journalled to disk. Removing an entry from memory
  does nothing about the copy in the file, and the file is what the next
  restart trusts, so an item saved between a run and a restart would
  otherwise be wrongly skipped - exactly the ImportAtStartup case.
- The identity fingerprint is built by hand rather than by serializing
  the settings, because dictionaries bound from configuration enumerate
  in provider order. If that wobbled, the identity would change on
  restart and the cache would silently never survive one.

Also adds an optional Message to CancelableuSyncItemNotification (used
instead of the generic "change stopped by delegate event" when set) and
Force to the importing/reporting notifications, so a subscriber that
short-cuts the check can stand down on a forced import.

Off by default: what it cannot see is a database change made by
something that raises no Umbraco notification. See
docs/perf/state-cache.md for the full set of limitations, including the
note for anyone writing a custom serializer that embeds data from
another item.

Not yet verified against a real site - the run-1 vs run-2 action list
comparison still needs doing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@KevinJump
KevinJump merged commit 4eeb3df into v18/main Aug 5, 2026
5 checks passed
@KevinJump
KevinJump deleted the v18/feature/import-state-cache branch August 5, 2026 10:03
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.

1 participant