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.

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

Akka.Persistence: Made DateTime.UtcNow the default timestamp for SnapshotMetdata #7313

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,8 @@ namespace Akka.Persistence
}
public sealed class SnapshotMetadata : System.IEquatable<Akka.Persistence.SnapshotMetadata>
{
public static System.DateTime TimestampNotSpecified;
[System.ObsoleteAttribute("This constructor is deprecated and will be removed in v1.6. Use the constructor w" +
Copy link
Member Author

Choose a reason for hiding this comment

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

Just marked the old CTOR as Obsolete - no other API changes.

"ith the timestamp parameter instead.", true)]
public SnapshotMetadata(string persistenceId, long sequenceNr) { }
[Newtonsoft.Json.JsonConstructorAttribute()]
public SnapshotMetadata(string persistenceId, long sequenceNr, System.DateTime timestamp) { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,8 @@ namespace Akka.Persistence
}
public sealed class SnapshotMetadata : System.IEquatable<Akka.Persistence.SnapshotMetadata>
{
public static System.DateTime TimestampNotSpecified;
[System.ObsoleteAttribute("This constructor is deprecated and will be removed in v1.6. Use the constructor w" +
Copy link
Contributor

Choose a reason for hiding this comment

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

A little nitpick, Would be better if we follow english language sentence breaking point here, makes it easier to read.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point - I can fix that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, no I can't - that's an artifact from the rendering of the API diff. The real code looks like this:

[Obsolete("This constructor is deprecated and will be removed in v1.6. Use the constructor with the timestamp parameter instead.", true)]

"ith the timestamp parameter instead.", true)]
public SnapshotMetadata(string persistenceId, long sequenceNr) { }
[Newtonsoft.Json.JsonConstructorAttribute()]
public SnapshotMetadata(string persistenceId, long sequenceNr, System.DateTime timestamp) { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ protected override void AfterAll()
public void LocalSnapshotStore_can_snapshot_actors_with_PersistenceId_containing_invalid_path_characters()
{
var pid = @"p\/:*?-1";
SnapshotStore.Tell(new SaveSnapshot(new SnapshotMetadata(pid, 1), "sample data"), TestActor);
SnapshotStore.Tell(new SaveSnapshot(new SnapshotMetadata(pid, 1, DateTime.UtcNow), "sample data"), TestActor);
ExpectMsg<SaveSnapshotSuccess>();

SnapshotStore.Tell(new LoadSnapshot(pid, SnapshotSelectionCriteria.Latest, long.MaxValue), TestActor);
Expand Down
2 changes: 1 addition & 1 deletion src/core/Akka.Persistence.TCK/Query/PersistenceIdsSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ protected IActorRef WriteSnapshot(string persistenceId, int n)
ExpectMsg($"{persistenceId}-{i}-done");
}

var metadata = new SnapshotMetadata(persistenceId, n + 10);
var metadata = new SnapshotMetadata(persistenceId, n + 10, Sys.Scheduler.Now.DateTime);
Copy link
Member Author

Choose a reason for hiding this comment

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

Standard call we're going to use to get the current UTC time from the ITimeProvider built into the IScheduler.

SnapshotStore.Tell(new SaveSnapshot(metadata, $"s-{n}"), _senderProbe.Ref);
_senderProbe.ExpectMsg<SaveSnapshotSuccess>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public virtual void SnapshotStore_should_serialize_Payload()

var snapshot = new Test.MySnapshot("a");

var metadata = new SnapshotMetadata(Pid, 1);
var metadata = new SnapshotMetadata(Pid, 1, Sys.Scheduler.Now.DateTime);
SnapshotStore.Tell(new SaveSnapshot(metadata, snapshot), probe.Ref);
probe.ExpectMsg<SaveSnapshotSuccess>();

Expand All @@ -85,7 +85,7 @@ public virtual void SnapshotStore_should_serialize_Payload_with_string_manifest(

var snapshot = new Test.MySnapshot2("a");

var metadata = new SnapshotMetadata(Pid, 1);
var metadata = new SnapshotMetadata(Pid, 1, Sys.Scheduler.Now.DateTime);
SnapshotStore.Tell(new SaveSnapshot(metadata, snapshot), probe.Ref);
probe.ExpectMsg<SaveSnapshotSuccess>();

Expand All @@ -107,7 +107,7 @@ public virtual void SnapshotStore_should_serialize_AtLeastOnceDeliverySnapshot()
};
var atLeastOnceDeliverySnapshot = new AtLeastOnceDeliverySnapshot(17, unconfirmed);

var metadata = new SnapshotMetadata(Pid, 2);
var metadata = new SnapshotMetadata(Pid, 2, Sys.Scheduler.Now.DateTime);
SnapshotStore.Tell(new SaveSnapshot(metadata, atLeastOnceDeliverySnapshot), probe.Ref);
probe.ExpectMsg<SaveSnapshotSuccess>();

Expand All @@ -123,7 +123,7 @@ public virtual void SnapshotStore_should_serialize_AtLeastOnceDeliverySnapshot_w
var unconfirmed = Array.Empty<UnconfirmedDelivery>();
var atLeastOnceDeliverySnapshot = new AtLeastOnceDeliverySnapshot(13, unconfirmed);

var metadata = new SnapshotMetadata(Pid, 2);
var metadata = new SnapshotMetadata(Pid, 2, Sys.Scheduler.Now.DateTime);
SnapshotStore.Tell(new SaveSnapshot(metadata, atLeastOnceDeliverySnapshot), probe.Ref);
probe.ExpectMsg<SaveSnapshotSuccess>();

Expand All @@ -138,7 +138,7 @@ public virtual void SnapshotStore_should_serialize_PersistentFSMSnapshot()

var persistentFSMSnapshot = new PersistentFSM.PersistentFSMSnapshot<string>("mystate", "mydata", TimeSpan.FromDays(4));

var metadata = new SnapshotMetadata(Pid, 2);
var metadata = new SnapshotMetadata(Pid, 2, Sys.Scheduler.Now.DateTime);
SnapshotStore.Tell(new SaveSnapshot(metadata, persistentFSMSnapshot), probe.Ref);
probe.ExpectMsg<SaveSnapshotSuccess>();

Expand Down
8 changes: 4 additions & 4 deletions src/core/Akka.Persistence.TCK/Snapshot/SnapshotStoreSpec.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ private IEnumerable<SnapshotMetadata> WriteSnapshots()
{
for (int i = 1; i <= 5; i++)
{
var metadata = new SnapshotMetadata(Pid, i + 10);
var metadata = new SnapshotMetadata(Pid, i + 10, Sys.Scheduler.Now.DateTime);
SnapshotStore.Tell(new SaveSnapshot(metadata, $"s-{i}"), _senderProbe.Ref);
yield return _senderProbe.ExpectMsg<SaveSnapshotSuccess>().Metadata;
}
Expand Down Expand Up @@ -181,7 +181,7 @@ public virtual void SnapshotStore_should_load_the_most_recent_snapshot_matching_
public virtual void SnapshotStore_should_delete_a_single_snapshot_identified_by_SequenceNr_in_snapshot_metadata()
{
var md = Metadata[2];
md = new SnapshotMetadata(md.PersistenceId, md.SequenceNr); // don't care about timestamp for delete of a single snap
md = new SnapshotMetadata(md.PersistenceId, md.SequenceNr, Sys.Scheduler.Now.DateTime); // don't care about timestamp for delete of a single snap
var command = new DeleteSnapshot(md);
var sub = CreateTestProbe();

Expand Down Expand Up @@ -260,7 +260,7 @@ public virtual void SnapshotStore_should_save_and_overwrite_snapshot_with_same_s
[Fact]
public virtual void SnapshotStore_should_save_bigger_size_snapshot()
{
var metadata = new SnapshotMetadata(Pid, 100);
var metadata = new SnapshotMetadata(Pid, 100, Sys.Scheduler.Now.DateTime);
var bigSnapshot = new byte[SnapshotByteSizeLimit];
new Random().NextBytes(bigSnapshot);
SnapshotStore.Tell(new SaveSnapshot(metadata, bigSnapshot), _senderProbe.Ref);
Expand All @@ -274,7 +274,7 @@ public virtual void ShouldSerializeSnapshots()
if (!SupportsSerialization) return;

var probe = CreateTestProbe();
var metadata = new SnapshotMetadata(Pid, 100L);
var metadata = new SnapshotMetadata(Pid, 100L, Sys.Scheduler.Now.DateTime);
var snap = new TestPayload(probe.Ref);

SnapshotStore.Tell(new SaveSnapshot(metadata, snap), _senderProbe.Ref);
Expand Down
4 changes: 2 additions & 2 deletions src/core/Akka.Persistence/Eventsourced.cs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public void LoadSnapshot(string persistenceId, SnapshotSelectionCriteria criteri
/// <param name="snapshot">TBD</param>
public void SaveSnapshot(object snapshot)
{
SnapshotStore.Tell(new SaveSnapshot(new SnapshotMetadata(SnapshotterId, SnapshotSequenceNr), snapshot));
SnapshotStore.Tell(new SaveSnapshot(new SnapshotMetadata(SnapshotterId, SnapshotSequenceNr, Context.System.Scheduler.Now.Date), snapshot));
Copy link
Member Author

Choose a reason for hiding this comment

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

Using the same standard call now for both saving and deleting snapshots - big question I have though: does this break deleting snapshots? It SHOULDN'T, but we'll need to see how the specs perform to know for certain.

}

/// <summary>
Expand All @@ -230,7 +230,7 @@ public void SaveSnapshot(object snapshot)
/// <param name="sequenceNr">TBD</param>
public void DeleteSnapshot(long sequenceNr)
{
SnapshotStore.Tell(new DeleteSnapshot(new SnapshotMetadata(SnapshotterId, sequenceNr)));
SnapshotStore.Tell(new DeleteSnapshot(new SnapshotMetadata(SnapshotterId, sequenceNr, DateTime.MinValue)));
}

/// <summary>
Expand Down
22 changes: 12 additions & 10 deletions src/core/Akka.Persistence/SnapshotProtocol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,32 +52,28 @@ public int Compare(SnapshotMetadata x, SnapshotMetadata y)
}

/// <summary>
/// TBD
/// The singleton comparer instance.
/// </summary>
public static IComparer<SnapshotMetadata> Comparer { get; } = new SnapshotMetadataComparer();

/// <summary>
/// TBD
/// </summary>
public static DateTime TimestampNotSpecified = DateTime.MinValue;

/// <summary>
/// Initializes a new instance of the <see cref="SnapshotMetadata"/> class.
/// </summary>
/// <param name="persistenceId">The id of the persistent actor fro which the snapshot was taken.</param>
/// <param name="sequenceNr">The sequence number at which the snapshot was taken.</param>
[Obsolete("This constructor is deprecated and will be removed in v1.6. Use the constructor with the timestamp parameter instead.", true)]
public SnapshotMetadata(string persistenceId, long sequenceNr)
: this(persistenceId, sequenceNr, TimestampNotSpecified)
: this(persistenceId, sequenceNr, DateTime.UtcNow)
Arkatufus marked this conversation as resolved.
Show resolved Hide resolved
{
}

/// <summary>
/// Initializes a new instance of the <see cref="SnapshotMetadata"/> class.
/// </summary>
/// <param name="persistenceId">The id of the persistent actor fro mwhich the snapshot was taken.</param>
/// <param name="persistenceId">The id of the persistent actor from which the snapshot was taken.</param>
/// <param name="sequenceNr">The sequence number at which the snapshot was taken.</param>
/// <param name="timestamp">The time at which the snapshot was saved.</param>
[JsonConstructor]
[JsonConstructor] // TODO: remove this
public SnapshotMetadata(string persistenceId, long sequenceNr, DateTime timestamp)
{
PersistenceId = persistenceId;
Expand All @@ -100,7 +96,13 @@ public SnapshotMetadata(string persistenceId, long sequenceNr, DateTime timestam
/// </summary>
public DateTime Timestamp { get; }


/// <summary>
/// We will probably use nullable in the future, but for the time being
/// we use <see cref="DateTime.MinValue"/> to represent "no timestamp"
/// </summary>
internal static DateTime TimestampNotSpecified => DateTime.MinValue;


public override bool Equals(object obj) => Equals(obj as SnapshotMetadata);


Expand Down
2 changes: 1 addition & 1 deletion src/core/Akka/Actor/Scheduler/SchedulerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ void IActionScheduler.ScheduleRepeatedly(TimeSpan initialDelay, TimeSpan interva
DateTimeOffset ITimeProvider.Now { get { return TimeNow; } }

/// <summary>
/// TBD
/// The current time in UTC.
/// </summary>
protected abstract DateTimeOffset TimeNow { get; }

Expand Down
Loading