Skip to content

Commit c4b445d

Browse files
ShazwazzaShazwazza
committed
Fixes SyncedFileSystemDirectory
Ensures replicators and dependencies are all disposed correctly.
1 parent 6ec92e4 commit c4b445d

17 files changed

+399
-156
lines changed

‎src/Examine.Benchmarks/ConcurrentAcquireBenchmarks.cs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ public override void Setup()
3030
_tempBasePath = Path.Combine(Path.GetTempPath(), "ExamineTests");
3131

3232
// indexer for lucene
33-
var tempIndexer = InitializeAndIndexItems(_tempBasePath, _analyzer, out var indexDir);
34-
tempIndexer.Dispose();
33+
InitializeAndIndexItems(_tempBasePath, _analyzer, out var indexDir);
3534
_indexDir = FSDirectory.Open(indexDir);
3635
var writerConfig = new IndexWriterConfig(LuceneVersion.LUCENE_48, _analyzer);
3736
//writerConfig.SetMaxBufferedDocs(1000);
@@ -113,16 +112,16 @@ public async Task TestAcquireThreadContention()
113112
protected override ILoggerFactory CreateLoggerFactory()
114113
=> Microsoft.Extensions.Logging.LoggerFactory.Create(builder => builder.AddConsole().SetMinimumLevel(LogLevel.Information));
115114
#endif
116-
private TestIndex InitializeAndIndexItems(
115+
private void InitializeAndIndexItems(
117116
string tempBasePath,
118117
Analyzer analyzer,
119118
out DirectoryInfo indexDir)
120119
{
121120
var tempPath = Path.Combine(tempBasePath, Guid.NewGuid().ToString());
122121
System.IO.Directory.CreateDirectory(tempPath);
123122
indexDir = new DirectoryInfo(tempPath);
124-
var luceneDirectory = FSDirectory.Open(indexDir);
125-
var indexer = GetTestIndex(luceneDirectory, analyzer);
123+
using var luceneDirectory = FSDirectory.Open(indexDir);
124+
using var indexer = GetTestIndex(luceneDirectory, analyzer);
126125

127126
var random = new Random();
128127
var valueSets = new List<ValueSet>();
@@ -138,8 +137,6 @@ private TestIndex InitializeAndIndexItems(
138137
}
139138

140139
indexer.IndexItems(valueSets);
141-
142-
return indexer;
143140
}
144141
}
145142
}

‎src/Examine.Benchmarks/ExamineBaseTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public TestIndex GetTestIndex(
3535
Mock.Of<IOptionsMonitor<LuceneDirectoryIndexOptions>>(x => x.Get(TestIndex.TestIndexName) == new LuceneDirectoryIndexOptions
3636
{
3737
FieldDefinitions = fieldDefinitions,
38-
DirectoryFactory = new GenericDirectoryFactory(_ => d),
38+
DirectoryFactory = new GenericDirectoryFactory(_ => d, true),
3939
Analyzer = analyzer,
4040
IndexDeletionPolicy = indexDeletionPolicy,
4141
IndexValueTypesFactory = indexValueTypesFactory,

‎src/Examine.Core/ExamineManager.cs

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ public class ExamineManager : IDisposable, IExamineManager
1212
{
1313
public ExamineManager(IEnumerable<IIndex> indexes, IEnumerable<ISearcher> searchers)
1414
{
15-
foreach(IIndex i in indexes)
15+
foreach (var i in indexes)
1616
{
1717
AddIndex(i);
1818
}
1919

20-
foreach(ISearcher s in searchers)
20+
foreach (var s in searchers)
2121
{
2222
AddSearcher(s);
2323
}
@@ -74,7 +74,7 @@ protected virtual void Dispose(bool disposing)
7474
{
7575
if (disposing)
7676
{
77-
Stop(false);
77+
// NOTE: Legacy - this used to dispose indexes, but that is managed by DI now
7878
Stop(true);
7979
}
8080

@@ -112,21 +112,6 @@ public virtual void Stop(bool immediate)
112112
// no strange lucene background thread stuff causes issues here.
113113
}
114114
}
115-
else
116-
{
117-
try
118-
{
119-
foreach (var indexer in Indexes.OfType<IDisposable>())
120-
{
121-
indexer.Dispose();
122-
}
123-
}
124-
catch (Exception)
125-
{
126-
//an exception shouldn't occur but if so we need to terminate
127-
Stop(true);
128-
}
129-
}
130115
}
131116
}
132117
}

‎src/Examine.Host/ServicesCollectionExtensions.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,10 @@ public static IServiceCollection AddExamineLuceneIndex<TIndex, TDirectoryFactory
6969

7070
return serviceCollection.AddSingleton<IIndex>(services =>
7171
{
72-
IOptionsMonitor<LuceneDirectoryIndexOptions> options
72+
var options
7373
= services.GetRequiredService<IOptionsMonitor<LuceneDirectoryIndexOptions>>();
7474

75-
TIndex index = ActivatorUtilities.CreateInstance<TIndex>(
75+
var index = ActivatorUtilities.CreateInstance<TIndex>(
7676
services,
7777
new object[] { name, options });
7878

@@ -97,10 +97,10 @@ public static IServiceCollection AddExamineSearcher<TSearcher>(
9797
where TSearcher : ISearcher
9898
=> serviceCollection.AddTransient<ISearcher>(services =>
9999
{
100-
IList<object> parameters = parameterFactory(services);
100+
var parameters = parameterFactory(services);
101101
parameters.Insert(0, name);
102102

103-
TSearcher searcher = ActivatorUtilities.CreateInstance<TSearcher>(
103+
var searcher = ActivatorUtilities.CreateInstance<TSearcher>(
104104
services,
105105
parameters.ToArray());
106106

‎src/Examine.Lucene/Directories/DirectoryFactory.cs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,25 @@ public class GenericDirectoryFactory : DirectoryFactoryBase
99
{
1010
private readonly Func<string, Directory> _factory;
1111

12-
public GenericDirectoryFactory(Func<string, Directory> factory) => _factory = factory;
12+
public GenericDirectoryFactory(Func<string, Directory> factory)
13+
{
14+
_factory = factory;
15+
}
16+
17+
internal GenericDirectoryFactory(Func<string, Directory> factory, bool externallyManaged)
18+
{
19+
_factory = factory;
20+
ExternallyManaged = externallyManaged;
21+
}
22+
23+
/// <summary>
24+
/// When set to true, indicates that the directory is managed externally and will be disposed of by the caller, not the index.
25+
/// </summary>
26+
internal bool ExternallyManaged { get; }
1327

1428
protected override Directory CreateDirectory(LuceneIndex luceneIndex, bool forceUnlock)
1529
{
16-
Directory dir = _factory(luceneIndex.Name);
30+
var dir = _factory(luceneIndex.Name);
1731
if (forceUnlock)
1832
{
1933
IndexWriter.Unlock(dir);

‎src/Examine.Lucene/Directories/DirectoryFactoryBase.cs

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,36 +6,17 @@ namespace Examine.Lucene.Directories
66
{
77
public abstract class DirectoryFactoryBase : IDirectoryFactory
88
{
9-
private readonly ConcurrentDictionary<string, Directory> _createdDirectories = new ConcurrentDictionary<string, Directory>();
10-
private bool _disposedValue;
11-
129
Directory IDirectoryFactory.CreateDirectory(LuceneIndex luceneIndex, bool forceUnlock)
13-
=> _createdDirectories.GetOrAdd(
14-
luceneIndex.Name,
15-
s => CreateDirectory(luceneIndex, forceUnlock));
10+
=> CreateDirectory(luceneIndex, forceUnlock);
1611

1712
protected abstract Directory CreateDirectory(LuceneIndex luceneIndex, bool forceUnlock);
1813

1914
protected virtual void Dispose(bool disposing)
2015
{
21-
if (!_disposedValue)
22-
{
23-
if (disposing)
24-
{
25-
foreach (Directory d in _createdDirectories.Values)
26-
{
27-
d.Dispose();
28-
}
29-
}
30-
31-
_disposedValue = true;
32-
}
3316
}
3417

35-
public void Dispose()
36-
{
18+
public void Dispose() =>
3719
// Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
3820
Dispose(disposing: true);
39-
}
4021
}
4122
}

‎src/Examine.Lucene/Directories/IDirectoryFactory.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace Examine.Lucene.Directories
88
/// Creates a Lucene <see cref="Lucene.Net.Store.Directory"/> for an index
99
/// </summary>
1010
/// <remarks>
11-
/// The directory created must only be created ONCE per index and disposed when the factory is disposed.
11+
/// The directory created must only be created ONCE per index and disposed when the index is disposed.
1212
/// </remarks>
1313
public interface IDirectoryFactory : IDisposable
1414
{

‎src/Examine.Lucene/Directories/SyncedFileSystemDirectory.cs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
using System.IO;
2+
using Examine.Lucene.Providers;
3+
using Lucene.Net.Store;
4+
using Microsoft.Extensions.Logging;
5+
using Directory = Lucene.Net.Store.Directory;
6+
7+
namespace Examine.Lucene.Directories
8+
{
9+
internal class SyncedFileSystemDirectory : FilterDirectory
10+
{
11+
private readonly ExamineReplicator _replicator;
12+
13+
public SyncedFileSystemDirectory(
14+
ILogger<ExamineReplicator> replicatorLogger,
15+
ILogger<LoggingReplicationClient> clientLogger,
16+
Directory localLuceneDirectory,
17+
Directory mainLuceneDirectory,
18+
LuceneIndex luceneIndex,
19+
DirectoryInfo tempDir)
20+
: base(localLuceneDirectory)
21+
{
22+
// now create the replicator that will copy from local to main on schedule
23+
_replicator = new ExamineReplicator(replicatorLogger, clientLogger, luceneIndex, localLuceneDirectory, mainLuceneDirectory, tempDir);
24+
LocalLuceneDirectory = localLuceneDirectory;
25+
MainLuceneDirectory = mainLuceneDirectory;
26+
}
27+
28+
internal Directory LocalLuceneDirectory { get; }
29+
30+
internal Directory MainLuceneDirectory { get; }
31+
32+
public override Lock MakeLock(string name)
33+
{
34+
// Start replicating back to main, this is ok to call multiple times, it will only execute once.
35+
_replicator.StartIndexReplicationOnSchedule(1000);
36+
37+
return base.MakeLock(name);
38+
}
39+
40+
protected override void Dispose(bool disposing)
41+
{
42+
_replicator.Dispose();
43+
MainLuceneDirectory.Dispose();
44+
base.Dispose(disposing);
45+
}
46+
}
47+
}

‎src/Examine.Lucene/Directories/SyncedFileSystemDirectoryFactory.cs

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ public class SyncedFileSystemDirectoryFactory : FileSystemDirectoryFactory
2727
private readonly ILoggerFactory _loggerFactory;
2828
private readonly bool _tryFixMainIndexIfCorrupt;
2929
private readonly ILogger<SyncedFileSystemDirectoryFactory> _logger;
30-
private ExamineReplicator _replicator;
31-
private Directory _mainLuceneDir;
30+
private readonly ILogger<ExamineReplicator> _replicatorLogger;
31+
private readonly ILogger<LoggingReplicationClient> _clientLogger;
3232

3333
[Obsolete("Use ctor with all dependencies")]
3434
public SyncedFileSystemDirectoryFactory(
@@ -75,6 +75,8 @@ public SyncedFileSystemDirectoryFactory(
7575
_loggerFactory = loggerFactory;
7676
_tryFixMainIndexIfCorrupt = tryFixMainIndexIfCorrupt;
7777
_logger = _loggerFactory.CreateLogger<SyncedFileSystemDirectoryFactory>();
78+
_replicatorLogger = _loggerFactory.CreateLogger<ExamineReplicator>();
79+
_clientLogger = _loggerFactory.CreateLogger<LoggingReplicationClient>();
7880
}
7981

8082
internal CreateResult TryCreateDirectory(LuceneIndex luceneIndex, bool forceUnlock, out Directory directory)
@@ -88,24 +90,27 @@ internal CreateResult TryCreateDirectory(LuceneIndex luceneIndex, bool forceUnlo
8890
// used by the replicator, will be a short lived directory for each synced revision and deleted when finished.
8991
var tempDir = new DirectoryInfo(Path.Combine(_localDir.FullName, "Rep", Guid.NewGuid().ToString("N")));
9092

91-
_mainLuceneDir = base.CreateDirectory(luceneIndex, forceUnlock);
93+
var mainLuceneDir = base.CreateDirectory(luceneIndex, forceUnlock);
9294
var localLuceneDir = FSDirectory.Open(
9395
localLuceneIndexFolder,
9496
LockFactory.GetLockFactory(localLuceneIndexFolder));
9597

96-
var mainIndexExists = DirectoryReader.IndexExists(_mainLuceneDir);
98+
var mainIndexExists = DirectoryReader.IndexExists(mainLuceneDir);
9799
var localIndexExists = DirectoryReader.IndexExists(localLuceneDir);
98100

99101
var mainResult = CreateResult.Init;
100102

101103
if (mainIndexExists)
102104
{
103-
mainResult = CheckIndexHealthAndFix(_mainLuceneDir, luceneIndex.Name, _tryFixMainIndexIfCorrupt);
105+
mainResult = CheckIndexHealthAndFix(mainLuceneDir, luceneIndex.Name, _tryFixMainIndexIfCorrupt);
104106
}
105107

106108
// the main index is/was unhealthy or missing, lets check the local index if it exists
107109
if (localIndexExists && (!mainIndexExists || mainResult.HasFlag(CreateResult.NotClean) || mainResult.HasFlag(CreateResult.MissingSegments)))
108110
{
111+
// TODO: add details here and more below too
112+
_logger.LogInformation("");
113+
109114
var localResult = CheckIndexHealthAndFix(localLuceneDir, luceneIndex.Name, false);
110115

111116
if (localResult == CreateResult.Init)
@@ -132,7 +137,7 @@ internal CreateResult TryCreateDirectory(LuceneIndex luceneIndex, bool forceUnlo
132137
? OpenMode.APPEND
133138
: OpenMode.CREATE;
134139

135-
mainResult |= TryGetIndexWriter(openMode, _mainLuceneDir, true, luceneIndex.Name, out var indexWriter);
140+
mainResult |= TryGetIndexWriter(openMode, mainLuceneDir, true, luceneIndex.Name, out var indexWriter);
136141
using (indexWriter)
137142
{
138143
if (!mainResult.HasFlag(CreateResult.SyncedFromLocal))
@@ -142,27 +147,25 @@ internal CreateResult TryCreateDirectory(LuceneIndex luceneIndex, bool forceUnlo
142147
}
143148
}
144149

145-
// now create the replicator that will copy from local to main on schedule
146-
_replicator = new ExamineReplicator(_loggerFactory, luceneIndex, _mainLuceneDir, tempDir);
147-
148150
if (forceUnlock)
149151
{
150152
IndexWriter.Unlock(localLuceneDir);
151153
}
152154

153-
// Start replicating back to main
154-
_replicator.StartIndexReplicationOnSchedule(1000);
155+
Directory luceneDir;
155156

156157
var options = IndexOptions.GetNamedOptions(luceneIndex.Name);
157158
if (options.NrtEnabled)
158159
{
159-
directory = new NRTCachingDirectory(localLuceneDir, options.NrtCacheMaxMergeSizeMB, options.NrtCacheMaxCachedMB);
160+
luceneDir = new NRTCachingDirectory(localLuceneDir, options.NrtCacheMaxMergeSizeMB, options.NrtCacheMaxCachedMB);
160161
}
161162
else
162163
{
163-
directory = localLuceneDir;
164+
luceneDir = localLuceneDir;
164165
}
165166

167+
directory = new SyncedFileSystemDirectory(_replicatorLogger, _clientLogger, luceneDir, mainLuceneDir, luceneIndex, tempDir);
168+
166169
return mainResult;
167170
}
168171

@@ -186,16 +189,6 @@ protected override Directory CreateDirectory(LuceneIndex luceneIndex, bool force
186189
return directory;
187190
}
188191

189-
protected override void Dispose(bool disposing)
190-
{
191-
base.Dispose(disposing);
192-
if (disposing)
193-
{
194-
_replicator?.Dispose();
195-
_mainLuceneDir?.Dispose();
196-
}
197-
}
198-
199192
private CreateResult TryGetIndexWriter(
200193
OpenMode openMode,
201194
Directory luceneDirectory,
@@ -228,6 +221,8 @@ private CreateResult TryGetIndexWriter(
228221
// Index is corrupted, typically this will be FileNotFoundException or CorruptIndexException
229222
_logger.LogError(ex, "{IndexName} index is corrupt, a new one will be created", indexName);
230223

224+
// TODO: Here I think we need to totally clear all files in the directory
225+
231226
indexWriter = GetIndexWriter(luceneDirectory, OpenMode.CREATE);
232227
}
233228
else
@@ -252,7 +247,7 @@ private void SyncIndex(IndexWriter sourceIndexWriter, bool forceUnlock, string i
252247

253248
using (var sourceIndex = new LuceneIndex(_loggerFactory, indexName, new TempOptions(), sourceIndexWriter))
254249
using (var destinationLuceneDirectory = FSDirectory.Open(destinationDirectory, LockFactory.GetLockFactory(destinationDirectory)))
255-
using (var replicator = new ExamineReplicator(_loggerFactory, sourceIndex, destinationLuceneDirectory, tempDir))
250+
using (var replicator = new ExamineReplicator(_replicatorLogger, _clientLogger, sourceIndex, sourceIndexWriter.Directory, destinationLuceneDirectory, tempDir))
256251
{
257252
if (forceUnlock)
258253
{
@@ -328,7 +323,7 @@ private CreateResult CheckIndexHealthAndFix(
328323
return result;
329324
}
330325

331-
private IndexWriter GetIndexWriter(Directory mainDir, OpenMode openMode)
326+
private static IndexWriter GetIndexWriter(Directory mainDir, OpenMode openMode)
332327
{
333328
var indexWriter = new IndexWriter(
334329
mainDir,

0 commit comments

Comments
 (0)