Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TEST OF REVIEW #11

Merged
merged 6 commits into from
Nov 7, 2024
Merged

TEST OF REVIEW #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9703222
Lease manager.
May 27, 2024
1f62404
try ensuring that files are deleted.
Jun 26, 2024
e30c918
park
jeme Sep 18, 2024
9d2cf5e
Merge branch 'issues/GH-6' of https://github.com/dotJEM/json-index2 i…
jeme Sep 18, 2024
898da46
update packages.
Nov 7, 2024
1e59c7f
Merge branch 'issues/GH-6' of https://github.com/dotJEM/json-index2 i…
Nov 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/DotJEM.Json.Index2.Contexts.Test/DotJEM.Json.Index2.Contexts.Test.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Update="DotJEM.ObservableExtensions" Version="0.0.14" />
<PackageReference Update="DotJEM.ObservableExtensions" Version="0.0.15" />
</ItemGroup>

</Project>
2 changes: 1 addition & 1 deletion src/DotJEM.Json.Index2.Contexts/DotJEM.Json.Index2.Contexts.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Update="DotJEM.ObservableExtensions" Version="0.0.14" />
<PackageReference Update="DotJEM.ObservableExtensions" Version="0.0.15" />
</ItemGroup>

</Project>
4 changes: 2 additions & 2 deletions src/DotJEM.Json.Index2.Management/DotJEM.Json.Index2.Management.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="DotJEM.Web.Scheduler" Version="0.0.10" />
<PackageReference Include="DotJEM.Web.Scheduler" Version="0.0.11" />
</ItemGroup>

<ItemGroup>
Expand All @@ -16,7 +16,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Update="DotJEM.ObservableExtensions" Version="0.0.14" />
<PackageReference Update="DotJEM.ObservableExtensions" Version="0.0.15" />
</ItemGroup>

</Project>
5 changes: 0 additions & 5 deletions src/DotJEM.Json.Index2.Management/IJsonIndexManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,6 @@ public async Task ResetIndexAsync()
{
await jsonDocumentSource.StopAsync().ConfigureAwait(false);
index.Storage.Delete();
//TODO: Force a commit after delete to see if that helps?
using (var lease = index.WriterManager.Lease())
{
lease.Value.Commit();
}
await jsonDocumentSource.ResetAsync().ConfigureAwait(false);
await jsonDocumentSource.StartAsync().ConfigureAwait(false);
}
Expand Down
3 changes: 3 additions & 0 deletions src/DotJEM.Json.Index2.Management/Snapshots/IJsonIndexSnapshotManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using DotJEM.Json.Index2.Snapshots;
using DotJEM.ObservableExtensions.InfoStreams;
using DotJEM.Web.Scheduler;
using Lucene.Net.Search;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;

Expand Down Expand Up @@ -41,6 +42,8 @@ public class JsonIndexSnapshotManager : IJsonIndexSnapshotManager
{
private readonly IJsonIndex index;
private readonly ISnapshotStrategy strategy;
//TODO: This binds us to a concrete idea (even though this is an interface).
// instead it would be preferred if we could provide an abstraction for what is needed.
Comment on lines +45 to +46
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address the design concern by refactoring the code to depend on an abstraction.

The TODO comment correctly identifies a design issue where the JsonIndexSnapshotManager class is tightly coupled to a concrete implementation of the snapshot strategy. This makes the code less flexible and harder to extend or modify.

Consider refactoring the code to depend on an interface instead of a concrete implementation. This aligns with the Dependency Inversion Principle (DIP) of SOLID and would make the code more flexible and easier to test.

Here's a high-level overview of the refactoring steps:

  1. Define an interface that encapsulates the behavior of the snapshot strategy. For example:
public interface ISnapshotStrategy
{
    IInfoStream InfoStream { get; }
    ISnapshotStorage Storage { get; }
    void CleanOldSnapshots();
    // Other methods as needed
}
  1. Update the JsonIndexSnapshotManager constructor to depend on the ISnapshotStrategy interface instead of a concrete implementation:
-public JsonIndexSnapshotManager(IJsonIndex index, ISnapshotStrategy snapshotStrategy, IWebTaskScheduler scheduler, string schedule)
+public JsonIndexSnapshotManager(IJsonIndex index, ISnapshotStrategy snapshotStrategy, IWebTaskScheduler scheduler, string schedule)
{
    this.index = index;
    this.scheduler = scheduler;
    this.schedule = schedule;
-   this.strategy = snapshotStrategy;
+   this.strategy = snapshotStrategy;
    this.strategy.InfoStream.Subscribe(infoStream);
}
  1. Update the code that creates an instance of JsonIndexSnapshotManager to pass in an implementation of ISnapshotStrategy.

This refactoring would decouple the JsonIndexSnapshotManager from a specific snapshot strategy implementation, making it more flexible and easier to extend or modify in the future.

private readonly IWebTaskScheduler scheduler;
private readonly IInfoStream<JsonIndexSnapshotManager> infoStream = new InfoStream<JsonIndexSnapshotManager>();

Expand Down
2 changes: 2 additions & 0 deletions src/DotJEM.Json.Index2.Management/Writer/IJsonIndexWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using DotJEM.Json.Index2.Documents;
using DotJEM.Json.Index2.Documents.Info;
using DotJEM.Json.Index2.IO;
using DotJEM.Json.Index2.Leases;
using DotJEM.ObservableExtensions.InfoStreams;
using Lucene.Net.Index;
using Newtonsoft.Json.Linq;
Expand Down Expand Up @@ -154,6 +155,7 @@ private void Commit()
try
{
lease.Value.Commit();

}
catch (Exception e)
{
Expand Down
4 changes: 2 additions & 2 deletions src/DotJEM.Json.Index2.QueryParsers.Test/DotJEM.Json.Index2.QueryParsers.Test.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

<ItemGroup>
<PackageReference Include="Bogus" Version="35.5.1" />
<PackageReference Include="Lucene.Net.QueryParser" Version="4.8.0-beta00016" />
<PackageReference Include="Lucene.Net.QueryParser" Version="4.8.0-beta00017" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
<PackageReference Include="NUnit" Version="3.14.0" />
</ItemGroup>
Expand All @@ -24,7 +24,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Update="DotJEM.ObservableExtensions" Version="0.0.14" />
<PackageReference Update="DotJEM.ObservableExtensions" Version="0.0.15" />
</ItemGroup>


Expand Down
2 changes: 1 addition & 1 deletion src/DotJEM.Json.Index2.QueryParsers/DotJEM.Json.Index2.QueryParsers.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Update="DotJEM.ObservableExtensions" Version="0.0.14" />
<PackageReference Update="DotJEM.ObservableExtensions" Version="0.0.15" />
</ItemGroup>

</Project>
2 changes: 1 addition & 1 deletion src/DotJEM.Json.Index2.Snapshots/DotJEM.Json.Index2.Snapshots.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Update="DotJEM.ObservableExtensions" Version="0.0.14" />
<PackageReference Update="DotJEM.ObservableExtensions" Version="0.0.15" />
</ItemGroup>

</Project>
1 change: 1 addition & 0 deletions src/DotJEM.Json.Index2.Snapshots/IndexSnapshotHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using DotJEM.Json.Index2.IO;
using DotJEM.Json.Index2.Leases;
using DotJEM.Json.Index2.Snapshots.Streams;
using Lucene.Net.Index;
using Lucene.Net.Store;
Expand Down
2 changes: 1 addition & 1 deletion src/DotJEM.Json.Index2.Test/DotJEM.Json.Index2.Test.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
</ItemGroup>

<ItemGroup>
<PackageReference Update="DotJEM.ObservableExtensions" Version="0.0.14" />
<PackageReference Update="DotJEM.ObservableExtensions" Version="0.0.15" />
</ItemGroup>

</Project>
23 changes: 23 additions & 0 deletions src/DotJEM.Json.Index2.Test/JsonIndexTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public async Task Create_AddsDocument()
//int count = searcher.Search(new MatchAllDocsQuery()).Count();
Assert.AreEqual(5, count);
}

[Test]
public async Task SayHello_ReturnsHello()
{
Expand Down Expand Up @@ -83,4 +84,26 @@ public async Task Search_Booleans()
int count = searcher.Search(new TermQuery(new Term("inStock", "true"))).Count();
Assert.AreEqual(3, count);
}
[Test]
public async Task FindBeforeCommit_AddsDocument()
{
IJsonIndex index = new JsonIndexBuilder("myIndex")
.UsingMemmoryStorage()
.WithAnalyzer(cfg => new StandardAnalyzer(cfg.Version))
.WithFieldResolver(new FieldResolver("uuid", "type"))
.Build();

IJsonIndexWriter writer = index.CreateWriter();
writer.Create(JObject.FromObject(new { uuid = Guid.NewGuid(), type = "CAR" }));
writer.Create(JObject.FromObject(new { uuid = Guid.NewGuid(), type = "CAR" }));
writer.Create(JObject.FromObject(new { uuid = Guid.NewGuid(), type = "CAR" }));
writer.Create(JObject.FromObject(new { uuid = Guid.NewGuid(), type = "CAR" }));
writer.Create(JObject.FromObject(new { uuid = Guid.NewGuid(), type = "CAR" }));

IJsonIndexSearcher? searcher = index.CreateSearcher();
Assert.That(searcher.Search(new TermQuery(new Term("type", "car"))).Count(), Is.EqualTo(5));

writer.Create(JObject.FromObject(new { uuid = Guid.NewGuid(), type = "CAR" }));
Assert.That(searcher.Search(new TermQuery(new Term("type", "car"))).Count(), Is.EqualTo(6));
}
}
6 changes: 3 additions & 3 deletions src/DotJEM.Json.Index2/DotJEM.Json.Index2.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@

<ItemGroup>
<PackageReference Include="DotJEM.Json.Visitor" Version="0.1.35" />
<PackageReference Include="DotJEM.ObservableExtensions" Version="0.0.14" />
<PackageReference Include="Lucene.Net" Version="4.8.0-beta00016" />
<PackageReference Include="Lucene.Net.Analysis.Common" Version="4.8.0-beta00016" />
<PackageReference Include="DotJEM.ObservableExtensions" Version="0.0.15" />
<PackageReference Include="Lucene.Net" Version="4.8.0-beta00017" />
<PackageReference Include="Lucene.Net.Analysis.Common" Version="4.8.0-beta00017" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
</ItemGroup>

Expand Down
1 change: 1 addition & 0 deletions src/DotJEM.Json.Index2/IO/JsonIndexWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Collections.Generic;
using System.Linq;
using DotJEM.Json.Index2.Documents;
using DotJEM.Json.Index2.Leases;
using DotJEM.Json.Index2.Util;
using Lucene.Net.Documents;
using Lucene.Net.Index;
Expand Down
130 changes: 15 additions & 115 deletions src/DotJEM.Json.Index2/IO/JsonIndexWriterManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,24 @@
using System.Diagnostics;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using DotJEM.Json.Index2.Configuration;
using DotJEM.Json.Index2.Leases;
using DotJEM.Json.Index2.Util;
using Lucene.Net.Analysis;
using Lucene.Net.Index;
using Lucene.Net.Search;

namespace DotJEM.Json.Index2.IO;

public interface IIndexWriterManager : IDisposable
{
event EventHandler<EventArgs> OnClose;

ILease<IndexWriter> Lease();
void Close();
}

public interface ILease<out T> : IDisposable
{
T Value { get; }
bool IsExpired { get; }
}

public class IndexWriterManager : Disposable, IIndexWriterManager
{
Expand All @@ -31,8 +30,9 @@ public class IndexWriterManager : Disposable, IIndexWriterManager
private readonly IJsonIndex index;
private volatile IndexWriter writer;
private readonly object writerPadLock = new();
private readonly object leasesPadLock = new();
private readonly LeaseManager<IndexWriter> leaseManager = new();

//TODO: With leases, this should not be needed.
public event EventHandler<EventArgs> OnClose;

private IndexWriter Writer
Expand All @@ -47,40 +47,13 @@ private IndexWriter Writer
if (writer != null)
return writer;

try
{
return writer = Open(index);
}
catch (Exception e)
{
Debug.WriteLine("ACTIVE LEASES: " +leases.Count);
throw;
}
return writer = Open(index);
}
}
}

private readonly List<TimeLimitedIndexWriterLease> leases = new List<TimeLimitedIndexWriterLease>();

public ILease<IndexWriter> Lease()
{
//TODO: Optimizied collection for this.
TimeLimitedIndexWriterLease lease = new(this, OnReturned);
lock (leasesPadLock)
{
leases.Add(lease);
}
return lease;
}

private void OnReturned(TimeLimitedIndexWriterLease lease)
{
lock (leasesPadLock)
{
leases.Remove(lease);
}
}

public ILease<IndexWriter> Lease() => leaseManager.Create(Writer, TimeSpan.FromSeconds(3));

public IndexWriterManager(IJsonIndex index)
{
Expand All @@ -89,7 +62,6 @@ public IndexWriterManager(IJsonIndex index)

private static IndexWriter Open(IJsonIndex index)
{
Debug.WriteLine("OPEN WRITER");
IndexWriterConfig config = new(index.Configuration.Version, index.Configuration.Analyzer);
config.RAMBufferSizeMB = DEFAULT_RAM_BUFFER_SIZE_MB;
config.OpenMode = OpenMode.CREATE_OR_APPEND;
Expand All @@ -103,28 +75,15 @@ public void Close()
if (writer == null)
return;

lock (leasesPadLock)
lock (writerPadLock)
{
TimeLimitedIndexWriterLease[] leasesCopy = leases.ToArray();

Debug.WriteLine("ACTIVE LEASES: " + leasesCopy.Length);
leases.Clear();
foreach (TimeLimitedIndexWriterLease lease in leasesCopy)
{
if (!lease.IsExpired)
lease.Wait();
lease.Dispose();
}

lock (writerPadLock)
{
if (writer == null)
return;
leaseManager.RecallAll();
if (writer == null)
return;

writer.Dispose();
writer = null;
RaiseOnClose();
}
writer.Dispose();
writer = null;
RaiseOnClose();
}
}

Expand All @@ -140,63 +99,4 @@ protected virtual void RaiseOnClose()
{
OnClose?.Invoke(this, EventArgs.Empty);
}

private class TimeLimitedIndexWriterLease : Disposable, ILease<IndexWriter>
{
private readonly DateTime leaseTime = DateTime.Now;
private readonly Action<TimeLimitedIndexWriterLease> onReturned;
private readonly IndexWriterManager manager;
public AutoResetEvent Handle { get; } = new(false);

public IndexWriter Value
{
get
{
if (IsDisposed)
{
throw new ObjectDisposedException("Index writer lease has been returned or is expired.");
}

if (IsExpired)
{
throw new LeaseExpiredException("Index writer lease has been returned or is expired.");
}
return manager.Writer;
}
}

public bool IsExpired => (DateTime.Now - leaseTime > TimeSpan.FromSeconds(5)) || IsDisposed;

public TimeLimitedIndexWriterLease(IndexWriterManager manager, Action<TimeLimitedIndexWriterLease> onReturned)
{
this.onReturned = onReturned;
this.manager = manager;
}

protected override void Dispose(bool disposing)
{
Handle.Set();
onReturned(this);
}

public void Wait()
{
if (IsExpired)
return;

Handle.WaitOne(TimeSpan.FromSeconds(6) - (DateTime.Now - leaseTime));
}
}

}

public class LeaseExpiredException : Exception
{
public LeaseExpiredException(string message) : base(message)
{
}

public LeaseExpiredException(string message, Exception innerException) : base(message, innerException)
{
}
}
Loading
Loading