Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not migrate blocks in parallel on SQLite #17416

Merged
merged 1 commit into from
Nov 4, 2024
Merged

Do not migrate blocks in parallel on SQLite #17416

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 34 additions & 19 deletions src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_0_0/ConvertBlockEditorPropertiesBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Collections.Concurrent;

Check warning on line 1 in src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_0_0/ConvertBlockEditorPropertiesBase.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (release/15.0)

❌ New issue: Overall Code Complexity 

This module has a mean cyclomatic complexity of 5.86 across 7 functions. The mean complexity threshold is 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.
using Microsoft.Extensions.Logging;
using NPoco;
using Umbraco.Cms.Core;
Expand Down Expand Up @@ -144,123 +144,138 @@

var progress = 0;

Parallel.ForEachAsync(updateBatch, async (update, token) =>
void HandleUpdateBatch(UpdateBatch<PropertyDataDto> update)
{
//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())
{
task = Task.Run(() =>
{
using ICoreScope scope = _coreScopeProvider.CreateCoreScope();
scope.Complete();
using UmbracoContextReference umbracoContextReference =
_umbracoContextFactory.EnsureUmbracoContext();
using UmbracoContextReference umbracoContextReference = _umbracoContextFactory.EnsureUmbracoContext();

progress++;
if (progress % 100 == 0)
{
_logger.LogInformation(" - finíshed {progress} of {total} properties", progress,
updateBatch.Count);
_logger.LogInformation(" - finíshed {progress} of {total} properties", progress, updateBatch.Count);
}

PropertyDataDto propertyDataDto = update.Poco;

// NOTE: some old property data DTOs can have variance defined, even if the property type no longer varies
var culture = propertyType.VariesByCulture()
&& propertyDataDto.LanguageId.HasValue
&& languagesById.TryGetValue(propertyDataDto.LanguageId.Value,
out ILanguage? language)
&& languagesById.TryGetValue(propertyDataDto.LanguageId.Value, out ILanguage? language)
? language.IsoCode
: null;

if (culture is null && propertyType.VariesByCulture())
{
// if we end up here, the property DTO is bound to a language that no longer exists. this is an error scenario,
// and we can't really handle it in any other way than logging; in all likelihood this is an old property version,
// and it won't cause any runtime issues
_logger.LogWarning(
" - property data with id: {propertyDataId} references a language that does not exist - language id: {languageId} (property type: {propertyTypeName}, id: {propertyTypeId}, alias: {propertyTypeAlias})",
propertyDataDto.Id,
propertyDataDto.LanguageId,
propertyType.Name,
propertyType.Id,
propertyType.Alias);
return;
}

var segment = propertyType.VariesBySegment() ? propertyDataDto.Segment : null;
var property = new Property(propertyType);
property.SetValue(propertyDataDto.Value, culture, segment);
var toEditorValue = valueEditor.ToEditor(property, culture, segment);
switch (toEditorValue)
{
case null:
_logger.LogWarning(
" - value editor yielded a null value for property data with id: {propertyDataId} (property type: {propertyTypeName}, id: {propertyTypeId}, alias: {propertyTypeAlias})",
propertyDataDto.Id,
propertyType.Name,
propertyType.Id,
propertyType.Alias);
updatesToSkip.Add(update);
return;

case string str when str.IsNullOrWhiteSpace():
// indicates either an empty block editor or corrupt block editor data - we can't do anything about either here
updatesToSkip.Add(update);
return;

default:
switch (DetermineEditorValueHandling(toEditorValue))
{
case EditorValueHandling.IgnoreConversion:
// nothing to convert, continue
updatesToSkip.Add(update);
return;
case EditorValueHandling.ProceedConversion:
// continue the conversion
break;
case EditorValueHandling.HandleAsError:
_logger.LogError(
" - value editor did not yield a valid ToEditor value for property data with id: {propertyDataId} - the value type was {valueType} (property type: {propertyTypeName}, id: {propertyTypeId}, alias: {propertyTypeAlias})",
propertyDataDto.Id,
toEditorValue.GetType(),
propertyType.Name,
propertyType.Id,
propertyType.Alias);
updatesToSkip.Add(update);
return;
default:
throw new ArgumentOutOfRangeException();
}

break;
}

toEditorValue = UpdateEditorValue(toEditorValue);

var editorValue = _jsonSerializer.Serialize(toEditorValue);
var dbValue = valueEditor.FromEditor(new ContentPropertyData(editorValue, null), null);
if (dbValue is not string stringValue || stringValue.DetectIsJson() is false)
{
_logger.LogError(
" - value editor did not yield a valid JSON string as FromEditor value property data with id: {propertyDataId} (property type: {propertyTypeName}, id: {propertyTypeId}, alias: {propertyTypeAlias})",
propertyDataDto.Id,
propertyType.Name,
propertyType.Id,
propertyType.Alias);
updatesToSkip.Add(update);
return;
}

stringValue = UpdateDatabaseValue(stringValue);

propertyDataDto.TextValue = stringValue;
}, token);
}
}

Check notice on line 248 in src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_0_0/ConvertBlockEditorPropertiesBase.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (release/15.0)

ℹ New issue: Complex Method 

Handle.HandleUpdateBatch has a cyclomatic complexity of 17, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

await task;
}).GetAwaiter().GetResult();
if (DatabaseType == DatabaseType.SQLite)
{
// SQLite locks up if we run the migration in parallel, so... let's not.
foreach (UpdateBatch<PropertyDataDto> update in updateBatch)
{
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())
{
task = Task.Run(
() =>
{
using ICoreScope scope = _coreScopeProvider.CreateCoreScope();
scope.Complete();
HandleUpdateBatch(update);
},
token);
}

await task;
}).GetAwaiter().GetResult();
}

Check notice on line 278 in src/Umbraco.Infrastructure/Migrations/Upgrade/V_15_0_0/ConvertBlockEditorPropertiesBase.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (release/15.0)

✅ Getting better: Complex Method 

Handle decreases in cyclomatic complexity from 28 to 14, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

updateBatch.RemoveAll(updatesToSkip.Contains);

Expand Down
Loading