-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Re-implement PersistenceIds persitence query to match scala implementation behavior #4531
Changes from 9 commits
3a6af76
ef43856
a7e2e5f
7796f2f
ea97555
f98a9d7
edb8e52
238fca8
f8c8460
1d8f659
738f855
1b79bf5
1352b0e
1ec20e4
e983d47
a7c5f81
f6f1b60
9e70477
fabeaa1
386d6fc
e7d174a
6982944
5a08ec5
8df7e67
4eb24e2
e5f7d98
8954464
c20ddb3
56f4f47
d75c717
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ | |
using Akka.Configuration; | ||
using Akka.Persistence.Journal; | ||
using Akka.Streams.Dsl; | ||
using Akka.Streams; | ||
using Akka.Util.Internal; | ||
|
||
namespace Akka.Persistence.Query.Sql | ||
|
@@ -40,12 +41,16 @@ public static Config DefaultConfiguration() | |
private readonly TimeSpan _refreshInterval; | ||
private readonly string _writeJournalPluginId; | ||
private readonly int _maxBufferSize; | ||
private readonly ExtendedActorSystem _system; | ||
|
||
private IPublisher<string> _persistenceIdsPublisher = null; | ||
|
||
public SqlReadJournal(ExtendedActorSystem system, Config config) | ||
{ | ||
_refreshInterval = config.GetTimeSpan("refresh-interval", null); | ||
_writeJournalPluginId = config.GetString("write-plugin", null); | ||
_maxBufferSize = config.GetInt("max-buffer-size", 0); | ||
_system = system; | ||
} | ||
|
||
/// <summary> | ||
|
@@ -68,18 +73,36 @@ public SqlReadJournal(ExtendedActorSystem system, Config config) | |
/// backend journal. | ||
/// </para> | ||
/// </summary> | ||
public Source<string, NotUsed> PersistenceIds() => | ||
Source.ActorPublisher<string>(AllPersistenceIdsPublisher.Props(true, _writeJournalPluginId)) | ||
.MapMaterializedValue(_ => NotUsed.Instance) | ||
.Named("AllPersistenceIds") as Source<string, NotUsed>; | ||
public Source<string, NotUsed> PersistenceIds() | ||
{ | ||
if (_persistenceIdsPublisher is null) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This call might need to be synchronized - we want all of the consumer of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you check and see if my implementation looks good? |
||
{ | ||
var graph = | ||
Source.ActorPublisher<string>( | ||
LivePersistenceIdsPublisher.Props( | ||
_refreshInterval, | ||
_writeJournalPluginId)) | ||
.ToMaterialized(Sink.DistinctRetainingFanOutPublisher<string>(PersistenceIdsShutdownCallback), Keep.Right); | ||
_persistenceIdsPublisher = graph.Run(_system.Materializer()); | ||
} | ||
|
||
return Source.FromPublisher(_persistenceIdsPublisher) | ||
.MapMaterializedValue(_ => NotUsed.Instance) | ||
.Named("AllPersistenceIds") as Source<string, NotUsed>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just do a direct cast here rather than an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The cast was not necessary anyway, I think it's there as a readability helper. I'll remove it. |
||
} | ||
|
||
private void PersistenceIdsShutdownCallback() | ||
{ | ||
_persistenceIdsPublisher = null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment as above, RE: synchronization |
||
} | ||
|
||
/// <summary> | ||
/// Same type of query as <see cref="PersistenceIds"/> but the stream | ||
/// is completed immediately when it reaches the end of the "result set". Persistent | ||
/// actors that are created after the query is completed are not included in the stream. | ||
/// </summary> | ||
public Source<string, NotUsed> CurrentPersistenceIds() => | ||
Source.ActorPublisher<string>(AllPersistenceIdsPublisher.Props(false, _writeJournalPluginId)) | ||
public Source<string, NotUsed> CurrentPersistenceIds() | ||
=> Source.ActorPublisher<string>(CurrentPersistenceIdsPublisher.Props(_writeJournalPluginId)) | ||
.MapMaterializedValue(_ => NotUsed.Instance) | ||
.Named("CurrentPersistenceIds") as Source<string, NotUsed>; | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good fix