yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #6688 from umbraco/v8/bugfix/AB3172-long-running-ops

Browse Source 
Long running operations causes SQL timeout
This commit is contained in:
Bjarke Berg
2019-10-21 07:56:53 +02:00
committed by GitHub
parent 16abdcac46 debe4b381c
commit 9ffb3bed57
11 changed files with 106 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
src/Umbraco.Core/Persistence/DatabasenodeLockExtensions.cs
View File

@@ -1,43 +0,0 @@
using System;
using System.Collections.Generic;
using System.Data;
using System.Runtime.CompilerServices;
namespace Umbraco.Core.Persistence
{
internal static class DatabaseNodeLockExtensions
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void ValidateDatabase(IUmbracoDatabase database)
{
if (database == null)
throw new ArgumentNullException("database");
if (database.GetCurrentTransactionIsolationLevel() < IsolationLevel.RepeatableRead)
throw new InvalidOperationException("A transaction with minimum RepeatableRead isolation level is required.");
}
// updating a record within a repeatable-read transaction gets an exclusive lock on
// that record which will be kept until the transaction is ended, effectively locking
// out all other accesses to that record - thus obtaining an exclusive lock over the
// protected resources.
public static void AcquireLockNodeWriteLock(this IUmbracoDatabase database, int nodeId)
{
ValidateDatabase(database);
database.Execute("UPDATE umbracoLock SET value = (CASE WHEN (value=1) THEN -1 ELSE 1 END) WHERE id=@id",
new { @id = nodeId });
}
// reading a record within a repeatable-read transaction gets a shared lock on
// that record which will be kept until the transaction is ended, effectively preventing
// other write accesses to that record - thus obtaining a shared lock over the protected
// resources.
public static void AcquireLockNodeReadLock(this IUmbracoDatabase database, int nodeId)
{
ValidateDatabase(database);
database.ExecuteScalar<int>("SELECT value FROM umbracoLock WHERE id=@id",
new { @id = nodeId });
}
}
}

9
src/Umbraco.Core/Persistence/SqlSyntax/ISqlSyntaxProvider.cs
View File

@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Data;
using System.Text.RegularExpressions;
using NPoco;
using Umbraco.Core.Persistence.DatabaseAnnotations;
@@ -76,6 +77,11 @@ namespace Umbraco.Core.Persistence.SqlSyntax
string ConvertIntegerToOrderableString { get; }
string ConvertDateToOrderableString { get; }
string ConvertDecimalToOrderableString { get; }
/// <summary>
/// Returns the default isolation level for the database
/// </summary>
IsolationLevel DefaultIsolationLevel { get; }
IEnumerable<string> GetTablesInSchema(IDatabase db);
IEnumerable<ColumnInfo> GetColumnsInSchema(IDatabase db);
@@ -121,5 +127,8 @@ namespace Umbraco.Core.Persistence.SqlSyntax
/// unspecified.</para>
/// </remarks>
bool TryGetDefaultConstraint(IDatabase db, string tableName, string columnName, out string constraintName);
void ReadLock(IDatabase db, params int[] lockIds);
void WriteLock(IDatabase db, params int[] lockIds);
}
}

37
src/Umbraco.Core/Persistence/SqlSyntax/SqlCeSyntaxProvider.cs
View File

@@ -1,5 +1,7 @@
using System;
using System.Collections.Generic;
using System.Data;
using System.Data.SqlServerCe;
using System.Linq;
using NPoco;
using Umbraco.Core.Persistence.DatabaseAnnotations;
@@ -52,6 +54,8 @@ namespace Umbraco.Core.Persistence.SqlSyntax
return "(" + string.Join("+", args) + ")";
}
public override System.Data.IsolationLevel DefaultIsolationLevel => System.Data.IsolationLevel.RepeatableRead;
public override string FormatColumnRename(string tableName, string oldName, string newName)
{
//NOTE Sql CE doesn't support renaming a column, so a new column needs to be created, then copy data and finally remove old column
@@ -152,6 +156,39 @@ where table_name=@0 and column_name=@1", tableName, columnName).FirstOrDefault()
return result > 0;
}
public override void WriteLock(IDatabase db, params int[] lockIds)
{
// soon as we get Database, a transaction is started
if (db.Transaction.IsolationLevel < IsolationLevel.RepeatableRead)
throw new InvalidOperationException("A transaction with minimum RepeatableRead isolation level is required.");
db.Execute(@"SET LOCK_TIMEOUT 1800;");
// *not* using a unique 'WHERE IN' query here because the *order* of lockIds is important to avoid deadlocks
foreach (var lockId in lockIds)
{
var i = db.Execute(@"UPDATE umbracoLock SET value = (CASE WHEN (value=1) THEN -1 ELSE 1 END) WHERE id=@id", new { id = lockId });
if (i == 0) // ensure we are actually locking!
throw new ArgumentException($"LockObject with id={lockId} does not exist.");
}
}
public override void ReadLock(IDatabase db, params int[] lockIds)
{
// soon as we get Database, a transaction is started
if (db.Transaction.IsolationLevel < IsolationLevel.RepeatableRead)
throw new InvalidOperationException("A transaction with minimum RepeatableRead isolation level is required.");
// *not* using a unique 'WHERE IN' query here because the *order* of lockIds is important to avoid deadlocks
foreach (var lockId in lockIds)
{
var i = db.ExecuteScalar<int?>("SELECT value FROM umbracoLock WHERE id=@id", new { id = lockId });
if (i == null) // ensure we are actually locking!
throw new ArgumentException($"LockObject with id={lockId} does not exist.");
}
}
protected override string FormatIdentity(ColumnDefinition column)
{
return column.IsIdentity ? GetIdentityString(column) : string.Empty;

38
src/Umbraco.Core/Persistence/SqlSyntax/SqlServerSyntaxProvider.cs
View File

@@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.Data;
using System.Data.Common;
using System.Data.SqlClient;
using System.Linq;
using NPoco;
using Umbraco.Core.Logging;
@@ -179,6 +180,8 @@ namespace Umbraco.Core.Persistence.SqlSyntax
return items.Select(x => x.TABLE_NAME).Cast<string>().ToList();
}
public override IsolationLevel DefaultIsolationLevel => IsolationLevel.ReadCommitted;
public override IEnumerable<ColumnInfo> GetColumnsInSchema(IDatabase db)
{
var items = db.Fetch<dynamic>("SELECT TABLE_NAME, COLUMN_NAME, ORDINAL_POSITION, COLUMN_DEFAULT, IS_NULLABLE, DATA_TYPE FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA = (SELECT SCHEMA_NAME())");
@@ -246,6 +249,41 @@ where tbl.[name]=@0 and col.[name]=@1;", tableName, columnName)
return result > 0;
}
public override void WriteLock(IDatabase db, params int[] lockIds)
{
// soon as we get Database, a transaction is started
if (db.Transaction.IsolationLevel < IsolationLevel.ReadCommitted)
throw new InvalidOperationException("A transaction with minimum ReadCommitted isolation level is required.");
// *not* using a unique 'WHERE IN' query here because the *order* of lockIds is important to avoid deadlocks
foreach (var lockId in lockIds)
{
db.Execute(@"SET LOCK_TIMEOUT 1800;");
var i = db.Execute(@"UPDATE umbracoLock WITH (REPEATABLEREAD) SET value = (CASE WHEN (value=1) THEN -1 ELSE 1 END) WHERE id=@id", new { id = lockId });
if (i == 0) // ensure we are actually locking!
throw new ArgumentException($"LockObject with id={lockId} does not exist.");
}
}
public override void ReadLock(IDatabase db, params int[] lockIds)
{
// soon as we get Database, a transaction is started
if (db.Transaction.IsolationLevel < IsolationLevel.ReadCommitted)
throw new InvalidOperationException("A transaction with minimum ReadCommitted isolation level is required.");
// *not* using a unique 'WHERE IN' query here because the *order* of lockIds is important to avoid deadlocks
foreach (var lockId in lockIds)
{
var i = db.ExecuteScalar<int?>("SELECT value FROM umbracoLock WITH (REPEATABLEREAD) WHERE id=@id", new { id = lockId });
if (i == null) // ensure we are actually locking!
throw new ArgumentException($"LockObject with id={lockId} does not exist.", nameof(lockIds));
}
}
public override string FormatColumnRename(string tableName, string oldName, string newName)
{
return string.Format(RenameColumn, tableName, oldName, newName);

7
src/Umbraco.Core/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs
View File

@@ -200,7 +200,9 @@ namespace Umbraco.Core.Persistence.SqlSyntax
return "NVARCHAR";
}
public abstract IsolationLevel DefaultIsolationLevel { get; }
public virtual IEnumerable<string> GetTablesInSchema(IDatabase db)
{
return new List<string>();
@@ -225,6 +227,9 @@ namespace Umbraco.Core.Persistence.SqlSyntax
public abstract bool TryGetDefaultConstraint(IDatabase db, string tableName, string columnName, out string constraintName);
public abstract void ReadLock(IDatabase db, params int[] lockIds);
public abstract void WriteLock(IDatabase db, params int[] lockIds);
public virtual bool DoesTableExist(IDatabase db, string tableName)
{
return false;

7
src/Umbraco.Core/Persistence/UmbracoDatabase.cs
View File

@@ -20,9 +20,6 @@ namespace Umbraco.Core.Persistence
/// </remarks>
public class UmbracoDatabase : Database, IUmbracoDatabase
{
// Umbraco's default isolation level is RepeatableRead
private const IsolationLevel DefaultIsolationLevel = IsolationLevel.RepeatableRead;
private readonly ILogger _logger;
private readonly RetryPolicy _connectionRetryPolicy;
private readonly RetryPolicy _commandRetryPolicy;
@@ -38,7 +35,7 @@ namespace Umbraco.Core.Persistence
/// <para>Also used by DatabaseBuilder for creating databases and installing/upgrading.</para>
/// </remarks>
public UmbracoDatabase(string connectionString, ISqlContext sqlContext, DbProviderFactory provider, ILogger logger, RetryPolicy connectionRetryPolicy = null, RetryPolicy commandRetryPolicy = null)
: base(connectionString, sqlContext.DatabaseType, provider, DefaultIsolationLevel)
: base(connectionString, sqlContext.DatabaseType, provider, sqlContext.SqlSyntax.DefaultIsolationLevel)
{
SqlContext = sqlContext;
@@ -54,7 +51,7 @@ namespace Umbraco.Core.Persistence
/// </summary>
/// <remarks>Internal for unit tests only.</remarks>
internal UmbracoDatabase(DbConnection connection, ISqlContext sqlContext, ILogger logger)
: base(connection, sqlContext.DatabaseType, DefaultIsolationLevel)
: base(connection, sqlContext.DatabaseType, sqlContext.SqlSyntax.DefaultIsolationLevel)
{
SqlContext = sqlContext;
_logger = logger;

36
src/Umbraco.Core/Scoping/Scope.cs
View File

@@ -33,8 +33,6 @@ namespace Umbraco.Core.Scoping
private ICompletable _fscope;
private IEventDispatcher _eventDispatcher;
private const IsolationLevel DefaultIsolationLevel = IsolationLevel.RepeatableRead;
// initializes a new scope
private Scope(ScopeProvider scopeProvider,
ILogger logger, FileSystems fileSystems, Scope parent, ScopeContext scopeContext, bool detachable,
@@ -205,7 +203,7 @@ namespace Umbraco.Core.Scoping
{
if (_isolationLevel != IsolationLevel.Unspecified) return _isolationLevel;
if (ParentScope != null) return ParentScope.IsolationLevel;
return DefaultIsolationLevel;
return Database.SqlContext.SqlSyntax.DefaultIsolationLevel;
}
}
@@ -488,37 +486,9 @@ namespace Umbraco.Core.Scoping
?? (_logUncompletedScopes = Current.Configs.CoreDebug().LogUncompletedScopes)).Value;
/// <inheritdoc />
public void ReadLock(params int[] lockIds)
{
// soon as we get Database, a transaction is started
if (Database.Transaction.IsolationLevel < IsolationLevel.RepeatableRead)
throw new InvalidOperationException("A transaction with minimum RepeatableRead isolation level is required.");
// *not* using a unique 'WHERE IN' query here because the *order* of lockIds is important to avoid deadlocks
foreach (var lockId in lockIds)
{
var i = Database.ExecuteScalar<int?>("SELECT value FROM umbracoLock WHERE id=@id", new { id = lockId });
if (i == null) // ensure we are actually locking!
throw new Exception($"LockObject with id={lockId} does not exist.");
}
}
public void ReadLock(params int[] lockIds) => Database.SqlContext.SqlSyntax.ReadLock(Database, lockIds);
/// <inheritdoc />
public void WriteLock(params int[] lockIds)
{
// soon as we get Database, a transaction is started
if (Database.Transaction.IsolationLevel < IsolationLevel.RepeatableRead)
throw new InvalidOperationException("A transaction with minimum RepeatableRead isolation level is required.");
// *not* using a unique 'WHERE IN' query here because the *order* of lockIds is important to avoid deadlocks
foreach (var lockId in lockIds)
{
var i = Database.Execute("UPDATE umbracoLock SET value = (CASE WHEN (value=1) THEN -1 ELSE 1 END) WHERE id=@id", new { id = lockId });
if (i == 0) // ensure we are actually locking!
throw new Exception($"LockObject with id={lockId} does not exist.");
}
}
public void WriteLock(params int[] lockIds) => Database.SqlContext.SqlSyntax.WriteLock(Database, lockIds);
}
}

1
src/Umbraco.Core/Umbraco.Core.csproj
View File

@@ -984,7 +984,6 @@
<Compile Include="Persistence\DatabaseModelDefinitions\ModificationType.cs" />
<Compile Include="Persistence\DatabaseModelDefinitions\SystemMethods.cs" />
<Compile Include="Persistence\DatabaseModelDefinitions\TableDefinition.cs" />
<Compile Include="Persistence\DatabaseNodeLockExtensions.cs" />
<Compile Include="Persistence\DbCommandExtensions.cs" />
<Compile Include="Persistence\DbConnectionExtensions.cs" />
<Compile Include="Persistence\EntityNotFoundException.cs" />

3
src/Umbraco.Tests/Models/UserExtensionsTests.cs
View File

@@ -57,6 +57,9 @@ namespace Umbraco.Tests.Models
[TestCase("1,-1", "1", "1")] // was an issue
[TestCase("-1,1", "1", "1")] // was an issue
[TestCase("-1", "", "-1")]
[TestCase("", "-1", "-1")]
public void CombineStartNodes(string groupSn, string userSn, string expected)
{
// 1

13
src/Umbraco.Tests/Persistence/LocksTests.cs
View File

@@ -5,7 +5,6 @@ using System.Threading;
using NPoco;
using NUnit.Framework;
using Umbraco.Core;
using Umbraco.Core.Persistence;
using Umbraco.Core.Persistence.Dtos;
using Umbraco.Tests.TestHelpers;
using Umbraco.Tests.Testing;
@@ -37,7 +36,7 @@ namespace Umbraco.Tests.Persistence
{
using (var scope = ScopeProvider.CreateScope())
{
scope.Database.AcquireLockNodeReadLock(Constants.Locks.Servers);
scope.ReadLock(Constants.Locks.Servers);
scope.Complete();
}
}
@@ -62,7 +61,7 @@ namespace Umbraco.Tests.Persistence
{
try
{
scope.Database.AcquireLockNodeReadLock(Constants.Locks.Servers);
scope.ReadLock(Constants.Locks.Servers);
lock (locker)
{
acquired++;
@@ -131,7 +130,7 @@ namespace Umbraco.Tests.Persistence
if (entered == threadCount) m1.Set();
}
ms[ic].WaitOne();
scope.Database.AcquireLockNodeWriteLock(Constants.Locks.Servers);
scope.WriteLock(Constants.Locks.Servers);
lock (locker)
{
acquired++;
@@ -221,7 +220,7 @@ namespace Umbraco.Tests.Persistence
{
otherEv.WaitOne();
Console.WriteLine($"[{id1}] WAIT {id1}");
scope.Database.AcquireLockNodeWriteLock(id1);
scope.WriteLock(id1);
Console.WriteLine($"[{id1}] GRANT {id1}");
WriteLocks(scope.Database);
myEv.Set();
@@ -232,7 +231,7 @@ namespace Umbraco.Tests.Persistence
Thread.Sleep(200); // cannot wait due to deadlock... just give it a bit of time
Console.WriteLine($"[{id1}] WAIT {id2}");
scope.Database.AcquireLockNodeWriteLock(id2);
scope.WriteLock(id2);
Console.WriteLine($"[{id1}] GRANT {id2}");
WriteLocks(scope.Database);
}
@@ -284,7 +283,7 @@ namespace Umbraco.Tests.Persistence
{
otherEv.WaitOne();
Console.WriteLine($"[{id}] WAIT {id}");
scope.Database.AcquireLockNodeWriteLock(id);
scope.WriteLock(id);
Console.WriteLine($"[{id}] GRANT {id}");
WriteLocks(scope.Database);
myEv.Set();

4
src/Umbraco.Tests/Persistence/UnitOfWorkTests.cs
View File

@@ -14,7 +14,7 @@ namespace Umbraco.Tests.Persistence
public void ReadLockNonExisting()
{
var provider = TestObjects.GetScopeProvider(Logger);
Assert.Throws<Exception>(() =>
Assert.Throws<ArgumentException>(() =>
{
using (var scope = provider.CreateScope())
{
@@ -39,7 +39,7 @@ namespace Umbraco.Tests.Persistence
public void WriteLockNonExisting()
{
var provider = TestObjects.GetScopeProvider(Logger);
Assert.Throws<Exception>(() =>
Assert.Throws<ArgumentException>(() =>
{
using (var scope = provider.CreateScope())
{