From aaa3d7e21fa431cd857bf44252a21fca750051ac Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 21 Aug 2024 11:09:45 -0600 Subject: [PATCH] Cleanup, ensure correct disposal, ensure absolute max results --- src/Examine.Core/PublicAPI.Unshipped.txt | 1 + src/Examine.Core/Search/QueryOptions.cs | 2 ++ .../Directories/FileSystemDirectoryFactory.cs | 19 +++++++++-- .../SyncedFileSystemDirectoryFactory.cs | 20 +++++++----- .../TempEnvFileSystemDirectoryFactory.cs | 6 ++-- src/Examine.Lucene/ExamineReplicator.cs | 1 - .../Providers/LuceneSearcher.cs | 4 +-- src/Examine.Lucene/PublicAPI.Unshipped.txt | 3 ++ .../Search/LuceneSearchExecutor.cs | 3 +- .../Search/MultiSearchSearcherReference.cs | 9 ++++-- src/Examine.Lucene/Search/SearchContext.cs | 25 +++------------ .../Search/SearcherReference.cs | 32 ++----------------- .../SyncedFileSystemDirectoryFactoryTests.cs | 2 ++ 13 files changed, 57 insertions(+), 70 deletions(-) diff --git a/src/Examine.Core/PublicAPI.Unshipped.txt b/src/Examine.Core/PublicAPI.Unshipped.txt index 3b367992e..cd28e4763 100644 --- a/src/Examine.Core/PublicAPI.Unshipped.txt +++ b/src/Examine.Core/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ +const Examine.Search.QueryOptions.AbsoluteMaxResults = 10000 -> int static Examine.SearchExtensions.Escape(this string s, float boost) -> Examine.Search.IExamineValue \ No newline at end of file diff --git a/src/Examine.Core/Search/QueryOptions.cs b/src/Examine.Core/Search/QueryOptions.cs index f4efadade..4b68860b7 100644 --- a/src/Examine.Core/Search/QueryOptions.cs +++ b/src/Examine.Core/Search/QueryOptions.cs @@ -4,6 +4,8 @@ namespace Examine.Search { public class QueryOptions { + public const int AbsoluteMaxResults = 10000; + public const int DefaultMaxResults = 500; public static QueryOptions SkipTake(int skip, int? take = null) => new QueryOptions(skip, take ?? DefaultMaxResults); diff --git a/src/Examine.Lucene/Directories/FileSystemDirectoryFactory.cs b/src/Examine.Lucene/Directories/FileSystemDirectoryFactory.cs index c2c258be2..f0eb05685 100644 --- a/src/Examine.Lucene/Directories/FileSystemDirectoryFactory.cs +++ b/src/Examine.Lucene/Directories/FileSystemDirectoryFactory.cs @@ -2,6 +2,7 @@ using Examine.Lucene.Providers; using Lucene.Net.Index; using Lucene.Net.Store; +using Microsoft.Extensions.Options; using Directory = Lucene.Net.Store.Directory; namespace Examine.Lucene.Directories @@ -9,11 +10,16 @@ namespace Examine.Lucene.Directories public class FileSystemDirectoryFactory : DirectoryFactoryBase { private readonly DirectoryInfo _baseDir; + private readonly IOptionsMonitor _indexOptions; - public FileSystemDirectoryFactory(DirectoryInfo baseDir, ILockFactory lockFactory) + public FileSystemDirectoryFactory( + DirectoryInfo baseDir, + ILockFactory lockFactory, + IOptionsMonitor indexOptions) { _baseDir = baseDir; LockFactory = lockFactory; + _indexOptions = indexOptions; } public ILockFactory LockFactory { get; } @@ -29,8 +35,15 @@ protected override Directory CreateDirectory(LuceneIndex luceneIndex, bool force IndexWriter.Unlock(dir); } - // TODO: Put this behind IOptions for NRT stuff, but I think this is going to be better - return new NRTCachingDirectory(dir, 5.0, 60.0); + var options = _indexOptions.GetNamedOptions(luceneIndex.Name); + if (options.NrtEnabled) + { + return new NRTCachingDirectory(dir, 5.0, 60.0); + } + else + { + return dir; + } } } } diff --git a/src/Examine.Lucene/Directories/SyncedFileSystemDirectoryFactory.cs b/src/Examine.Lucene/Directories/SyncedFileSystemDirectoryFactory.cs index e1400f48b..43be66bc0 100644 --- a/src/Examine.Lucene/Directories/SyncedFileSystemDirectoryFactory.cs +++ b/src/Examine.Lucene/Directories/SyncedFileSystemDirectoryFactory.cs @@ -28,13 +28,15 @@ public class SyncedFileSystemDirectoryFactory : FileSystemDirectoryFactory private readonly bool _tryFixMainIndexIfCorrupt; private readonly ILogger _logger; private ExamineReplicator _replicator; + private Directory _mainLuceneDir; public SyncedFileSystemDirectoryFactory( DirectoryInfo localDir, DirectoryInfo mainDir, ILockFactory lockFactory, - ILoggerFactory loggerFactory) - : this(localDir, mainDir, lockFactory, loggerFactory, false) + ILoggerFactory loggerFactory, + IOptionsMonitor indexOptions) + : this(localDir, mainDir, lockFactory, loggerFactory, indexOptions, false) { } @@ -43,8 +45,9 @@ public SyncedFileSystemDirectoryFactory( DirectoryInfo mainDir, ILockFactory lockFactory, ILoggerFactory loggerFactory, + IOptionsMonitor indexOptions, bool tryFixMainIndexIfCorrupt) - : base(mainDir, lockFactory) + : base(mainDir, lockFactory, indexOptions) { _localDir = localDir; _mainDir = mainDir; @@ -64,19 +67,19 @@ 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"))); - var mainLuceneDir = base.CreateDirectory(luceneIndex, forceUnlock); + _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 @@ -108,7 +111,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)) @@ -119,7 +122,7 @@ 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); + _replicator = new ExamineReplicator(_loggerFactory, luceneIndex, _mainLuceneDir, tempDir); if (forceUnlock) { @@ -161,6 +164,7 @@ protected override void Dispose(bool disposing) if (disposing) { _replicator?.Dispose(); + _mainLuceneDir?.Dispose(); } } diff --git a/src/Examine.Lucene/Directories/TempEnvFileSystemDirectoryFactory.cs b/src/Examine.Lucene/Directories/TempEnvFileSystemDirectoryFactory.cs index 0e5260101..04a924d51 100644 --- a/src/Examine.Lucene/Directories/TempEnvFileSystemDirectoryFactory.cs +++ b/src/Examine.Lucene/Directories/TempEnvFileSystemDirectoryFactory.cs @@ -1,5 +1,6 @@ using System; using System.IO; +using Microsoft.Extensions.Options; namespace Examine.Lucene.Directories { @@ -14,8 +15,9 @@ public class TempEnvFileSystemDirectoryFactory : FileSystemDirectoryFactory { public TempEnvFileSystemDirectoryFactory( IApplicationIdentifier applicationIdentifier, - ILockFactory lockFactory) - : base(new DirectoryInfo(GetTempPath(applicationIdentifier)), lockFactory) + ILockFactory lockFactory, + IOptionsMonitor indexOptions) + : base(new DirectoryInfo(GetTempPath(applicationIdentifier)), lockFactory, indexOptions) { } diff --git a/src/Examine.Lucene/ExamineReplicator.cs b/src/Examine.Lucene/ExamineReplicator.cs index 12a0a552e..d3590a839 100644 --- a/src/Examine.Lucene/ExamineReplicator.cs +++ b/src/Examine.Lucene/ExamineReplicator.cs @@ -149,7 +149,6 @@ protected virtual void Dispose(bool disposing) { _sourceIndex.IndexCommitted -= SourceIndex_IndexCommitted; _localReplicationClient.Dispose(); - _destinationDirectory.Dispose(); } _disposedValue = true; diff --git a/src/Examine.Lucene/Providers/LuceneSearcher.cs b/src/Examine.Lucene/Providers/LuceneSearcher.cs index 9642a0f76..d285919ae 100644 --- a/src/Examine.Lucene/Providers/LuceneSearcher.cs +++ b/src/Examine.Lucene/Providers/LuceneSearcher.cs @@ -12,7 +12,6 @@ namespace Examine.Lucene.Providers /// public class LuceneSearcher : BaseLuceneSearcher, IDisposable { - private readonly object _locker = new object(); private readonly SearcherManager _searcherManager; private readonly FieldValueTypeCollection _fieldValueTypeCollection; private bool _disposedValue; @@ -34,6 +33,7 @@ public LuceneSearcher(string name, SearcherManager searcherManager, Analyzer ana public override ISearchContext GetSearchContext() { + // Don't create a new search context unless something has changed var isCurrent = _searcherManager.IsSearcherCurrent(); if (_searchContext is null || !isCurrent) { @@ -41,8 +41,6 @@ public override ISearchContext GetSearchContext() } return _searchContext; - - //return new SearchContext(_searcherManager, _fieldValueTypeCollection); } protected virtual void Dispose(bool disposing) diff --git a/src/Examine.Lucene/PublicAPI.Unshipped.txt b/src/Examine.Lucene/PublicAPI.Unshipped.txt index c9e29a3e6..47d77c746 100644 --- a/src/Examine.Lucene/PublicAPI.Unshipped.txt +++ b/src/Examine.Lucene/PublicAPI.Unshipped.txt @@ -1,8 +1,11 @@ +Examine.Lucene.Directories.FileSystemDirectoryFactory.FileSystemDirectoryFactory(System.IO.DirectoryInfo baseDir, Examine.Lucene.Directories.ILockFactory lockFactory, Microsoft.Extensions.Options.IOptionsMonitor indexOptions) -> void Examine.Lucene.Directories.SyncedFileSystemDirectoryFactory.SyncedFileSystemDirectoryFactory(System.IO.DirectoryInfo localDir, System.IO.DirectoryInfo mainDir, Examine.Lucene.Directories.ILockFactory lockFactory, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory, bool tryFixMainIndexIfCorrupt) -> void +Examine.Lucene.Directories.TempEnvFileSystemDirectoryFactory.TempEnvFileSystemDirectoryFactory(Examine.Lucene.Directories.IApplicationIdentifier applicationIdentifier, Examine.Lucene.Directories.ILockFactory lockFactory, Microsoft.Extensions.Options.IOptionsMonitor indexOptions) -> void Examine.Lucene.LuceneIndexOptions.NrtEnabled.get -> bool Examine.Lucene.LuceneIndexOptions.NrtEnabled.set -> void Examine.Lucene.LuceneIndexOptions.NrtTargetMaxStaleSec.get -> double Examine.Lucene.LuceneIndexOptions.NrtTargetMaxStaleSec.set -> void Examine.Lucene.LuceneIndexOptions.NrtTargetMinStaleSec.get -> double Examine.Lucene.LuceneIndexOptions.NrtTargetMinStaleSec.set -> void +Examine.Lucene.Search.SearcherReference.SearcherReference() -> void virtual Examine.Lucene.Providers.LuceneIndex.UpdateLuceneDocument(Lucene.Net.Index.Term term, Lucene.Net.Documents.Document doc) -> long? \ No newline at end of file diff --git a/src/Examine.Lucene/Search/LuceneSearchExecutor.cs b/src/Examine.Lucene/Search/LuceneSearchExecutor.cs index b43944c11..053c62158 100644 --- a/src/Examine.Lucene/Search/LuceneSearchExecutor.cs +++ b/src/Examine.Lucene/Search/LuceneSearchExecutor.cs @@ -5,6 +5,7 @@ using Lucene.Net.Documents; using Lucene.Net.Index; using Lucene.Net.Search; +using Lucene.Net.Util; namespace Examine.Lucene.Search { @@ -70,7 +71,7 @@ public ISearchResults Execute() using (var searcher = _searchContext.GetSearcher()) { - var maxResults = (_options.Skip + 1) * _options.Take; + var maxResults = Math.Min((_options.Skip + 1) * _options.Take, QueryOptions.AbsoluteMaxResults); maxResults = maxResults >= 1 ? maxResults : QueryOptions.DefaultMaxResults; int numHits = maxResults; diff --git a/src/Examine.Lucene/Search/MultiSearchSearcherReference.cs b/src/Examine.Lucene/Search/MultiSearchSearcherReference.cs index 89c25bba2..96688739e 100644 --- a/src/Examine.Lucene/Search/MultiSearchSearcherReference.cs +++ b/src/Examine.Lucene/Search/MultiSearchSearcherReference.cs @@ -1,11 +1,14 @@ -using Lucene.Net.Index; +using Lucene.Net.Index; using Lucene.Net.Search; namespace Examine.Lucene.Search { public class MultiSearchSearcherReference : ISearcherReference { - public MultiSearchSearcherReference(ISearcherReference[] inner) => _inner = inner; + public MultiSearchSearcherReference(ISearcherReference[] inner) + { + _inner = inner; + } private bool _disposedValue; private IndexSearcher _searcher; @@ -35,7 +38,7 @@ protected virtual void Dispose(bool disposing) { if (disposing) { - foreach(ISearcherReference i in _inner) + foreach (var i in _inner) { i.Dispose(); } diff --git a/src/Examine.Lucene/Search/SearchContext.cs b/src/Examine.Lucene/Search/SearchContext.cs index d17d4d889..b4d514b03 100644 --- a/src/Examine.Lucene/Search/SearchContext.cs +++ b/src/Examine.Lucene/Search/SearchContext.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Linq; using Examine.Lucene.Indexing; using Lucene.Net.Index; @@ -12,32 +11,19 @@ public class SearchContext : ISearchContext { private readonly SearcherManager _searcherManager; private readonly FieldValueTypeCollection _fieldValueTypeCollection; - private readonly Lazy _searcherReference; private string[] _searchableFields; public SearchContext(SearcherManager searcherManager, FieldValueTypeCollection fieldValueTypeCollection) { - _searcherManager = searcherManager; + _searcherManager = searcherManager; _fieldValueTypeCollection = fieldValueTypeCollection ?? throw new ArgumentNullException(nameof(fieldValueTypeCollection)); - _searcherReference = new Lazy(() => - { - // TODO: Only if NRT is disabled? - //_searcherManager.MaybeRefresh(); - - return new SearcherReference(_searcherManager); - }); } // TODO: Do we want to create a new searcher every time? I think so, but we shouldn't allocate so much - public ISearcherReference GetSearcher() - { - //return _searcherReference.Value; - + public ISearcherReference GetSearcher() => // TODO: Only if NRT is disabled? //_searcherManager.MaybeRefresh(); - - return new SearcherReference(_searcherManager); - } + new SearcherReference(_searcherManager); public string[] SearchableFields { @@ -48,8 +34,7 @@ public string[] SearchableFields // IMPORTANT! Do not resolve the IndexSearcher from the `IndexSearcher` property above since this // will not release it from the searcher manager. When we are collecting fields, we are essentially // performing a 'search'. We must ensure that the underlying reader has the correct reference counts. - IndexSearcher searcher = _searcherManager.Acquire(); - //var searcher = GetSearcher().IndexSearcher; + var searcher = _searcherManager.Acquire(); try { @@ -76,7 +61,7 @@ public IIndexFieldValueType GetFieldValueType(string fieldName) { //Get the value type for the field, or use the default if not defined return _fieldValueTypeCollection.GetValueType( - fieldName, + fieldName, _fieldValueTypeCollection.ValueTypeFactories.GetRequiredFactory(FieldDefinitionTypes.FullText)); } } diff --git a/src/Examine.Lucene/Search/SearcherReference.cs b/src/Examine.Lucene/Search/SearcherReference.cs index f658bad51..71fb9dff3 100644 --- a/src/Examine.Lucene/Search/SearcherReference.cs +++ b/src/Examine.Lucene/Search/SearcherReference.cs @@ -1,45 +1,19 @@ -using System; using Lucene.Net.Search; namespace Examine.Lucene.Search { - // TODO: struct public readonly struct SearcherReference : ISearcherReference { - //private bool _disposedValue; private readonly SearcherManager _searcherManager; - private readonly IndexSearcher _searcher; public SearcherReference(SearcherManager searcherManager) { _searcherManager = searcherManager; - _searcher = _searcherManager.Acquire(); + IndexSearcher = _searcherManager.Acquire(); } - public IndexSearcher IndexSearcher - { - get - { - //if (_disposedValue) - //{ - // throw new ObjectDisposedException($"{nameof(SearcherReference)} is disposed"); - //} - - //return _searcher ??= _searcherManager.Acquire(); - return _searcher; - } - } + public IndexSearcher IndexSearcher { get; } - public void Dispose() - { - //if (!_disposedValue) - //{ - //if (_searcher != null) - //{ - _searcherManager.Release(_searcher); - //} - // _disposedValue = true; - //} - } + public void Dispose() => _searcherManager.Release(IndexSearcher); } } diff --git a/src/Examine.Test/Examine.Lucene/Directories/SyncedFileSystemDirectoryFactoryTests.cs b/src/Examine.Test/Examine.Lucene/Directories/SyncedFileSystemDirectoryFactoryTests.cs index e124c784e..e591a03f9 100644 --- a/src/Examine.Test/Examine.Lucene/Directories/SyncedFileSystemDirectoryFactoryTests.cs +++ b/src/Examine.Test/Examine.Lucene/Directories/SyncedFileSystemDirectoryFactoryTests.cs @@ -42,6 +42,7 @@ public void Given_ExistingCorruptIndex_When_CreatingDirectory_Then_IndexCreatedO new DirectoryInfo(mainPath), new DefaultLockFactory(), LoggerFactory, + Mock.Of>(x => x.Get(TestIndex.TestIndexName) == new LuceneDirectoryIndexOptions()), true); using var index = new LuceneIndex( @@ -82,6 +83,7 @@ public void Given_CorruptMainIndex_And_HealthyLocalIndex_When_CreatingDirectory_ new DirectoryInfo(mainPath), new DefaultLockFactory(), LoggerFactory, + Mock.Of>(x => x.Get(TestIndex.TestIndexName) == new LuceneDirectoryIndexOptions()), true)) { using var index = new LuceneIndex(