Skip to content

Commit

Permalink
improve latest snapshot query (#12)
Browse files Browse the repository at this point in the history
* introduce performance regression tests

* ensure the harmony table names are consistent now that we have a hardcoded sql query

* add test to ensure changes are round tripped through the db properly

* Fix bug where changes outside the model were saved in snapshots, covered with `ChangesToSnapshotsAreNotSaved`. Fixing that uncovered tests that depend on snapshots being tracked between test phases. Also fixed tests which were over-specifying the expected time when there was a mismatch in time offset that was acceptable.

* add HybridDateTimeTests.cs

* Define interface to describe Changes return value, should help APIs implement the matching return type without restricting the collection type. Split `GetChanges` helper to `GetMissingCommits` which returns an IAsyncEnumerable, this allows the server to use less memory when responding to requests with a large number of commits

* introduce test to ensure that changes can still be written after an application restart

* fix bug where you couldn't submit updates to an existing entity after creating a new data model instance.
  • Loading branch information
hahn-kev authored Oct 7, 2024
1 parent 803b3bd commit 0340cd4
Show file tree
Hide file tree
Showing 20 changed files with 592 additions and 96 deletions.
43 changes: 29 additions & 14 deletions src/SIL.Harmony.Core/QueryHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,34 +13,49 @@ public static async Task<SyncState> GetSyncState(this IQueryable<CommitBase> com
return new SyncState(dict);
}

public static async Task<ChangesResult<TCommit>> GetChanges<TCommit, TChange>(this IQueryable<TCommit> commits, SyncState remoteState) where TCommit : CommitBase<TChange>
public static async Task<ChangesResult<TCommit>> GetChanges<TCommit, TChange>(this IQueryable<TCommit> commits,
SyncState remoteState) where TCommit : CommitBase<TChange>
{
var newHistory = new List<TCommit>();
var localSyncState = await commits.GetSyncState();
foreach (var (clientId, localTimestamp) in localSyncState.ClientHeads)
var localState = await commits.AsNoTracking().GetSyncState();
return new ChangesResult<TCommit>(
await GetMissingCommits<TCommit, TChange>(commits, localState, remoteState).ToArrayAsync(),
localState);
}

public static async IAsyncEnumerable<TCommit> GetMissingCommits<TCommit, TChange>(
this IQueryable<TCommit> commits,
SyncState localState,
SyncState remoteState) where TCommit : CommitBase<TChange>
{
commits = commits.AsNoTracking();
foreach (var (clientId, localTimestamp) in localState.ClientHeads)
{
//client is new to the other history
if (!remoteState.ClientHeads.TryGetValue(clientId, out var otherTimestamp))
{
//todo slow, it would be better if we could query on client id and get latest changes per client
newHistory.AddRange(await commits.Include(c => c.ChangeEntities).DefaultOrder()
.Where(c => c.ClientId == clientId)
.ToArrayAsync());
await foreach (var commit in commits.Include(c => c.ChangeEntities).DefaultOrder()
.Where(c => c.ClientId == clientId)
.AsAsyncEnumerable())
{
yield return commit;
}
}
//client has newer history than the other history
else if (localTimestamp > otherTimestamp)
{
var otherDt = DateTimeOffset.FromUnixTimeMilliseconds(otherTimestamp);
//todo even slower because we need to filter out changes that are already in the other history
newHistory.AddRange((await commits.Include(c => c.ChangeEntities).DefaultOrder()
.Where(c => c.ClientId == clientId && c.HybridDateTime.DateTime > otherDt)
.ToArrayAsync())
//fixes an issue where the query would include commits that are already in the other history
.Where(c => c.DateTime.ToUnixTimeMilliseconds() > otherTimestamp));
await foreach (var commit in commits.Include(c => c.ChangeEntities)
.DefaultOrder()
.Where(c => c.ClientId == clientId && c.HybridDateTime.DateTime > otherDt)
.AsAsyncEnumerable())
{
if (commit.DateTime.ToUnixTimeMilliseconds() > otherTimestamp)
yield return commit;
}
}
}

return new(newHistory.ToArray(), localSyncState);
}

public static IQueryable<T> DefaultOrder<T>(this IQueryable<T> queryable) where T: CommitBase
Expand Down
9 changes: 7 additions & 2 deletions src/SIL.Harmony.Core/SyncState.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
namespace SIL.Harmony.Core;

public record SyncState(Dictionary<Guid, long> ClientHeads);

public record ChangesResult<TCommit>(TCommit[] MissingFromClient, SyncState ServerSyncState) where TCommit : CommitBase
public interface IChangesResult
{
IEnumerable<CommitBase> MissingFromClient { get; }
SyncState ServerSyncState { get; }
}
public record ChangesResult<TCommit>(TCommit[] MissingFromClient, SyncState ServerSyncState): IChangesResult where TCommit : CommitBase
{
IEnumerable<CommitBase> IChangesResult.MissingFromClient => MissingFromClient;
public static ChangesResult<TCommit> Empty => new([], new SyncState([]));
}
15 changes: 13 additions & 2 deletions src/SIL.Harmony.Sample/CrdtSampleKernel.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Diagnostics;
using System.Data.Common;
using System.Diagnostics;
using SIL.Harmony.Changes;
using SIL.Harmony.Linq2db;
using SIL.Harmony.Sample.Changes;
Expand All @@ -11,11 +12,21 @@ namespace SIL.Harmony.Sample;
public static class CrdtSampleKernel
{
public static IServiceCollection AddCrdtDataSample(this IServiceCollection services, string dbPath)
{
return services.AddCrdtDataSample(builder => builder.UseSqlite($"Data Source={dbPath}"));
}
public static IServiceCollection AddCrdtDataSample(this IServiceCollection services, DbConnection connection)
{
return services.AddCrdtDataSample(builder => builder.UseSqlite(connection, true));
}

public static IServiceCollection AddCrdtDataSample(this IServiceCollection services,
Action<DbContextOptionsBuilder> optionsBuilder)
{
services.AddDbContext<SampleDbContext>((provider, builder) =>
{
builder.UseLinqToDbCrdt(provider);
builder.UseSqlite($"Data Source={dbPath}");
optionsBuilder(builder);
builder.EnableDetailedErrors();
builder.EnableSensitiveDataLogging();
#if DEBUG
Expand Down
77 changes: 77 additions & 0 deletions src/SIL.Harmony.Tests/Core/HybridDateTimeTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
using SIL.Harmony.Core;

namespace SIL.Harmony.Tests.Core;

public class HybridDateTimeTests
{
[Fact]
public void Equals_TrueWhenTheSame()
{
var dateTime = new HybridDateTime(new DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero), 0);
var otherDateTime = new HybridDateTime(new DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero), 0);

(dateTime == otherDateTime).Should().BeTrue();
}

[Fact]
public void Equals_FalseWhenDifferentDateTime()
{
var dateTime = new HybridDateTime(new DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero), 0);
var otherDateTime = new HybridDateTime(new DateTimeOffset(2001, 1, 1, 0, 0, 0, TimeSpan.Zero), 0);

(dateTime != otherDateTime).Should().BeTrue();
}

[Fact]
public void Equals_FalseWhenDifferentCounter()
{
var dateTime = new HybridDateTime(new DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero), 0);
var otherDateTime = new HybridDateTime(new DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero), 1);

dateTime.Should().NotBe(otherDateTime);
}

[Fact]
public void Constructor_ThrowsArgumentOutOfRangeExceptionWhenCounterIsNegative()
{
Action action = () => new HybridDateTime(new DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero), -1);
action.Should().Throw<ArgumentOutOfRangeException>();
}

[Fact]
public void CompareTo_ReturnsOneWhenOtherIsNull()
{
var dateTime = new HybridDateTime(new DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero), 0);
dateTime.CompareTo(null).Should().Be(1);
}

[Fact]
public void CompareTo_ReturnsNegativeOneWhenThisIsLessThanOther()
{
var dateTime = new HybridDateTime(new DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero), 0);
var otherDateTime = new HybridDateTime(new DateTimeOffset(2000, 1, 2, 0, 0, 0, TimeSpan.Zero), 0);

var result = dateTime.CompareTo(otherDateTime);
result.Should().BeLessThan(0);
}

[Fact]
public void CompareTo_ReturnsZeroWhenThisIsEqualToOther()
{
var dateTime = new HybridDateTime(new DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero), 0);
var otherDateTime = new HybridDateTime(new DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero), 0);

var result = dateTime.CompareTo(otherDateTime);
result.Should().Be(0);
}

[Fact]
public void CompareTo_ReturnsOneWhenThisIsGreaterThanOther()
{
var dateTime = new HybridDateTime(new DateTimeOffset(2000, 1, 2, 0, 0, 0, TimeSpan.Zero), 0);
var otherDateTime = new HybridDateTime(new DateTimeOffset(2000, 1, 1, 0, 0, 0, TimeSpan.Zero), 0);

var result = dateTime.CompareTo(otherDateTime);
result.Should().Be(1);
}
}
Loading

0 comments on commit 0340cd4

Please sign in to comment.