Skip to content

Commit

Permalink
Cleanup, ensure correct disposal, ensure absolute max results
Browse files Browse the repository at this point in the history
  • Loading branch information
Shazwazza committed Aug 21, 2024
1 parent 3760023 commit aaa3d7e
Show file tree
Hide file tree
Showing 13 changed files with 57 additions and 70 deletions.
1 change: 1 addition & 0 deletions src/Examine.Core/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
const Examine.Search.QueryOptions.AbsoluteMaxResults = 10000 -> int
static Examine.SearchExtensions.Escape(this string s, float boost) -> Examine.Search.IExamineValue

Check warning on line 2 in src/Examine.Core/PublicAPI.Unshipped.txt

View workflow job for this annotation

GitHub Actions / build

Symbol 'static Examine.SearchExtensions.Escape(this string s, float boost) -> Examine.Search.IExamineValue' is part of the declared API, but is either not public or could not be found (https://github.com/dotnet/roslyn-analyzers/blob/main/src/PublicApiAnalyzers/PublicApiAnalyzers.Help.md)

Check warning on line 2 in src/Examine.Core/PublicAPI.Unshipped.txt

View workflow job for this annotation

GitHub Actions / build

Symbol 'static Examine.SearchExtensions.Escape(this string s, float boost) -> Examine.Search.IExamineValue' is part of the declared API, but is either not public or could not be found (https://github.com/dotnet/roslyn-analyzers/blob/main/src/PublicApiAnalyzers/PublicApiAnalyzers.Help.md)
2 changes: 2 additions & 0 deletions src/Examine.Core/Search/QueryOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
19 changes: 16 additions & 3 deletions src/Examine.Lucene/Directories/FileSystemDirectoryFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,24 @@
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
{
public class FileSystemDirectoryFactory : DirectoryFactoryBase
{
private readonly DirectoryInfo _baseDir;
private readonly IOptionsMonitor<LuceneDirectoryIndexOptions> _indexOptions;

public FileSystemDirectoryFactory(DirectoryInfo baseDir, ILockFactory lockFactory)
public FileSystemDirectoryFactory(
DirectoryInfo baseDir,
ILockFactory lockFactory,
IOptionsMonitor<LuceneDirectoryIndexOptions> indexOptions)
{
_baseDir = baseDir;
LockFactory = lockFactory;
_indexOptions = indexOptions;
}

public ILockFactory LockFactory { get; }
Expand All @@ -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;
}
}
}
}
20 changes: 12 additions & 8 deletions src/Examine.Lucene/Directories/SyncedFileSystemDirectoryFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ public class SyncedFileSystemDirectoryFactory : FileSystemDirectoryFactory
private readonly bool _tryFixMainIndexIfCorrupt;
private readonly ILogger<SyncedFileSystemDirectoryFactory> _logger;
private ExamineReplicator _replicator;
private Directory _mainLuceneDir;

public SyncedFileSystemDirectoryFactory(

Check warning on line 33 in src/Examine.Lucene/Directories/SyncedFileSystemDirectoryFactory.cs

View workflow job for this annotation

GitHub Actions / build

Symbol 'SyncedFileSystemDirectoryFactory' is not part of the declared public API (https://github.com/dotnet/roslyn-analyzers/blob/main/src/PublicApiAnalyzers/PublicApiAnalyzers.Help.md)
DirectoryInfo localDir,
DirectoryInfo mainDir,
ILockFactory lockFactory,
ILoggerFactory loggerFactory)
: this(localDir, mainDir, lockFactory, loggerFactory, false)
ILoggerFactory loggerFactory,
IOptionsMonitor<LuceneDirectoryIndexOptions> indexOptions)
: this(localDir, mainDir, lockFactory, loggerFactory, indexOptions, false)
{
}

Expand All @@ -43,8 +45,9 @@ public SyncedFileSystemDirectoryFactory(
DirectoryInfo mainDir,
ILockFactory lockFactory,
ILoggerFactory loggerFactory,
IOptionsMonitor<LuceneDirectoryIndexOptions> indexOptions,
bool tryFixMainIndexIfCorrupt)
: base(mainDir, lockFactory)
: base(mainDir, lockFactory, indexOptions)
{
_localDir = localDir;
_mainDir = mainDir;
Expand All @@ -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
Expand Down Expand Up @@ -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))
Expand All @@ -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)
{
Expand Down Expand Up @@ -161,6 +164,7 @@ protected override void Dispose(bool disposing)
if (disposing)
{
_replicator?.Dispose();
_mainLuceneDir?.Dispose();
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.IO;
using Microsoft.Extensions.Options;

namespace Examine.Lucene.Directories
{
Expand All @@ -14,8 +15,9 @@ public class TempEnvFileSystemDirectoryFactory : FileSystemDirectoryFactory
{
public TempEnvFileSystemDirectoryFactory(
IApplicationIdentifier applicationIdentifier,
ILockFactory lockFactory)
: base(new DirectoryInfo(GetTempPath(applicationIdentifier)), lockFactory)
ILockFactory lockFactory,
IOptionsMonitor<LuceneDirectoryIndexOptions> indexOptions)
: base(new DirectoryInfo(GetTempPath(applicationIdentifier)), lockFactory, indexOptions)
{
}

Expand Down
1 change: 0 additions & 1 deletion src/Examine.Lucene/ExamineReplicator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@ protected virtual void Dispose(bool disposing)
{
_sourceIndex.IndexCommitted -= SourceIndex_IndexCommitted;
_localReplicationClient.Dispose();
_destinationDirectory.Dispose();
}

_disposedValue = true;
Expand Down
4 changes: 1 addition & 3 deletions src/Examine.Lucene/Providers/LuceneSearcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ namespace Examine.Lucene.Providers
///</summary>
public class LuceneSearcher : BaseLuceneSearcher, IDisposable
{
private readonly object _locker = new object();
private readonly SearcherManager _searcherManager;
private readonly FieldValueTypeCollection _fieldValueTypeCollection;
private bool _disposedValue;
Expand All @@ -34,15 +33,14 @@ 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)
{
_searchContext = new SearchContext(_searcherManager, _fieldValueTypeCollection);
}

return _searchContext;

//return new SearchContext(_searcherManager, _fieldValueTypeCollection);
}

protected virtual void Dispose(bool disposing)
Expand Down
3 changes: 3 additions & 0 deletions src/Examine.Lucene/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
Examine.Lucene.Directories.FileSystemDirectoryFactory.FileSystemDirectoryFactory(System.IO.DirectoryInfo baseDir, Examine.Lucene.Directories.ILockFactory lockFactory, Microsoft.Extensions.Options.IOptionsMonitor<Examine.Lucene.LuceneDirectoryIndexOptions> 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<Examine.Lucene.LuceneDirectoryIndexOptions> 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?
3 changes: 2 additions & 1 deletion src/Examine.Lucene/Search/LuceneSearchExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Lucene.Net.Documents;
using Lucene.Net.Index;
using Lucene.Net.Search;
using Lucene.Net.Util;

namespace Examine.Lucene.Search
{
Expand Down Expand Up @@ -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;

Expand Down
9 changes: 6 additions & 3 deletions src/Examine.Lucene/Search/MultiSearchSearcherReference.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -35,7 +38,7 @@ protected virtual void Dispose(bool disposing)
{
if (disposing)
{
foreach(ISearcherReference i in _inner)
foreach (var i in _inner)
{
i.Dispose();
}
Expand Down
25 changes: 5 additions & 20 deletions src/Examine.Lucene/Search/SearchContext.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using System.Collections.Generic;
using System.Linq;
using Examine.Lucene.Indexing;
using Lucene.Net.Index;
Expand All @@ -12,32 +11,19 @@ public class SearchContext : ISearchContext
{
private readonly SearcherManager _searcherManager;
private readonly FieldValueTypeCollection _fieldValueTypeCollection;
private readonly Lazy<ISearcherReference> _searcherReference;
private string[] _searchableFields;

public SearchContext(SearcherManager searcherManager, FieldValueTypeCollection fieldValueTypeCollection)
{
_searcherManager = searcherManager;
_searcherManager = searcherManager;
_fieldValueTypeCollection = fieldValueTypeCollection ?? throw new ArgumentNullException(nameof(fieldValueTypeCollection));
_searcherReference = new Lazy<ISearcherReference>(() =>
{
// 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
{
Expand All @@ -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
{
Expand All @@ -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));
}
}
Expand Down
32 changes: 3 additions & 29 deletions src/Examine.Lucene/Search/SearcherReference.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public void Given_ExistingCorruptIndex_When_CreatingDirectory_Then_IndexCreatedO
new DirectoryInfo(mainPath),
new DefaultLockFactory(),
LoggerFactory,
Mock.Of<IOptionsMonitor<LuceneDirectoryIndexOptions>>(x => x.Get(TestIndex.TestIndexName) == new LuceneDirectoryIndexOptions()),
true);

using var index = new LuceneIndex(
Expand Down Expand Up @@ -82,6 +83,7 @@ public void Given_CorruptMainIndex_And_HealthyLocalIndex_When_CreatingDirectory_
new DirectoryInfo(mainPath),
new DefaultLockFactory(),
LoggerFactory,
Mock.Of<IOptionsMonitor<LuceneDirectoryIndexOptions>>(x => x.Get(TestIndex.TestIndexName) == new LuceneDirectoryIndexOptions()),
true))
{
using var index = new LuceneIndex(
Expand Down

0 comments on commit aaa3d7e

Please sign in to comment.