From f408d2a1b33525bb76a42f8248dc0ffdbb2340be Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Tue, 2 Dec 2025 02:09:54 +0100 Subject: [PATCH] Migrations: Optimise `ConvertLocalLinks` migration to process data in pages, to avoid having to load all property data into memory (#21003) * Optimize ConvertLocalLinks migration to process data in pages, to avoid having to load all property data into memory. * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Updated obsoletion warning. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> (cherry picked from commit 742de79f4692dd6dccb671f5e1049fdf1abacfd5) --- .../Upgrade/V_15_0_0/ConvertLocalLinks.cs | 235 +++++++++++------- 1 file changed, 147 insertions(+), 88 deletions(-) diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_0_0/ConvertLocalLinks.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_0_0/ConvertLocalLinks.cs index f3914b854c..73508bcc7b 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_0_0/ConvertLocalLinks.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_0_0/ConvertLocalLinks.cs @@ -17,6 +17,13 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_15_0_0; +/// +/// Migrates local links in content and media properties from the legacy format using UDIs +/// to the new one with GUIDs. +/// +/// +/// See: https://github.com/umbraco/Umbraco-CMS/pull/17307. +/// public class ConvertLocalLinks : MigrationBase { private readonly IUmbracoContextFactory _umbracoContextFactory; @@ -30,7 +37,9 @@ public class ConvertLocalLinks : MigrationBase private readonly ICoreScopeProvider _coreScopeProvider; private readonly LocalLinkMigrationTracker _linkMigrationTracker; - [Obsolete("Use non obsoleted contructor instead")] + /// + /// Initializes a new instance of the class. + /// public ConvertLocalLinks( IMigrationContext context, IUmbracoContextFactory umbracoContextFactory, @@ -57,6 +66,10 @@ public class ConvertLocalLinks : MigrationBase _linkMigrationTracker = linkMigrationTracker; } + /// + /// Initializes a new instance of the class. + /// + [Obsolete("Please use the constructor taking all parameters. Scheduled for removal along with all other migrations to 17 in Umbraco 18.")] public ConvertLocalLinks( IMigrationContext context, IUmbracoContextFactory umbracoContextFactory, @@ -83,6 +96,7 @@ public class ConvertLocalLinks : MigrationBase { } + /// protected override void Migrate() { IEnumerable propertyEditorAliases = _localLinkProcessor.GetSupportedPropertyEditorAliases(); @@ -116,7 +130,7 @@ public class ConvertLocalLinks : MigrationBase _logger.LogInformation( "Migration starting for all properties of type: {propertyEditorAlias}", propertyEditorAlias); - if (ProcessPropertyTypes(propertyTypes, languagesById)) + if (ProcessPropertyTypes(propertyEditorAlias, propertyTypes, languagesById)) { _logger.LogInformation( "Migration succeeded for all properties of type: {propertyEditorAlias}", @@ -134,7 +148,7 @@ public class ConvertLocalLinks : MigrationBase RebuildCache = true; } - private bool ProcessPropertyTypes(IPropertyType[] propertyTypes, IDictionary languagesById) + private bool ProcessPropertyTypes(string propertyEditorAlias, IPropertyType[] propertyTypes, IDictionary languagesById) { foreach (IPropertyType propertyType in propertyTypes) { @@ -145,112 +159,157 @@ public class ConvertLocalLinks : MigrationBase ?? throw new InvalidOperationException( "The data type value editor could not be fetched."); - Sql sql = Sql() - .Select() - .From() - .InnerJoin() - .On((propertyData, contentVersion) => - propertyData.VersionId == contentVersion.Id) - .LeftJoin() - .On((contentVersion, documentVersion) => - contentVersion.Id == documentVersion.Id) - .Where( - (propertyData, contentVersion, documentVersion) => - (contentVersion.Current == true || documentVersion.Published == true) - && propertyData.PropertyTypeId == propertyType.Id); - - List propertyDataDtos = Database.Fetch(sql); - if (propertyDataDtos.Count < 1) + long propertyDataCount = Database.ExecuteScalar(BuildPropertyDataSql(propertyType, true)); + if (propertyDataCount == 0) { continue; } - var updateBatch = propertyDataDtos.Select(propertyDataDto => - UpdateBatch.For(propertyDataDto, Database.StartSnapshot(propertyDataDto))).ToList(); + _logger.LogInformation( + "Migrating {PropertyDataCount} property data values for property {PropertyTypeAlias} ({PropertyTypeKey}) with property editor alias {PropertyEditorAlias}", + propertyDataCount, + propertyType.Alias, + propertyType.Key, + propertyEditorAlias); - var updatesToSkip = new ConcurrentBag>(); - - var progress = 0; - - void HandleUpdateBatch(UpdateBatch update) + // Process in pages to avoid loading all property data from the database into memory at once. + Sql sql = BuildPropertyDataSql(propertyType); + const int PageSize = 10000; + long pageNumber = 1; + long pageCount = (propertyDataCount + PageSize - 1) / PageSize; + int processedCount = 0; + while (processedCount < propertyDataCount) { - using UmbracoContextReference umbracoContextReference = _umbracoContextFactory.EnsureUmbracoContext(); - - progress++; - if (progress % 100 == 0) + Page propertyDataDtoPage = Database.Page(pageNumber, PageSize, sql); + if (propertyDataDtoPage.Items.Count == 0) { - _logger.LogInformation(" - finíshed {progress} of {total} properties", progress, - updateBatch.Count); + break; } - PropertyDataDto propertyDataDto = update.Poco; + var updateBatchCollection = propertyDataDtoPage.Items + .Select(propertyDataDto => + UpdateBatch.For(propertyDataDto, Database.StartSnapshot(propertyDataDto))) + .ToList(); - if (ProcessPropertyDataDto(propertyDataDto, propertyType, languagesById, valueEditor) == false) - { - updatesToSkip.Add(update); - } - } + var updatesToSkip = new ConcurrentBag>(); - if (DatabaseType == DatabaseType.SQLite) - { - // SQLite locks up if we run the migration in parallel, so... let's not. - foreach (UpdateBatch update in updateBatch) + var progress = 0; + + void HandleUpdateBatch(UpdateBatch update) { - HandleUpdateBatch(update); - } - } - else - { - Parallel.ForEachAsync(updateBatch, async (update, token) => - { - //Foreach here, but we need to suppress the flow before each task, but not the actuall await of the task - Task task; - using (ExecutionContext.SuppressFlow()) + using UmbracoContextReference umbracoContextReference = _umbracoContextFactory.EnsureUmbracoContext(); + + progress++; + if (progress % 100 == 0) { - task = Task.Run( - () => - { - using ICoreScope scope = _coreScopeProvider.CreateCoreScope(); - scope.Complete(); - HandleUpdateBatch(update); - }, - token); + _logger.LogInformation( + " - finished {Progress} of {PageTotal} properties in page {PageNumber} of {PageCount}", + progress, + updateBatchCollection.Count, + pageNumber, + pageCount); } - await task; - }).GetAwaiter().GetResult(); + PropertyDataDto propertyDataDto = update.Poco; + + if (ProcessPropertyDataDto(propertyDataDto, propertyType, languagesById, valueEditor) == false) + { + updatesToSkip.Add(update); + } + } + + if (DatabaseType == DatabaseType.SQLite) + { + // SQLite locks up if we run the migration in parallel, so... let's not. + foreach (UpdateBatch update in updateBatchCollection) + { + HandleUpdateBatch(update); + } + } + else + { + Parallel.ForEachAsync(updateBatchCollection, async (update, token) => + { + //Foreach here, but we need to suppress the flow before each task, but not the actual await of the task + Task task; + using (ExecutionContext.SuppressFlow()) + { + task = Task.Run( + () => + { + using ICoreScope scope = _coreScopeProvider.CreateCoreScope(); + scope.Complete(); + HandleUpdateBatch(update); + }, + token); + } + + await task; + }).GetAwaiter().GetResult(); + } + + updateBatchCollection.RemoveAll(updatesToSkip.Contains); + + if (updateBatchCollection.Any() is false) + { + _logger.LogDebug(" - no properties to convert, continuing"); + + pageNumber++; + processedCount += propertyDataDtoPage.Items.Count; + + continue; + } + + _logger.LogInformation(" - {totalConverted} properties converted, saving...", updateBatchCollection.Count); + var result = Database.UpdateBatch(updateBatchCollection, new BatchOptions { BatchSize = 100 }); + if (result != updateBatchCollection.Count) + { + throw new InvalidOperationException( + $"The database batch update was supposed to update {updateBatchCollection.Count} property DTO entries, but it updated {result} entries."); + } + + _logger.LogDebug( + "Migration completed for property type: {propertyTypeName} (id: {propertyTypeId}, alias: {propertyTypeAlias}, editor alias: {propertyTypeEditorAlias}) - {updateCount} property DTO entries updated.", + propertyType.Name, + propertyType.Id, + propertyType.Alias, + propertyType.PropertyEditorAlias, + result); + + pageNumber++; + processedCount += propertyDataDtoPage.Items.Count; } - - updateBatch.RemoveAll(updatesToSkip.Contains); - - if (updateBatch.Any() is false) - { - _logger.LogDebug(" - no properties to convert, continuing"); - continue; - } - - _logger.LogInformation(" - {totalConverted} properties converted, saving...", updateBatch.Count); - var result = Database.UpdateBatch(updateBatch, new BatchOptions { BatchSize = 100 }); - if (result != updateBatch.Count) - { - throw new InvalidOperationException( - $"The database batch update was supposed to update {updateBatch.Count} property DTO entries, but it updated {result} entries."); - } - - _logger.LogDebug( - "Migration completed for property type: {propertyTypeName} (id: {propertyTypeId}, alias: {propertyTypeAlias}, editor alias: {propertyTypeEditorAlias}) - {updateCount} property DTO entries updated.", - propertyType.Name, - propertyType.Id, - propertyType.Alias, - propertyType.PropertyEditorAlias, - result); } return true; } - private bool ProcessPropertyDataDto(PropertyDataDto propertyDataDto, IPropertyType propertyType, - IDictionary languagesById, IDataValueEditor valueEditor) + private Sql BuildPropertyDataSql(IPropertyType propertyType, bool isCount = false) + { + Sql sql = isCount + ? Sql().SelectCount() + : Sql().Select(); + + sql = sql.From() + .InnerJoin() + .On((propertyData, contentVersion) => + propertyData.VersionId == contentVersion.Id) + .LeftJoin() + .On((contentVersion, documentVersion) => + contentVersion.Id == documentVersion.Id) + .Where( + (propertyData, contentVersion, documentVersion) => + (contentVersion.Current || documentVersion.Published) + && propertyData.PropertyTypeId == propertyType.Id); + + return sql; + } + + private bool ProcessPropertyDataDto( + PropertyDataDto propertyDataDto, + IPropertyType propertyType, + IDictionary languagesById, + IDataValueEditor valueEditor) { // NOTE: some old property data DTOs can have variance defined, even if the property type no longer varies var culture = propertyType.VariesByCulture()