fix(actors): subscribe one local cache invalidator PID per cluster member - #174
Conversation
Register StartClusterMember with priority -100 and add regression coverage for hosted-service ordering. Fixes elsa-workflows#167
Reuse the RemoteConfig attached to the ActorSystem when registering virtual actor protobuf descriptors. Add regression coverage for factory invocation count and descriptor registration.
Replace the virtual LocalCache grain and ClusterIdentity subscription with a named member-local system actor subscribed by concrete PID. Await subscription during startup, then unsubscribe and stop the actor during shutdown. Remove the obsolete LocalCache grain contract and virtual actor kind while retaining protobuf descriptor registration for remote Pub/Sub delivery. Fixes elsa-workflows#173 BREAKING CHANGE: Removes the generated LocalCache grain APIs and changes StartLocalCacheActor from BackgroundService to IHostedService.
Add real multi-member coverage proving that startup awaits PID subscription, each member owns one local invalidator, signals are delivered exactly once, and graceful shutdown removes the subscription and actor. Verify that the legacy virtual actor kind is removed while its protobuf message descriptor remains registered.
|
PR author is not in the allowed authors list. |
sfmskywalker
left a comment
There was a problem hiding this comment.
Maintainer review completed directly on current head 8a8ac83.
Reviewed correctness and lifecycle ordering, multi-member PID ownership, failure cleanup, breaking API removal, test determinism, and security/performance scope. The only actionable finding was missing verification for subscription/unsubscription failure cleanup; this is fixed in 8a8ac83 with two bounded regression tests.
Local validation: distributed-cache integration suite 8/8 plus three repeat runs, actor unit suite 6/6, net8/net9/net10 module build succeeded, and diff check passed. No remaining actionable findings. Copilot is intentionally bypassed; Greptile is unavailable for this contributor due its author allowlist.
Summary
Replace the placement-managed
LocalCachevirtual grain with one named, member-local cache invalidator actor per Proto.Actor cluster member. Each actor subscribes with its concrete PID; host startup awaits the acknowledgement, and shutdown unsubscribes before stopping the actor.Fixes #173.
Changes
$memory-cache-invalidatorper actor systemLocalCache.Messages.protoandLocalCacheMessagesReflection.Descriptorfor Pub/Sub serializationBreaking change and upgrade limitation
The generated public LocalCache grain APIs and
BackgroundServicebase type are intentionally removed. Consumers using them must update and recompile. Mixed-version rolling upgrades are unsupported: stop every old member first, and clear durablechange-token-signalssubscriber state containing legacy cluster identities before starting the new version.Validation
git diff --check origin/main...HEAD: passedThe tests cover acknowledgement-gated startup, one PID per member, exact per-member delivery, graceful removal, post-stop delivery, descriptor/kind registration, subscription failure cleanup, and unsubscription failure cleanup.
Review
Maintainer review is being performed directly on the current head. Copilot is intentionally bypassed per maintainer direction. Greptile is unavailable because its author allowlist rejects this contributor PR.