Fixes SyncedFileSystemDirectory

Ensures replicators and dependencies are all disposed correctly.

Author: Shazwazza

src/Examine.Benchmarks/ConcurrentAcquireBenchmarks.cs

@@ -30,8 +30,7 @@ public override void Setup()
             _tempBasePath = Path.Combine(Path.GetTempPath(), "ExamineTests");
 
             // indexer for lucene
-            var tempIndexer = InitializeAndIndexItems(_tempBasePath, _analyzer, out var indexDir);
-            tempIndexer.Dispose();
+            InitializeAndIndexItems(_tempBasePath, _analyzer, out var indexDir);
             _indexDir = FSDirectory.Open(indexDir);
             var writerConfig = new IndexWriterConfig(LuceneVersion.LUCENE_48, _analyzer);
             //writerConfig.SetMaxBufferedDocs(1000);
@@ -113,16 +112,16 @@ public async Task TestAcquireThreadContention()
         protected override ILoggerFactory CreateLoggerFactory()
             => Microsoft.Extensions.Logging.LoggerFactory.Create(builder => builder.AddConsole().SetMinimumLevel(LogLevel.Information));
 #endif
-        private TestIndex InitializeAndIndexItems(
+        private void InitializeAndIndexItems(
             string tempBasePath,
             Analyzer analyzer,
             out DirectoryInfo indexDir)
         {
             var tempPath = Path.Combine(tempBasePath, Guid.NewGuid().ToString());
             System.IO.Directory.CreateDirectory(tempPath);
             indexDir = new DirectoryInfo(tempPath);
-            var luceneDirectory = FSDirectory.Open(indexDir);
-            var indexer = GetTestIndex(luceneDirectory, analyzer);
+            using var luceneDirectory = FSDirectory.Open(indexDir);
+            using var indexer = GetTestIndex(luceneDirectory, analyzer);
 
             var random = new Random();
             var valueSets = new List<ValueSet>();
@@ -138,8 +137,6 @@ private TestIndex InitializeAndIndexItems(
             }
 
             indexer.IndexItems(valueSets);
-
-            return indexer;
         }
     }
 }

src/Examine.Benchmarks/ExamineBaseTest.cs

@@ -35,7 +35,7 @@ public TestIndex GetTestIndex(
                 Mock.Of<IOptionsMonitor<LuceneDirectoryIndexOptions>>(x => x.Get(TestIndex.TestIndexName) == new LuceneDirectoryIndexOptions
                 {
                     FieldDefinitions = fieldDefinitions,
-                    DirectoryFactory = new GenericDirectoryFactory(_ => d),
+                    DirectoryFactory = new GenericDirectoryFactory(_ => d, true),
                     Analyzer = analyzer,
                     IndexDeletionPolicy = indexDeletionPolicy,
                     IndexValueTypesFactory = indexValueTypesFactory,

src/Examine.Core/ExamineManager.cs

@@ -12,12 +12,12 @@ public class ExamineManager : IDisposable, IExamineManager
     {
         public ExamineManager(IEnumerable<IIndex> indexes, IEnumerable<ISearcher> searchers)
         {
-            foreach(IIndex i in indexes)
+            foreach (var i in indexes)
             {
                 AddIndex(i);
             }
 
-            foreach(ISearcher s in searchers)
+            foreach (var s in searchers)
             {
                 AddSearcher(s);
             }
@@ -74,7 +74,7 @@ protected virtual void Dispose(bool disposing)
             {
                 if (disposing)
                 {
-                    Stop(false);
+                    // NOTE: Legacy - this used to dispose indexes, but that is managed by DI now
                     Stop(true);
                 }
 
@@ -112,21 +112,6 @@ public virtual void Stop(bool immediate)
                     // no strange lucene background thread stuff causes issues here.
                 }                
             }
-            else
-            {
-                try
-                {
-                    foreach (var indexer in Indexes.OfType<IDisposable>())
-                    {
-                        indexer.Dispose();
-                    }
-                }
-                catch (Exception)
-                {
-                    //an exception shouldn't occur but if so we need to terminate
-                    Stop(true);
-                }
-            }
         }
     }
 }

src/Examine.Host/ServicesCollectionExtensions.cs

@@ -69,10 +69,10 @@ public static IServiceCollection AddExamineLuceneIndex<TIndex, TDirectoryFactory
 
             return serviceCollection.AddSingleton<IIndex>(services =>
             {
-                IOptionsMonitor<LuceneDirectoryIndexOptions> options
+                var options
                         = services.GetRequiredService<IOptionsMonitor<LuceneDirectoryIndexOptions>>();
 
-                TIndex index = ActivatorUtilities.CreateInstance<TIndex>(
+                var index = ActivatorUtilities.CreateInstance<TIndex>(
                     services,
                     new object[] { name, options });
 
@@ -97,10 +97,10 @@ public static IServiceCollection AddExamineSearcher<TSearcher>(
             where TSearcher : ISearcher
            => serviceCollection.AddTransient<ISearcher>(services =>
            {
-               IList<object> parameters = parameterFactory(services);
+               var parameters = parameterFactory(services);
                parameters.Insert(0, name);
 
-               TSearcher searcher = ActivatorUtilities.CreateInstance<TSearcher>(
+               var searcher = ActivatorUtilities.CreateInstance<TSearcher>(
                    services,
                    parameters.ToArray());
 

src/Examine.Lucene/Directories/DirectoryFactory.cs

@@ -9,11 +9,25 @@ public class GenericDirectoryFactory : DirectoryFactoryBase
     {
         private readonly Func<string, Directory> _factory;
 
-        public GenericDirectoryFactory(Func<string, Directory> factory) => _factory = factory;
+        public GenericDirectoryFactory(Func<string, Directory> factory)
+        {
+            _factory = factory;
+        }
+
+        internal GenericDirectoryFactory(Func<string, Directory> factory, bool externallyManaged)
+        {
+            _factory = factory;
+            ExternallyManaged = externallyManaged;
+        }
+
+        /// <summary>
+        /// When set to true, indicates that the directory is managed externally and will be disposed of by the caller, not the index.
+        /// </summary>
+        internal bool ExternallyManaged { get; }
 
         protected override Directory CreateDirectory(LuceneIndex luceneIndex, bool forceUnlock)
         {
-            Directory dir = _factory(luceneIndex.Name);
+            var dir = _factory(luceneIndex.Name);
             if (forceUnlock)
             {
                 IndexWriter.Unlock(dir);

src/Examine.Lucene/Directories/DirectoryFactoryBase.cs

@@ -6,36 +6,17 @@ namespace Examine.Lucene.Directories
 {
     public abstract class DirectoryFactoryBase : IDirectoryFactory
     {
-        private readonly ConcurrentDictionary<string, Directory> _createdDirectories = new ConcurrentDictionary<string, Directory>();
-        private bool _disposedValue;
-
         Directory IDirectoryFactory.CreateDirectory(LuceneIndex luceneIndex, bool forceUnlock)
-            => _createdDirectories.GetOrAdd(
-                luceneIndex.Name,
-                s => CreateDirectory(luceneIndex, forceUnlock));
+            => CreateDirectory(luceneIndex, forceUnlock);
 
         protected abstract Directory CreateDirectory(LuceneIndex luceneIndex, bool forceUnlock);
 
         protected virtual void Dispose(bool disposing)
         {
-            if (!_disposedValue)
-            {
-                if (disposing)
-                {                    
-                    foreach (Directory d in _createdDirectories.Values)
-                    {
-                        d.Dispose();
-                    }
-                }
-
-                _disposedValue = true;
-            }
         }
 
-        public void Dispose()
-        {
+        public void Dispose() =>
             // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
             Dispose(disposing: true);
-        }
     }
 }

src/Examine.Lucene/Directories/IDirectoryFactory.cs

@@ -8,7 +8,7 @@ namespace Examine.Lucene.Directories
     /// Creates a Lucene <see cref="Lucene.Net.Store.Directory"/> for an index
     /// </summary>
     /// <remarks>
-    /// The directory created must only be created ONCE per index and disposed when the factory is disposed.
+    /// The directory created must only be created ONCE per index and disposed when the index is disposed.
     /// </remarks>
     public interface IDirectoryFactory : IDisposable
     {

src/Examine.Lucene/Directories/SyncedFileSystemDirectory.cs

@@ -0,0 +1,47 @@
+using System.IO;
+using Examine.Lucene.Providers;
+using Lucene.Net.Store;
+using Microsoft.Extensions.Logging;
+using Directory = Lucene.Net.Store.Directory;
+
+namespace Examine.Lucene.Directories
+{
+    internal class SyncedFileSystemDirectory : FilterDirectory
+    {
+        private readonly ExamineReplicator _replicator;
+
+        public SyncedFileSystemDirectory(
+            ILogger<ExamineReplicator> replicatorLogger,
+            ILogger<LoggingReplicationClient> clientLogger,
+            Directory localLuceneDirectory,
+            Directory mainLuceneDirectory,
+            LuceneIndex luceneIndex,
+            DirectoryInfo tempDir)
+            : base(localLuceneDirectory)
+        {
+            // now create the replicator that will copy from local to main on schedule
+            _replicator = new ExamineReplicator(replicatorLogger, clientLogger, luceneIndex, localLuceneDirectory, mainLuceneDirectory, tempDir);
+            LocalLuceneDirectory = localLuceneDirectory;
+            MainLuceneDirectory = mainLuceneDirectory;
+        }
+
+        internal Directory LocalLuceneDirectory { get; }
+
+        internal Directory MainLuceneDirectory { get; }
+
+        public override Lock MakeLock(string name)
+        {
+            // Start replicating back to main, this is ok to call multiple times, it will only execute once.
+            _replicator.StartIndexReplicationOnSchedule(1000);
+
+            return base.MakeLock(name);
+        }
+
+        protected override void Dispose(bool disposing)
+        {
+            _replicator.Dispose();
+            MainLuceneDirectory.Dispose();
+            base.Dispose(disposing);
+        }
+    }
+}

src/Examine.Lucene/Directories/SyncedFileSystemDirectoryFactory.cs

@@ -27,8 +27,8 @@ public class SyncedFileSystemDirectoryFactory : FileSystemDirectoryFactory
         private readonly ILoggerFactory _loggerFactory;
         private readonly bool _tryFixMainIndexIfCorrupt;
         private readonly ILogger<SyncedFileSystemDirectoryFactory> _logger;
-        private ExamineReplicator _replicator;
-        private Directory _mainLuceneDir;
+        private readonly ILogger<ExamineReplicator> _replicatorLogger;
+        private readonly ILogger<LoggingReplicationClient> _clientLogger;
 
         [Obsolete("Use ctor with all dependencies")]
         public SyncedFileSystemDirectoryFactory(
@@ -75,6 +75,8 @@ public SyncedFileSystemDirectoryFactory(
             _loggerFactory = loggerFactory;
             _tryFixMainIndexIfCorrupt = tryFixMainIndexIfCorrupt;
             _logger = _loggerFactory.CreateLogger<SyncedFileSystemDirectoryFactory>();
+            _replicatorLogger = _loggerFactory.CreateLogger<ExamineReplicator>();
+            _clientLogger = _loggerFactory.CreateLogger<LoggingReplicationClient>();
         }
 
         internal CreateResult TryCreateDirectory(LuceneIndex luceneIndex, bool forceUnlock, out Directory directory)
@@ -88,24 +90,27 @@ internal CreateResult TryCreateDirectory(LuceneIndex luceneIndex, bool forceUnlo
             // used by the replicator, will be a short lived directory for each synced revision and deleted when finished.
             var tempDir = new DirectoryInfo(Path.Combine(_localDir.FullName, "Rep", Guid.NewGuid().ToString("N")));
 
-            _mainLuceneDir = base.CreateDirectory(luceneIndex, forceUnlock);
+            var mainLuceneDir = base.CreateDirectory(luceneIndex, forceUnlock);
             var localLuceneDir = FSDirectory.Open(
                 localLuceneIndexFolder,
                 LockFactory.GetLockFactory(localLuceneIndexFolder));
 
-            var mainIndexExists = DirectoryReader.IndexExists(_mainLuceneDir);
+            var mainIndexExists = DirectoryReader.IndexExists(mainLuceneDir);
             var localIndexExists = DirectoryReader.IndexExists(localLuceneDir);
 
             var mainResult = CreateResult.Init;
 
             if (mainIndexExists)
             {
-                mainResult = CheckIndexHealthAndFix(_mainLuceneDir, luceneIndex.Name, _tryFixMainIndexIfCorrupt);
+                mainResult = CheckIndexHealthAndFix(mainLuceneDir, luceneIndex.Name, _tryFixMainIndexIfCorrupt);
             }
 
             // the main index is/was unhealthy or missing, lets check the local index if it exists
             if (localIndexExists && (!mainIndexExists || mainResult.HasFlag(CreateResult.NotClean) || mainResult.HasFlag(CreateResult.MissingSegments)))
             {
+                // TODO: add details here and more below too
+                _logger.LogInformation("");
+
                 var localResult = CheckIndexHealthAndFix(localLuceneDir, luceneIndex.Name, false);
 
                 if (localResult == CreateResult.Init)
@@ -132,7 +137,7 @@ internal CreateResult TryCreateDirectory(LuceneIndex luceneIndex, bool forceUnlo
                             ? OpenMode.APPEND
                             : OpenMode.CREATE;
 
-                mainResult |= TryGetIndexWriter(openMode, _mainLuceneDir, true, luceneIndex.Name, out var indexWriter);
+                mainResult |= TryGetIndexWriter(openMode, mainLuceneDir, true, luceneIndex.Name, out var indexWriter);
                 using (indexWriter)
                 {
                     if (!mainResult.HasFlag(CreateResult.SyncedFromLocal))
@@ -142,27 +147,25 @@ internal CreateResult TryCreateDirectory(LuceneIndex luceneIndex, bool forceUnlo
                 }
             }
 
-            // now create the replicator that will copy from local to main on schedule
-            _replicator = new ExamineReplicator(_loggerFactory, luceneIndex, _mainLuceneDir, tempDir);
-
             if (forceUnlock)
             {
                 IndexWriter.Unlock(localLuceneDir);
             }
 
-            // Start replicating back to main
-            _replicator.StartIndexReplicationOnSchedule(1000);
+            Directory luceneDir;
 
             var options = IndexOptions.GetNamedOptions(luceneIndex.Name);
             if (options.NrtEnabled)
             {
-                directory = new NRTCachingDirectory(localLuceneDir, options.NrtCacheMaxMergeSizeMB, options.NrtCacheMaxCachedMB);
+                luceneDir = new NRTCachingDirectory(localLuceneDir, options.NrtCacheMaxMergeSizeMB, options.NrtCacheMaxCachedMB);
             }
             else
             {
-                directory = localLuceneDir;
+                luceneDir = localLuceneDir;
             }
 
+            directory = new SyncedFileSystemDirectory(_replicatorLogger, _clientLogger, luceneDir, mainLuceneDir, luceneIndex, tempDir);
+
             return mainResult;
         }
 
@@ -186,16 +189,6 @@ protected override Directory CreateDirectory(LuceneIndex luceneIndex, bool force
             return directory;
         }
 
-        protected override void Dispose(bool disposing)
-        {
-            base.Dispose(disposing);
-            if (disposing)
-            {
-                _replicator?.Dispose();
-                _mainLuceneDir?.Dispose();
-            }
-        }
-
         private CreateResult TryGetIndexWriter(
             OpenMode openMode,
             Directory luceneDirectory,
@@ -228,6 +221,8 @@ private CreateResult TryGetIndexWriter(
                     // Index is corrupted, typically this will be FileNotFoundException or CorruptIndexException
                     _logger.LogError(ex, "{IndexName} index is corrupt, a new one will be created", indexName);
 
+                    // TODO: Here I think we need to totally clear all files in the directory
+
                     indexWriter = GetIndexWriter(luceneDirectory, OpenMode.CREATE);
                 }
                 else
@@ -252,7 +247,7 @@ private void SyncIndex(IndexWriter sourceIndexWriter, bool forceUnlock, string i
 
             using (var sourceIndex = new LuceneIndex(_loggerFactory, indexName, new TempOptions(), sourceIndexWriter))
             using (var destinationLuceneDirectory = FSDirectory.Open(destinationDirectory, LockFactory.GetLockFactory(destinationDirectory)))
-            using (var replicator = new ExamineReplicator(_loggerFactory, sourceIndex, destinationLuceneDirectory, tempDir))
+            using (var replicator = new ExamineReplicator(_replicatorLogger, _clientLogger, sourceIndex, sourceIndexWriter.Directory, destinationLuceneDirectory, tempDir))
             {
                 if (forceUnlock)
                 {
@@ -328,7 +323,7 @@ private CreateResult CheckIndexHealthAndFix(
             return result;
         }
 
-        private IndexWriter GetIndexWriter(Directory mainDir, OpenMode openMode)
+        private static IndexWriter GetIndexWriter(Directory mainDir, OpenMode openMode)
         {
             var indexWriter = new IndexWriter(
                 mainDir,