Using lock files instead of IndexSynLock

--HG--
branch : indexing

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<IContentDefinitionManager> _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<IndexingTaskExecutor>().As<IIndexNotifierHandler>();
             builder.RegisterType<DefaultIndexManager>().As<IIndexManager>();
             builder.RegisterType<IndexingTaskManager>().As<IIndexingTaskManager>();
-            builder.RegisterType<IndexSynLock>().As<IIndexSynLock>();
             builder.RegisterType<DefaultContentManager>().As<IContentManager>();
             builder.RegisterType<DefaultContentManagerSession>().As<IContentManagerSession>();
             builder.RegisterInstance(_contentDefinitionManager.Object);
@@ -80,6 +83,9 @@ namespace Orchard.Tests.Modules.Indexing {
             builder.RegisterType<BodyPartHandler>().As<IContentHandler>();
             builder.RegisterType<StubExtensionManager>().As<IExtensionManager>();
 
+            builder.RegisterType<DefaultLockFileManager>().As<ILockFileManager>();
+            builder.RegisterInstance<IClock>(_clock = new StubClock());
+
             // setting up a ShellSettings instance
             _shellSettings = new ShellSettings { Name = "My Site" };
             builder.RegisterInstance(_shellSettings).As<ShellSettings>();
@@ -100,7 +106,7 @@ namespace Orchard.Tests.Modules.Indexing {
 
         public override void Init() {
             base.Init();
-
+            _lockFileManager = _container.Resolve<ILockFileManager>();
             _provider = _container.Resolve<IIndexProvider>();
             _indexNotifier = _container.Resolve<IIndexNotifierHandler>();
             _contentManager = _container.Resolve<IContentManager>();
@@ -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<LogEntry>(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<LogEntry>(entry => entry.LogFormat == "Index was requested but was already running"));
+            }
+
+            _logger.LogEntries.Clear();
+            _indexNotifier.UpdateIndex(IndexName);
+            Assert.That(_logger.LogEntries, Has.None.Matches<LogEntry>(entry => entry.LogFormat == "Index was requested but was already running"));
+        }
+
         [Test]
         public void ShouldUpdateTheIndexWhenContentIsUnPublished() {
             _contentManager.Create<Thing>(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,

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"));
+        }
     }
 }

src/Orchard.Web/Modules/Orchard.Indexing/Orchard.Indexing.csproj

@@ -60,7 +60,6 @@
     <Compile Include="Services\IndexingTaskExecutor.cs" />
     <Compile Include="Services\IndexingTaskManager.cs" />
     <Compile Include="Services\IIndexService.cs" />
-    <Compile Include="Services\IndexSynLock.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="Services\IndexService.cs" />
     <Compile Include="Settings\EditorEvents.cs" />

src/Orchard.Web/Modules/Orchard.Indexing/Services/IndexSynLock.cs

@@ -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<string, object> _synLocks;
-        private readonly object _synLock = new object();
-
-        public IndexSynLock() {
-            _synLocks =new Dictionary<string, object>();       
-        }
-
-        public object GetSynLock(string indexName) {
-            lock(_synLock) {
-                if(!_synLocks.ContainsKey(indexName)) {
-                    _synLocks[indexName] = new object();
-                }
-                return _synLocks[indexName];
-            }
-        }
-    }
-}

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 {
     /// <summary>
     /// Contains the logic which is regularly executed to retrieve index information from multiple content handlers.
     /// </summary>
+    /// <remarks>
+    /// 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.
+    /// </remarks>
     [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 => 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 => 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<TypeIndexing>();
         }
+
+        private string GetSettingsFileName(string indexName) {
+            return _appDataFolder.Combine("Sites", _shellSettings.Name, indexName + ".settings.xml");
+        }
     }
 }