Skip to content

Performance: Skip the content cache rebuild when a document type property is removed - #23330

Merged
Zeegaan merged 5 commits into
v17/devfrom
v17/improvement/skip-content-rebuild-for-deleted-property
Jul 14, 2026
Merged

Performance: Skip the content cache rebuild when a document type property is removed#23330
Zeegaan merged 5 commits into
v17/devfrom
v17/improvement/skip-content-rebuild-for-deleted-property

Conversation

@AndyButland

@AndyButland AndyButland commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to the content-cache-rebuild investigation. Removing a property from a document type currently triggers a full rebuild of the published database cache (cmsContentNu) for every document of that type — expensive on large sites — even though the removal does not actually change what published content exposes.

A property removal does not require a cmsContentNu rebuild: published content is always resolved against the current content type, so a removed alias simply stops mapping to a property and its (now orphaned) value in the stored blob is never read. This PR skips the rebuild for that case and only clears the converted in-memory cache instead.

How it works

  • Adds ContentTypeChangeTypes.RawDataUnaffected — an additive flag that supplements RefreshMain. It is set (in ComposeContentTypeChanges) only when a property removal is the sole structural cause of a change (no alias change, variation change, composition removal, etc.).
  • RefreshMain is retained, so everything else keyed off it is unchanged — search re-indexing, published-content-type cache clearing and model-factory reset all still run. Only the raw cmsContentNu rebuild is skipped.
  • The three cache handlers interpret the flag: CacheRefreshingNotificationHandler (in-scope), DeferredCacheRebuildNotificationHandler (deferred), and ContentTypeCacheRefresher (distributed / other servers) rebuild only when RequiresRawDataRebuild(), otherwise just clear the converted cache.
  • A guard in ComposeContentTypeChanges (keyed by content type Id) ensures a type that also independently needs a rebuild — e.g. a batch save where it inherits a renamed property via a composition — is never flagged.

Why the flag rather than reclassifying to RefreshOther

RefreshMain drives more than the cmsContentNu rebuild (notably Examine re-indexing). Reclassifying a removal as RefreshOther would silently turn those off, and would misrepresent a destructive change. The additive flag keeps all RefreshMain handling and lets only the rebuild step opt out. It is also backwards- and rolling-upgrade-tolerant (a not-yet-upgraded node treats the change as a normal RefreshMain).

Notes

  • Correctness depends on the published content type cache being refreshed (it always is, independent of the rebuild). The removed property's bytes linger in the stored blob until the row is next rewritten; they are unreachable and reclaimed naturally.

Testing

Automated

Solution builds and CI checks should pass. Adds unit tests for the new change-type helpers and the three handlers' routing, and integration tests proving a removed property is no longer exposed by any published read path while the stored blob is provably not rebuilt, that alias/composition changes still rebuild, and that the batch multi-type guard holds.

Manual

These steps verify from the log that removing a property skips the cmsContentNu rebuild, while another structural change (e.g. a variation change) still triggers it.

1. Enable debug logging

Add a Serilog level override for the cache-refreshing handler in appsettings.json (or appsettings.Development.json) and restart the site:

{
  "Serilog": {
    "MinimumLevel": {
      "Override": {
        "Umbraco.Cms.Infrastructure.HybridCache.NotificationHandlers.CacheRefreshingNotificationHandler": "Debug"
      }
    }
  }
}

Log output goes to the console and to umbraco/Logs.

2. Prepare

Have (or create) a document type with at least two properties and at least one document of that type; optionally publish it.

3. Remove a property → no rebuild

Edit the document type, delete a property, and save. Expect a converted-cache clear and no rebuild line:

[DBG] Content type change: clearing the converted document cache only (no database rebuild) for content type(s) [1234].

You should not see any Content type change: rebuilding the document database cache ... line for this save.

Functional check: the removed property is no longer exposed by the published content — e.g. @Model.Value("<alias>") returns nothing, and the Delivery API response omits it — while the remaining properties are unaffected.

4. Structural change (variation) → rebuild still happens

Edit the same document type, change its variation setting (toggle Allow varying by culture), and save. Expect the rebuild line (Immediate mode, the default):

[DBG] Content type change: rebuilding the document database cache for content type(s) [1234].

(Renaming a property's alias is a simpler alternative trigger if you'd rather not change variance — it produces the same rebuilding ... line.)

5. (Optional) Deferred mode

If the site runs Umbraco:CMS:Cache:ContentTypeRebuildMode = Deferred, the structural change from step 4 instead logs that the rebuild is handed off, followed by the background rebuild's own Information-level lines:

[DBG] Content type change: document database cache rebuild for content type(s) [1234] left to the deferred rebuild (ContentTypeRebuildMode.Deferred).
[INF] Deferred rebuild starting for content type IDs: [1234]
[INF] Deferred rebuild completed for content type IDs: [1234]

A property removal (step 3) in deferred mode still logs only the converted-cache clear — it is never queued for a deferred rebuild.

Media types: the same behaviour applies to media types; the log lines read Media type change: ....

Copilot AI review requested due to automatic review settings July 8, 2026 15:03
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

Claude finished @AndyButland's task in 8m 59s —— View job


PR Review

Target: origin/v17/dev · Based on commit: 2f337fb8860b11198cf20bb53d2448e313f56291

Adds ContentTypeChangeTypes.RawDataUnaffected to skip the expensive cmsContentNu rebuild when a document/media type property is removed, replacing it with a cheaper converted-cache clear — while preserving all other RefreshMain handling (Examine reindexing, published content type cache clearing, model factory reset).

  • Modified public API: ContentTypeChangeTypes (new RawDataUnaffected = 32 value); ContentTypeChangeExtensions (new RequiresRawDataRebuild(), RequiresConvertedCacheClearOnly() extension methods)
  • Affected implementations (outside this PR): ContentTypeIndexingNotificationHandler, DeliveryApiContentIndexHandleContentTypeChanges, and ServerEventSender all call HasType(ContentTypeChangeTypes.RefreshMain) — they are unaffected (the flag is additive, RefreshMain stays set). DocumentUrlServiceContentTypeChangedNotificationHandler checks HasFlag(VariationChanged) — also unaffected, as VariationChanged is never set for a property-removal-only change.
  • Other changes: Three cache handler paths (in-scope CacheRefreshingNotificationHandler, deferred DeferredCacheRebuildNotificationHandler, distributed ContentTypeCacheRefresher) now use RequiresRawDataRebuild()/RequiresConvertedCacheClearOnly() instead of IsStructuralChange()/IsNonStructuralChange() to decide whether to rebuild or just clear the converted cache.

Important

  • tests/Umbraco.Tests.Integration/.../DocumentHybridCacheDocumentTypeTests.cs:186: ContentTypeChangeCapture.Changes is a static readonly List<> shared across all tests in the session. Each capture-relying test clears it before its Act — fragile: a future test that forgets, or a test that throws before clearing, will contaminate subsequent tests. Instance field + [SetUp] reset is safer. (Inline comment posted.)

  • tests/Umbraco.Tests.UnitTests/.../CacheRefreshingNotificationHandlerTests.cs:143: RawDataUnaffected_Content_Type_Change_Skips_Rebuild_And_Clears_Converted_Cache only covers immediate mode. The deferred + RawDataUnaffected path has its own subtle interaction: CacheRefreshingNotificationHandler must still clear the converted cache in deferred mode, while DeferredCacheRebuildNotificationHandler must not queue a rebuild. A companion test with ContentTypeRebuildMode.Deferred would close that gap, especially since both handlers were updated in this PR. (Inline comment posted.)

Suggestions

  • src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/CacheRefreshingNotificationHandler.cs:132: The else-branch log "deferring the document database cache rebuild" is slightly misleading — this handler does not enqueue the rebuild; that is DeferredCacheRebuildNotificationHandler. Consider dropping the else-branch log or rephrasing to reference the handler that actually performs the deferral. Same pattern on line 208 for media types. (Inline comment posted.)

Approved with Suggestions for improvement

Good to go, but please carefully consider the importance of the suggestions.

The correctness argument is solid: published content is always resolved against the current content type definition, so an orphaned blob value for a removed property is simply never mapped. The Id-based guard in ComposeContentTypeChanges correctly handles batch saves where the same type arrives via both direct save and composition propagation. Rolling-upgrade safety is preserved (pre-v17 nodes read the payload as a plain RefreshMain and do a full rebuild). All three handler paths (in-scope, deferred, distributed) are consistently updated and well-tested.

Copilot AI left a comment

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.

Pull request overview

This PR introduces a new “raw data unaffected” content-type change flag so that document type property removals no longer trigger expensive cmsContentNu rebuilds, while still performing the other RefreshMain side-effects (e.g., published content type cache refresh / indexing triggers).

Changes:

  • Added ContentTypeChangeTypes.RawDataUnaffected plus helper extensions (RequiresRawDataRebuild, RequiresConvertedCacheClearOnly) to route rebuild vs. converted-cache clearing.
  • Updated the three cache-handling paths (in-scope, deferred, and distributed) to skip raw-data rebuild when the change is a pure property removal.
  • Added unit + integration coverage ensuring property removal hides published values without rebuilding the stored blob, while alias/composition changes still rebuild.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/DeferredCacheRebuildNotificationHandlerTests.cs Adds coverage that RawDataUnaffected changes do not queue deferred rebuilds.
tests/Umbraco.Tests.UnitTests/Umbraco.PublishedCache.HybridCache/CacheRefreshingNotificationHandlerTests.cs Verifies rebuild is skipped and converted cache is cleared for RawDataUnaffected changes.
tests/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ContentTypeChangeExtensionsTests.cs Tests the new change-type helper methods for rebuild vs. clear-only behavior.
tests/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/ContentTypeCacheRefresherTests.cs Ensures distributed refresher clears converted cache without rebuilding memory cache for RawDataUnaffected.
tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DocumentHybridCacheDocumentTypeTests.cs Integration proof: property removal hides published values without cmsContentNu rebuild; guards for alias/composition/batch cases.
src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/DeferredCacheRebuildNotificationHandler.cs Queues deferred rebuilds only when raw-data rebuild is actually required.
src/Umbraco.PublishedCache.HybridCache/NotificationHandlers/CacheRefreshingNotificationHandler.cs Routes between rebuild vs. clear-converted-only and adds debug logging for rebuild/deferral decisions.
src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs Classifies property-removal-only structural changes as RawDataUnaffected with an Id-based guard against false positives in batch/composition scenarios.
src/Umbraco.Core/Services/Changes/ContentTypeChangeTypes.cs Introduces the new RawDataUnaffected flag on the public enum.
src/Umbraco.Core/Services/Changes/ContentTypeChangeExtensions.cs Adds the new change-type helper methods used by handlers/refreshers.
src/Umbraco.Core/Cache/Refreshers/Implement/ContentTypeCacheRefresher.cs Distributed routing: rebuild memory cache only when raw-data rebuild is needed; otherwise clear converted cache selectively.

Comment thread src/Umbraco.Core/Services/ContentTypeServiceBase{TRepository,TItem}.cs Outdated
@claude claude Bot added area/backend category/performance Fixes for performance (generally cpu or memory) fixes labels Jul 8, 2026
@sonarqubecloud

Copy link
Copy Markdown

@Zeegaan Zeegaan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this makes good sense, great too see all the tests too 🙌

@Zeegaan
Zeegaan merged commit 8c98c63 into v17/dev Jul 14, 2026
30 of 31 checks passed
@Zeegaan
Zeegaan deleted the v17/improvement/skip-content-rebuild-for-deleted-property branch July 14, 2026 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/backend category/performance Fixes for performance (generally cpu or memory) fixes release/17.6.0 release/18.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants