From 4351ce6ee4b544bb4575d15f9b70c61ffacb6b55 Mon Sep 17 00:00:00 2001 From: Paul Johnson Date: Fri, 25 Feb 2022 08:22:37 +0000 Subject: [PATCH] Further changes requested during review of #12049 (#12053) --- .../Configuration/Models/GlobalSettings.cs | 13 ++++ .../UmbracoBuilder.CoreServices.cs | 2 +- .../Runtime/FileSystemMainDomLock.cs | 70 ++++++------------- .../Runtime/SqlMainDomLock.cs | 2 +- .../Runtime/FileSystemMainDomLockTests.cs | 13 ++-- 5 files changed, 45 insertions(+), 55 deletions(-) diff --git a/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs b/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs index 53584af1d1..31068efd9f 100644 --- a/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs @@ -29,6 +29,7 @@ namespace Umbraco.Cms.Core.Configuration.Models internal const string StaticNoNodesViewPath = "~/umbraco/UmbracoWebsite/NoNodes.cshtml"; internal const string StaticSqlWriteLockTimeOut = "00:00:05"; internal const bool StaticSanitizeTinyMce = false; + internal const int StaticMainDomReleaseSignalPollingInterval = 2000; /// /// Gets or sets a value for the reserved URLs (must end with a comma). @@ -145,6 +146,18 @@ namespace Umbraco.Cms.Core.Configuration.Models /// public string MainDomKeyDiscriminator { get; set; } = string.Empty; + /// + /// Gets or sets the duration (in milliseconds) for which the MainDomLock release signal polling task should sleep. + /// + /// + /// Doesn't apply to MainDomSemaphoreLock. + /// + /// The default value is 2000ms. + /// + /// + [DefaultValue(StaticMainDomReleaseSignalPollingInterval)] + public int MainDomReleaseSignalPollingInterval { get; set; } = StaticMainDomReleaseSignalPollingInterval; + /// /// Gets or sets the telemetry ID. /// diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index 1c8b3ec0de..c4b9a6367c 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -234,7 +234,7 @@ namespace Umbraco.Cms.Infrastructure.DependencyInjection if (globalSettings.Value.MainDomLock == "FileSystemMainDomLock") { - return new FileSystemMainDomLock(loggerFactory.CreateLogger(), mainDomKeyGenerator, hostingEnvironment); + return new FileSystemMainDomLock(loggerFactory.CreateLogger(), mainDomKeyGenerator, hostingEnvironment, factory.GetRequiredService>()); } return globalSettings.Value.MainDomLock.Equals("SqlMainDomLock") || isWindows == false diff --git a/src/Umbraco.Infrastructure/Runtime/FileSystemMainDomLock.cs b/src/Umbraco.Infrastructure/Runtime/FileSystemMainDomLock.cs index f77cb74c3f..32b93513b4 100644 --- a/src/Umbraco.Infrastructure/Runtime/FileSystemMainDomLock.cs +++ b/src/Umbraco.Infrastructure/Runtime/FileSystemMainDomLock.cs @@ -1,10 +1,11 @@ using System; -using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.Runtime; @@ -13,6 +14,7 @@ namespace Umbraco.Cms.Infrastructure.Runtime internal class FileSystemMainDomLock : IMainDomLock { private readonly ILogger _logger; + private readonly IOptionsMonitor _globalSettings; private readonly CancellationTokenSource _cancellationTokenSource = new(); private readonly string _lockFilePath; private readonly string _releaseSignalFilePath; @@ -20,14 +22,14 @@ namespace Umbraco.Cms.Infrastructure.Runtime private FileStream _lockFileStream; private Task _listenForReleaseSignalFileTask; - private const int s_maxTriesRemovingLockReleaseSignalFile = 3; - public FileSystemMainDomLock( ILogger logger, IMainDomKeyGenerator mainDomKeyGenerator, - IHostingEnvironment hostingEnvironment) + IHostingEnvironment hostingEnvironment, + IOptionsMonitor globalSettings) { _logger = logger; + _globalSettings = globalSettings; var lockFileName = $"MainDom_{mainDomKeyGenerator.GenerateKey()}.lock"; _lockFilePath = Path.Combine(hostingEnvironment.LocalTempPath, lockFileName); @@ -45,18 +47,18 @@ namespace Umbraco.Cms.Infrastructure.Runtime { _logger.LogDebug("Attempting to obtain MainDom lock file handle {lockFilePath}", _lockFilePath); _lockFileStream = File.Open(_lockFilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); - DeleteLockReleaseFile(); + DeleteLockReleaseSignalFile(); return Task.FromResult(true); } catch (IOException) { _logger.LogDebug("Couldn't obtain MainDom lock file handle, signalling for release of {lockFilePath}", _lockFilePath); - CreateLockReleaseFile(); - Thread.Sleep(500); + CreateLockReleaseSignalFile(); } catch (Exception ex) { _logger.LogError(ex, "Unexpected exception attempting to obtain MainDom lock file handle {lockFilePath}, giving up", _lockFilePath); + _lockFileStream?.Close(); return Task.FromResult(false); } } @@ -65,6 +67,12 @@ namespace Umbraco.Cms.Infrastructure.Runtime return Task.FromResult(false); } + public void CreateLockReleaseSignalFile() => + _ = File.Open(_releaseSignalFilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.ReadWrite | FileShare.Delete); + + public void DeleteLockReleaseSignalFile() => + File.Delete(_releaseSignalFilePath); + // Create a long running task to poll to check if anyone has created a lock release file. public Task ListenAsync() { @@ -82,46 +90,6 @@ namespace Umbraco.Cms.Infrastructure.Runtime return _listenForReleaseSignalFileTask; } - public void Dispose() - { - _lockFileStream?.Close(); - _lockFileStream = null; - } - - private void CreateLockReleaseFile() - { - try - { - // Dispose immediately to release the file handle so it's easier to cleanup in any process. - using FileStream releaseFileStream = File.Open(_releaseSignalFilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.ReadWrite); - } - catch (Exception ex) - { - _logger.LogError(ex, "Unexpected exception attempting to create lock release signal file {file}", _releaseSignalFilePath); - } - } - - private void DeleteLockReleaseFile() - { - List encounteredExceptions = new(); - for (var i = 0; i < s_maxTriesRemovingLockReleaseSignalFile; i++) - { - try - { - File.Delete(_releaseSignalFilePath); - return; - } - catch (Exception ex) - { - _logger.LogError(ex, "Unexpected exception attempting to delete release signal file {file}", _releaseSignalFilePath); - encounteredExceptions.Add(ex); - Thread.Sleep(500 * (i + 1)); - } - } - - throw new ApplicationException($"Failed to remove lock release signal file {_releaseSignalFilePath}", new AggregateException(encounteredExceptions)); - } - private void ListeningLoop() { while (true) @@ -140,8 +108,14 @@ namespace Umbraco.Cms.Infrastructure.Runtime break; } - Thread.Sleep(2000); + Thread.Sleep(_globalSettings.CurrentValue.MainDomReleaseSignalPollingInterval); } } + + public void Dispose() + { + _lockFileStream?.Close(); + _lockFileStream = null; + } } } diff --git a/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs b/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs index 635b0b9c28..8a6698b92a 100644 --- a/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs +++ b/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs @@ -236,7 +236,7 @@ namespace Umbraco.Cms.Infrastructure.Runtime { // poll every couple of seconds // local testing shows the actual query to be executed from client/server is approx 300ms but would change depending on environment/IO - Thread.Sleep(2000); + Thread.Sleep(_globalSettings.Value.MainDomReleaseSignalPollingInterval); if (!_dbFactory.Configured) { diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Runtime/FileSystemMainDomLockTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Runtime/FileSystemMainDomLockTests.cs index 59442a683a..c709a756b8 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Runtime/FileSystemMainDomLockTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Runtime/FileSystemMainDomLockTests.cs @@ -1,7 +1,10 @@ using System.IO; using System.Threading.Tasks; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Moq; using NUnit.Framework; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.Runtime; using Umbraco.Cms.Infrastructure.Runtime; @@ -31,8 +34,11 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Runtime LockFilePath = Path.Combine(HostingEnvironment.LocalTempPath, lockFileName); LockReleaseFilePath = LockFilePath + "_release"; + var globalSettings = Mock.Of>(); + Mock.Get(globalSettings).Setup(x => x.CurrentValue).Returns(new GlobalSettings()); + var log = GetRequiredService>(); - FileSystemMainDomLock = new FileSystemMainDomLock(log, MainDomKeyGenerator, HostingEnvironment); + FileSystemMainDomLock = new FileSystemMainDomLock(log, MainDomKeyGenerator, HostingEnvironment, globalSettings); } [TearDown] @@ -79,10 +85,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Runtime var before = await sut.AcquireLockAsync(1000); - await using (_ = File.Open(LockReleaseFilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.ReadWrite)) - { - } - + sut.CreateLockReleaseSignalFile(); await sut.ListenAsync(); var after = await sut.AcquireLockAsync(1000);