-
Notifications
You must be signed in to change notification settings - Fork 37
chore: improvements around dynamic scaling of sharded daemon processes #1437
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
|
|
||
| import akka.Done; | ||
| import akka.actor.typed.ActorSystem; | ||
| import akka.cluster.sharding.typed.ShardedDaemonProcessSettings; | ||
| import akka.cluster.sharding.typed.javadsl.ClusterSharding; | ||
| import akka.cluster.sharding.typed.javadsl.ShardedDaemonProcess; | ||
| import akka.http.javadsl.model.HttpRequest; | ||
|
|
@@ -81,17 +82,20 @@ public static void initPushedEventsConsumer(ActorSystem<?> system) { | |
| var numberOfSliceRanges = | ||
| system.settings().config().getInt("iot-service.temperature.projections-slice-count"); | ||
|
|
||
| var sliceRanges = | ||
| EventSourcedProvider.sliceRanges( | ||
| system, R2dbcReadJournal.Identifier(), numberOfSliceRanges); | ||
|
|
||
| ShardedDaemonProcess.get(system) | ||
| .init( | ||
| .initWithContext( | ||
| ProjectionBehavior.Command.class, | ||
| "TemperatureProjection", | ||
| sliceRanges.size(), | ||
| i -> ProjectionBehavior.create(projection(system, sliceRanges.get(i))), | ||
| ProjectionBehavior.stopMessage()); | ||
| numberOfSliceRanges, | ||
| daemonContext -> { | ||
| var sliceRanges = | ||
| EventSourcedProvider.sliceRanges( | ||
| system, R2dbcReadJournal.Identifier(), daemonContext.totalProcesses()); | ||
|
Contributor
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. why do we use EventSourcedProvider here and in some other places would be easier to always use Persistence.sliceRanges?
Contributor
Author
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. Agree, strange
Contributor
Author
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. 21 places across projections samples and specs (!!)
Contributor
Author
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. What it adds is looking up the read journal for that specific plugin/config, so that the actual journal could have some other slicing scheme. But in practice it is always the same, so should we always use the simpler one? In that case, why does the PR adding the method didn't shed any light on it #609
Contributor
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. ok, I would guess that the guy thought it would be nice to stay within projections api surface and not reach out to persistence. It's typically used together with Let's leave it as is, using
Contributor
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. Might look better to move the whole slice range stuff into the |
||
| return ProjectionBehavior.create( | ||
| projection(system, sliceRanges.get(daemonContext.processNumber()))); | ||
| }, | ||
| ShardedDaemonProcessSettings.create(system), | ||
| Optional.of(ProjectionBehavior.stopMessage())); | ||
| } | ||
|
|
||
| private static Projection<EventEnvelope<TemperatureRead>> projection( | ||
|
|
||
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.
do we want to promote initWithContext in all places, even if scaling isn't used/needed? the signature looks more complex than init?
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.
Hmm, good point, I went for all the places to make sure anything users could copy paste is safe to evolve/scale, but maybe that is overdoing it.