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>

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;
 
+/// <summary>
+/// Migrates local links in content and media properties from the legacy format using UDIs
+/// to the new one with GUIDs.
+/// </summary>
+/// <remarks>
+/// See: https://github.com/umbraco/Umbraco-CMS/pull/17307.
+/// </remarks>
 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")]
+    /// <summary>
+    /// Initializes a new instance of the <see cref="ConvertLocalLinks"/> class.
+    /// </summary>
     public ConvertLocalLinks(
         IMigrationContext context,
         IUmbracoContextFactory umbracoContextFactory,
@@ -57,6 +66,10 @@ public class ConvertLocalLinks : MigrationBase
         _linkMigrationTracker = linkMigrationTracker;
     }
 
+    /// <summary>
+    /// Initializes a new instance of the <see cref="ConvertLocalLinks"/> class.
+    /// </summary>
+    [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
     {
     }
 
+    /// <inheritdoc/>
     protected override void Migrate()
     {
         IEnumerable<string> 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<int, ILanguage> languagesById)
+    private bool ProcessPropertyTypes(string propertyEditorAlias, IPropertyType[] propertyTypes, IDictionary<int, ILanguage> languagesById)
     {
         foreach (IPropertyType propertyType in propertyTypes)
         {
@@ -145,28 +159,37 @@ public class ConvertLocalLinks : MigrationBase
                                            ?? throw new InvalidOperationException(
                                                "The data type value editor could not be fetched.");
 
-            Sql<ISqlContext> sql = Sql()
+            long propertyDataCount = Database.ExecuteScalar<long>(BuildPropertyDataSql(propertyType, true));
-                .Select<PropertyDataDto>()
+            if (propertyDataCount == 0)
-                .From<PropertyDataDto>()
-                .InnerJoin<ContentVersionDto>()
-                .On<PropertyDataDto, ContentVersionDto>((propertyData, contentVersion) =>
-                    propertyData.VersionId == contentVersion.Id)
-                .LeftJoin<DocumentVersionDto>()
-                .On<ContentVersionDto, DocumentVersionDto>((contentVersion, documentVersion) =>
-                    contentVersion.Id == documentVersion.Id)
-                .Where<PropertyDataDto, ContentVersionDto, DocumentVersionDto>(
-                    (propertyData, contentVersion, documentVersion) =>
-                        (contentVersion.Current == true || documentVersion.Published == true)
-                        && propertyData.PropertyTypeId == propertyType.Id);
-
-            List<PropertyDataDto> propertyDataDtos = Database.Fetch<PropertyDataDto>(sql);
-            if (propertyDataDtos.Count < 1)
             {
                 continue;
             }
 
-            var updateBatch = propertyDataDtos.Select(propertyDataDto =>
+            _logger.LogInformation(
-                UpdateBatch.For(propertyDataDto, Database.StartSnapshot(propertyDataDto))).ToList();
+                "Migrating {PropertyDataCount} property data values for property {PropertyTypeAlias} ({PropertyTypeKey}) with property editor alias {PropertyEditorAlias}",
+                propertyDataCount,
+                propertyType.Alias,
+                propertyType.Key,
+                propertyEditorAlias);
+
+            // Process in pages to avoid loading all property data from the database into memory at once.
+            Sql<ISqlContext> sql = BuildPropertyDataSql(propertyType);
+            const int PageSize = 10000;
+            long pageNumber = 1;
+            long pageCount = (propertyDataCount + PageSize - 1) / PageSize;
+            int processedCount = 0;
+            while (processedCount < propertyDataCount)
+            {
+                Page<PropertyDataDto> propertyDataDtoPage = Database.Page<PropertyDataDto>(pageNumber, PageSize, sql);
+                if (propertyDataDtoPage.Items.Count == 0)
+                {
+                    break;
+                }
+
+                var updateBatchCollection = propertyDataDtoPage.Items
+                    .Select(propertyDataDto =>
+                        UpdateBatch.For(propertyDataDto, Database.StartSnapshot(propertyDataDto)))
+                    .ToList();
 
                 var updatesToSkip = new ConcurrentBag<UpdateBatch<PropertyDataDto>>();
 
@@ -179,8 +202,12 @@ public class ConvertLocalLinks : MigrationBase
                     progress++;
                     if (progress % 100 == 0)
                     {
-                    _logger.LogInformation("  - finíshed {progress} of {total} properties", progress,
+                        _logger.LogInformation(
-                        updateBatch.Count);
+                            "  - finished {Progress} of {PageTotal} properties in page {PageNumber} of {PageCount}",
+                            progress,
+                            updateBatchCollection.Count,
+                            pageNumber,
+                            pageCount);
                     }
 
                     PropertyDataDto propertyDataDto = update.Poco;
@@ -194,16 +221,16 @@ public class ConvertLocalLinks : MigrationBase
                 if (DatabaseType == DatabaseType.SQLite)
                 {
                     // SQLite locks up if we run the migration in parallel, so... let's not.
-                foreach (UpdateBatch<PropertyDataDto> update in updateBatch)
+                    foreach (UpdateBatch<PropertyDataDto> update in updateBatchCollection)
                     {
                         HandleUpdateBatch(update);
                     }
                 }
                 else
                 {
-                Parallel.ForEachAsync(updateBatch, async (update, token) =>
+                    Parallel.ForEachAsync(updateBatchCollection, async (update, token) =>
                     {
-                    //Foreach here, but we need to suppress the flow before each task, but not the actuall await of the task
+                        //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())
                         {
@@ -221,20 +248,24 @@ public class ConvertLocalLinks : MigrationBase
                     }).GetAwaiter().GetResult();
                 }
 
-            updateBatch.RemoveAll(updatesToSkip.Contains);
+                updateBatchCollection.RemoveAll(updatesToSkip.Contains);
 
-            if (updateBatch.Any() is false)
+                if (updateBatchCollection.Any() is false)
                 {
                     _logger.LogDebug("  - no properties to convert, continuing");
+
+                    pageNumber++;
+                    processedCount += propertyDataDtoPage.Items.Count;
+
                     continue;
                 }
 
-            _logger.LogInformation("  - {totalConverted} properties converted, saving...", updateBatch.Count);
+                _logger.LogInformation("  - {totalConverted} properties converted, saving...", updateBatchCollection.Count);
-            var result = Database.UpdateBatch(updateBatch, new BatchOptions { BatchSize = 100 });
+                var result = Database.UpdateBatch(updateBatchCollection, new BatchOptions { BatchSize = 100 });
-            if (result != updateBatch.Count)
+                if (result != updateBatchCollection.Count)
                 {
                     throw new InvalidOperationException(
-                    $"The database batch update was supposed to update {updateBatch.Count} property DTO entries, but it updated {result} entries.");
+                        $"The database batch update was supposed to update {updateBatchCollection.Count} property DTO entries, but it updated {result} entries.");
                 }
 
                 _logger.LogDebug(
@@ -244,13 +275,41 @@ public class ConvertLocalLinks : MigrationBase
                     propertyType.Alias,
                     propertyType.PropertyEditorAlias,
                     result);
+
+                pageNumber++;
+                processedCount += propertyDataDtoPage.Items.Count;
+            }
         }
 
         return true;
     }
 
-    private bool ProcessPropertyDataDto(PropertyDataDto propertyDataDto, IPropertyType propertyType,
+    private Sql<ISqlContext> BuildPropertyDataSql(IPropertyType propertyType, bool isCount = false)
-        IDictionary<int, ILanguage> languagesById, IDataValueEditor valueEditor)
+    {
+        Sql<ISqlContext> sql = isCount
+            ? Sql().SelectCount()
+            : Sql().Select<PropertyDataDto>();
+
+        sql = sql.From<PropertyDataDto>()
+            .InnerJoin<ContentVersionDto>()
+            .On<PropertyDataDto, ContentVersionDto>((propertyData, contentVersion) =>
+                propertyData.VersionId == contentVersion.Id)
+            .LeftJoin<DocumentVersionDto>()
+            .On<ContentVersionDto, DocumentVersionDto>((contentVersion, documentVersion) =>
+                contentVersion.Id == documentVersion.Id)
+            .Where<PropertyDataDto, ContentVersionDto, DocumentVersionDto>(
+                (propertyData, contentVersion, documentVersion) =>
+                    (contentVersion.Current || documentVersion.Published)
+                    && propertyData.PropertyTypeId == propertyType.Id);
+
+        return sql;
+    }
+
+    private bool ProcessPropertyDataDto(
+        PropertyDataDto propertyDataDto,
+        IPropertyType propertyType,
+        IDictionary<int, ILanguage> 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()