Skip to content

Conversation

planerist
Copy link
Contributor

@planerist planerist commented Oct 3, 2017

  1. If your actor has a big number of snapshots then system experiences performance degradation.

The reason for this degradation is SQL-driver rows pre-fetch. Furthermore, the current implementation of SelectSnapshotAsync reads only one row, so there is no reason to query more than one snapshot row.

  1. DbCommand has memory leaks in ExecuteNonQueryAsync and ExecuteDbDataReaderAsync methods. So you should provide new cancellation token for each SQL command and dispose it (more here: https://github.com/dotnet/corefx/issues/16439).

@@ -273,7 +273,8 @@ protected AbstractQueryExecutor(QueryConfiguration configuration, Akka.Serializa
WHERE {Configuration.PersistenceIdColumnName} = @PersistenceId
AND {Configuration.SequenceNrColumnName} <= @SequenceNr
AND {Configuration.TimestampColumnName} <= @Timestamp
ORDER BY {Configuration.SequenceNrColumnName} DESC";
ORDER BY {Configuration.SequenceNrColumnName} DESC
LIMIT 1";
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -110,9 +110,10 @@ protected override void PostStop()
try
{
using (var connection = CreateDbConnection())
using (var nestedCancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(_pendingRequestsCancellation.Token))
{
Copy link
Member

Choose a reason for hiding this comment

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

Good solution on using CreateLinkedTokenSource

Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

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

These look good. Thanks for your contribution!

@Aaronontheweb Aaronontheweb merged commit b81e6a6 into akkadotnet:dev Oct 3, 2017
@Aaronontheweb Aaronontheweb added this to the 1.3.2 milestone Oct 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants