Proposal to Allow Multiple Reminders/Actors Implementation #50
Conversation
Signed-off-by: Deepanshu Agarwal <[email protected]>
Signed-off-by: Deepanshu Agarwal <[email protected]>
|
|
||
| This Control Plane Service may require some component, like a database etc. which it should be able to load, using Operator. But, components visible to a Control Plane service should not be visible to Daprd sidecar and vice-versa. | ||
| Hence, just a logical grouping is required. | ||
|
|
||
| (https://github.com/dapr/dapr/pull/7314) |
There was a problem hiding this comment.
@DeepanshuA Can you add more details for this?
|
|
||
| ## Overview | ||
|
|
||
| As a user, I should be able to use Dapr core functionality of Actors/reminders i.e. leverage Actors/reminders interfaces BUT to be able to implement these interfaces in a new Control Plane service, which as a user, I should be allowed to design as per the way it solves issues for me. |
|
|
||
| Depending on the implementation, it can be decided that what all RPCs need to be defined for a Service. Example, for a Placement Service which just creates and pushes Actor Types v/s Dapr Hosts table information to All sidecars, only table related ReportDaprStatusneeds to be as rpc. | ||
|
|
||
| Whereas, for a Service, where Actor information is Pulled by sidecars from Actors Service needs some methods like LookupActor or ReportActorDeactivation, which thus form a part of RPCs. |
There was a problem hiding this comment.
When you say for a service, do you mean any control plane service? Or like any service a user creates for their own separate control plane service needs?
| Whereas, for a Service, where Actor information is Pulled by sidecars from Actors Service needs some methods like LookupActor or ReportActorDeactivation, which thus form a part of RPCs. | |
| Whereas, for a Service, where Actor information is pulled by sidecars from the Actors Service, it only needs some methods like LookupActor or ReportActorDeactivation, which thus form a part, or subset, of the RPCs. |
|
|
||
| Whereas, for a Service, where Actor information is Pulled by sidecars from Actors Service needs some methods like LookupActor or ReportActorDeactivation, which thus form a part of RPCs. | ||
|
|
||
| For some minor changes left in interfaces, has been raised (https://github.com/dapr/dapr/pull/7281). |
There was a problem hiding this comment.
I think its worth elaborating on what changes the PR made if you are linking to it. It reads better if explained.
| For some minor changes left in interfaces, has been raised (https://github.com/dapr/dapr/pull/7281). | |
| Standardization efforts have been made for the actor services, namely placement and reminders in [this PR](https://github.com/dapr/dapr/pull/7281). |
| * If the user has set the `dapr.io/placement-host-address` annotation, that forces the injector to set `--placement-host-address` to the value passed, as is the case today | ||
| * Otherwise, if there's no explicit dapr.io/placement-host-address annotation: | ||
| * If the configuration (via Helm) is to use placement, then sets `--placement-host-address` exactly as today (this is default value) | ||
| * If the configuration is to use another actors service, it sets `--actors-service <name>:<address>:<port>` following the same logic as the reminders svc) |
There was a problem hiding this comment.
| * If the configuration is to use another actors service, it sets `--actors-service <name>:<address>:<port>` following the same logic as the reminders svc) | |
| * If the configuration is to use another actors service, it sets `--actors-service <name>:<address>:<port>` following the same logic as the reminders service) |
| * If the configuration is to use another actors service, it sets `--actors-service <name>:<address>:<port>` following the same logic as the reminders svc) | ||
| This is backwards-compatible. | ||
|
|
||
| Note: Exception is thrown when `--actors-service` or `--reminders-service` are set, if the injector is trying to inject a sidecar that use an oldeer version of Dapr, daprd that will crash because the flag doesn't exist. This is a "feature": if the cluster wants to use a different actors/reminders service, versions of Dapr that don't support that should crash. |
There was a problem hiding this comment.
| Note: Exception is thrown when `--actors-service` or `--reminders-service` are set, if the injector is trying to inject a sidecar that use an oldeer version of Dapr, daprd that will crash because the flag doesn't exist. This is a "feature": if the cluster wants to use a different actors/reminders service, versions of Dapr that don't support that should crash. | |
| Note: An exception is thrown when `--actors-service` or `--reminders-service` are set if the injector is trying to inject a sidecar that uses an older version of Dapr. This will cause the daprd sidecar to crash because the flag doesn't exist. Therefore, if the cluster wants to use a different actors/reminders service, the version of Dapr used must support that flag or the sidecar will crash. | |
|
|
||
| #### Step 2: Component Loading: | ||
|
|
||
| This Control Plane Service may require some component, like a database etc. which it should be able to load, using Operator. But, components visible to a Control Plane service should not be visible to Daprd sidecar and vice-versa. |
There was a problem hiding this comment.
I assume nothing will break if a component is not required?
| type PlacementService interface { | ||
| io.Closer | ||
|
|
||
| Start(context.Context) error |
There was a problem hiding this comment.
This API seems to be biased towards an alternative implementation. Is there a way to show how the existing placement service will work with this interface? Is there a way to elevate the abstraction to a small set of methods:
- Start()
- WaitUntilReady()
- Lookup()
- RegisterActorType()
Or clarify what each of the other methods are needed for. If the interface is simply an union of the methods of each possible implementation, then the interface is not abstracting them properly and each vendor will need to add to this list of methods.
There was a problem hiding this comment.
These interfaces are taken as it is directly from current dapr/dapr master.
Existing placement client works with this interface.
There are two methods which are not used right now: ReportActorDeactivation and HaltActorFn, but they are useful when actors lifecycle information needs to be updated in Placement service, if it maintains them.
I have added comments with these APIs.
|
|
||
| #### Step 1: Configuration changes: | ||
|
|
||
| A) Changes in charts/dapr/charts/dapr_config/templates/dapr_default_config.yaml |
There was a problem hiding this comment.
I don't think a vendor's implementation should impact the OSS Chart. It is the vendor's responsibility to provide the Chart for their own services. Otherwise, each vendor will need to PR against the OSS Chart for any specific config of their custom implementation.
There was a problem hiding this comment.
Yeah, it is not affected. I will remove it from list of updates. dapr/dapr#7318 doesn't add any such change.
| DrainRebalancedReminders(actorType string, actorID string) | ||
| OnPlacementTablesUpdated(ctx context.Context) | ||
|
|
||
| SetResiliencyProvider(resiliency resiliency.Provider) |
There was a problem hiding this comment.
Can't resiliency be offered at the layer above this interface?
There was a problem hiding this comment.
Just noticed, this was removed already from dapr/dapr master interface. I have removed from here too.
Co-authored-by: Cassie Coyle <[email protected]> Signed-off-by: Deepanshu Agarwal <[email protected]>
Co-authored-by: Cassie Coyle <[email protected]> Signed-off-by: Deepanshu Agarwal <[email protected]>
Co-authored-by: Cassie Coyle <[email protected]> Signed-off-by: Deepanshu Agarwal <[email protected]>
Signed-off-by: Deepanshu Agarwal <[email protected]>
Signed-off-by: Deepanshu Agarwal <[email protected]>
Signed-off-by: Deepanshu Agarwal <[email protected]>
Proposal to Allow Multiple Versions of Reminder/Actors Implementation