From 32efbc19ccbd6c308ea95dad997bc3e147cc92f4 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Thu, 3 Mar 2011 14:19:02 -0800 Subject: [PATCH] Using lock files instead of IndexSynLock --HG-- branch : indexing --- .../Indexing/IndexingTaskExecutorTests.cs | 37 ++++++++++++----- .../Lucene/Services/LuceneIndexProvider.cs | 8 ++-- .../Orchard.Indexing/Orchard.Indexing.csproj | 1 - .../Orchard.Indexing/Services/IndexSynLock.cs | 25 ------------ .../Services/IndexingTaskExecutor.cs | 40 +++++++++++++------ 5 files changed, 59 insertions(+), 52 deletions(-) delete mode 100644 src/Orchard.Web/Modules/Orchard.Indexing/Services/IndexSynLock.cs diff --git a/src/Orchard.Tests.Modules/Indexing/IndexingTaskExecutorTests.cs b/src/Orchard.Tests.Modules/Indexing/IndexingTaskExecutorTests.cs index 664b10f06..2c7b5bbc7 100644 --- a/src/Orchard.Tests.Modules/Indexing/IndexingTaskExecutorTests.cs +++ b/src/Orchard.Tests.Modules/Indexing/IndexingTaskExecutorTests.cs @@ -19,12 +19,14 @@ using Orchard.Environment; using Orchard.Environment.Configuration; using Orchard.Environment.Extensions; using Orchard.FileSystems.AppData; +using Orchard.FileSystems.LockFile; using Orchard.Indexing; using Orchard.Indexing.Handlers; using Orchard.Indexing.Models; using Orchard.Indexing.Services; using Orchard.Logging; using Orchard.Security; +using Orchard.Services; using Orchard.Tasks.Indexing; using Orchard.Tests.FileSystems.AppData; using Orchard.Tests.Stubs; @@ -38,10 +40,12 @@ namespace Orchard.Tests.Modules.Indexing { private IContentManager _contentManager; private Mock _contentDefinitionManager; private StubLogger _logger; - private const string IndexName = "Search"; + private ILockFileManager _lockFileManager; + private const string IndexName = "Search"; private readonly string _basePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - + + [TestFixtureTearDown] public void Clean() { if (Directory.Exists(_basePath)) { @@ -62,7 +66,6 @@ namespace Orchard.Tests.Modules.Indexing { builder.RegisterType().As(); builder.RegisterType().As(); builder.RegisterType().As(); - builder.RegisterType().As(); builder.RegisterType().As(); builder.RegisterType().As(); builder.RegisterInstance(_contentDefinitionManager.Object); @@ -80,6 +83,9 @@ namespace Orchard.Tests.Modules.Indexing { builder.RegisterType().As(); builder.RegisterType().As(); + builder.RegisterType().As(); + builder.RegisterInstance(_clock = new StubClock()); + // setting up a ShellSettings instance _shellSettings = new ShellSettings { Name = "My Site" }; builder.RegisterInstance(_shellSettings).As(); @@ -100,7 +106,7 @@ namespace Orchard.Tests.Modules.Indexing { public override void Init() { base.Init(); - + _lockFileManager = _container.Resolve(); _provider = _container.Resolve(); _indexNotifier = _container.Resolve(); _contentManager = _container.Resolve(); @@ -116,10 +122,6 @@ namespace Orchard.Tests.Modules.Indexing { .Returns(thingType); } - private string[] Indexes() { - return new DirectoryInfo(Path.Combine(_basePath, "Sites", "My Site", "Indexes")).GetDirectories().Select(d => d.Name).ToArray(); - } - [Test] public void IndexShouldBeEmptyWhenThereIsNoContent() { _indexNotifier.UpdateIndex(IndexName); @@ -130,7 +132,7 @@ namespace Orchard.Tests.Modules.Indexing { } [Test] - public void ShouldIngoreNonIndexableContentWhenRebuildingTheIndex() { + public void ShouldIgnoreNonIndexableContentWhenRebuildingTheIndex() { var alphaType = new ContentTypeDefinitionBuilder() .Named("alpha") .Build(); @@ -195,6 +197,21 @@ namespace Orchard.Tests.Modules.Indexing { Assert.That(_logger.LogEntries, Has.None.Matches(entry => entry.LogFormat == "Rebuild index started")); } + [Test] + public void IndexingTaskExecutorShouldBeReEntrant() { + ILockFile lockFile = null; + _lockFileManager.TryAcquireLock("Sites/My Site/Search.settings.xml.lock", ref lockFile); + using (lockFile) { + _indexNotifier.UpdateIndex(IndexName); + Assert.That(_logger.LogEntries.Count, Is.EqualTo(1)); + Assert.That(_logger.LogEntries, Has.Some.Matches(entry => entry.LogFormat == "Index was requested but was already running")); + } + + _logger.LogEntries.Clear(); + _indexNotifier.UpdateIndex(IndexName); + Assert.That(_logger.LogEntries, Has.None.Matches(entry => entry.LogFormat == "Index was requested but was already running")); + } + [Test] public void ShouldUpdateTheIndexWhenContentIsUnPublished() { _contentManager.Create(ThingDriver.ContentTypeName).Text = "Lorem ipsum"; @@ -293,7 +310,7 @@ namespace Orchard.Tests.Modules.Indexing { } public void Log(LogLevel level, Exception exception, string format, params object[] args) { - LogEntries.Add(new LogEntry() { + LogEntries.Add(new LogEntry { LogArgs = args, LogException = exception, LogFormat = format, diff --git a/src/Orchard.Web/Modules/Lucene/Services/LuceneIndexProvider.cs b/src/Orchard.Web/Modules/Lucene/Services/LuceneIndexProvider.cs index d89e8e32f..2ace88b61 100644 --- a/src/Orchard.Web/Modules/Lucene/Services/LuceneIndexProvider.cs +++ b/src/Orchard.Web/Modules/Lucene/Services/LuceneIndexProvider.cs @@ -206,10 +206,6 @@ namespace Lucene.Services { return new LuceneSearchBuilder(GetDirectory(indexName)) { Logger = Logger }; } - private string GetSettingsFileName(string indexName) { - return _appDataFolder.MapPath(_appDataFolder.Combine(_basePath, indexName + ".settings.xml")); - } - public DateTime? GetLastIndexUtc(string indexName) { var settingsFileName = GetSettingsFileName(indexName); @@ -254,5 +250,9 @@ namespace Lucene.Services { reader.Close(); } } + + private string GetSettingsFileName(string indexName) { + return _appDataFolder.MapPath(_appDataFolder.Combine(_basePath, indexName + ".settings.xml")); + } } } diff --git a/src/Orchard.Web/Modules/Orchard.Indexing/Orchard.Indexing.csproj b/src/Orchard.Web/Modules/Orchard.Indexing/Orchard.Indexing.csproj index be7ea6005..aff4ca59d 100644 --- a/src/Orchard.Web/Modules/Orchard.Indexing/Orchard.Indexing.csproj +++ b/src/Orchard.Web/Modules/Orchard.Indexing/Orchard.Indexing.csproj @@ -60,7 +60,6 @@ - diff --git a/src/Orchard.Web/Modules/Orchard.Indexing/Services/IndexSynLock.cs b/src/Orchard.Web/Modules/Orchard.Indexing/Services/IndexSynLock.cs deleted file mode 100644 index 7b545ccaf..000000000 --- a/src/Orchard.Web/Modules/Orchard.Indexing/Services/IndexSynLock.cs +++ /dev/null @@ -1,25 +0,0 @@ -using System.Collections.Generic; - -namespace Orchard.Indexing.Services { - public interface IIndexSynLock : ISingletonDependency { - object GetSynLock(string indexName); - } - - public class IndexSynLock : IIndexSynLock { - private readonly Dictionary _synLocks; - private readonly object _synLock = new object(); - - public IndexSynLock() { - _synLocks =new Dictionary(); - } - - public object GetSynLock(string indexName) { - lock(_synLock) { - if(!_synLocks.ContainsKey(indexName)) { - _synLocks[indexName] = new object(); - } - return _synLocks[indexName]; - } - } - } -} \ No newline at end of file diff --git a/src/Orchard.Web/Modules/Orchard.Indexing/Services/IndexingTaskExecutor.cs b/src/Orchard.Web/Modules/Orchard.Indexing/Services/IndexingTaskExecutor.cs index 3747fe997..e9a969d17 100644 --- a/src/Orchard.Web/Modules/Orchard.Indexing/Services/IndexingTaskExecutor.cs +++ b/src/Orchard.Web/Modules/Orchard.Indexing/Services/IndexingTaskExecutor.cs @@ -4,6 +4,9 @@ using System.Linq; using JetBrains.Annotations; using Orchard.ContentManagement; using Orchard.Data; +using Orchard.Environment.Configuration; +using Orchard.FileSystems.AppData; +using Orchard.FileSystems.LockFile; using Orchard.Indexing.Models; using Orchard.Indexing.Settings; using Orchard.Logging; @@ -14,6 +17,10 @@ namespace Orchard.Indexing.Services { /// /// Contains the logic which is regularly executed to retrieve index information from multiple content handlers. /// + /// + /// This class is synchronized using a lock file as both command line and web workers can potentially use it, + /// and singleton locks would not be shared accross those two. + /// [UsedImplicitly] public class IndexingTaskExecutor : IIndexNotifierHandler { private readonly IClock _clock; @@ -22,7 +29,9 @@ namespace Orchard.Indexing.Services { private readonly IIndexManager _indexManager; private readonly IIndexingTaskManager _indexingTaskManager; private readonly IContentManager _contentManager; - private readonly IIndexSynLock _indexSynLock; + private readonly IAppDataFolder _appDataFolder; + private readonly ShellSettings _shellSettings; + private readonly ILockFileManager _lockFileManager; public IndexingTaskExecutor( IClock clock, @@ -30,28 +39,34 @@ namespace Orchard.Indexing.Services { IIndexManager indexManager, IIndexingTaskManager indexingTaskManager, IContentManager contentManager, - IIndexSynLock indexSynLock) { + IAppDataFolder appDataFolder, + ShellSettings shellSettings, + ILockFileManager lockFileManager) { _clock = clock; _repository = repository; _indexManager = indexManager; _indexingTaskManager = indexingTaskManager; _contentManager = contentManager; - _indexSynLock = indexSynLock; + _appDataFolder = appDataFolder; + _shellSettings = shellSettings; + _lockFileManager = lockFileManager; + Logger = NullLogger.Instance; } public ILogger Logger { get; set; } public void UpdateIndex(string indexName) { - var synLock = _indexSynLock.GetSynLock(indexName); + ILockFile lockFile = null; + var settingsFilename = GetSettingsFileName(indexName); + var lockFilename = settingsFilename + ".lock"; - if (!System.Threading.Monitor.TryEnter(synLock)) { + if (!_lockFileManager.TryAcquireLock(lockFilename, ref lockFile)) { Logger.Information("Index was requested but was already running"); return; } - try { - + using (lockFile) { if (!_indexManager.HasIndexProvider()) { return; } @@ -99,8 +114,8 @@ namespace Orchard.Indexing.Services { // retrieve not yet processed tasks var taskRecords = lastIndexUtc == null - ? _repository.Fetch(x => true).ToArray() - : _repository.Fetch(x => x.CreatedUtc >= lastIndexUtc).ToArray(); // CreatedUtc and lastIndexUtc might be equal if a content item is created in a background task + ? _repository.Fetch(x => true).ToArray() + : _repository.Fetch(x => x.CreatedUtc >= lastIndexUtc).ToArray(); // CreatedUtc and lastIndexUtc might be equal if a content item is created in a background task // nothing to do ?))) if (taskRecords.Length + updateIndexDocuments.Count == 0) { @@ -158,9 +173,6 @@ namespace Orchard.Indexing.Services { } } } - finally { - System.Threading.Monitor.Exit(synLock); - } } static TypeIndexing GetTypeIndexingSettings(ContentItem contentItem) { @@ -171,5 +183,9 @@ namespace Orchard.Indexing.Services { } return contentItem.TypeDefinition.Settings.GetModel(); } + + private string GetSettingsFileName(string indexName) { + return _appDataFolder.Combine("Sites", _shellSettings.Name, indexName + ".settings.xml"); + } } }