Skip to content

fix(actors): Recreate actor object when Dapr reports a new activation - #1876

Open
cicoyle wants to merge 4 commits into
dapr:masterfrom
cicoyle:fix-sidecar-restart-overwrite-actor
Open

cicoyle wants to merge 4 commits into
dapr:masterfrom
cicoyle:fix-sidecar-restart-overwrite-actor

Conversation

@cicoyle

@cicoyle cicoyle commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Pairs with the dapr/dapr change that sends Dapr-Actor-Activation-Id on actor callbacks.

After daprd is killed the app keeps its actor objects, and the new daprd never sends DELETE for them. The SDK reused the stale object, skipped onActivate and kept its in-memory fields.

  • new id: onDeactivate the old object (errors logged), activate a fresh one
  • same id or no id (older daprd): reuse, same as today
  • a call on a replaced or deleted object still runs onPostActorMethod, then skips the save and fails with IllegalStateException
  • a late first activation never replaces a newer one, a different id fails
  • ActorRuntime and DaprController get overloads with the id, old signatures still work
  • HTTP only, this SDK has no gRPC actor callback stream

Not covered: a save already in flight can still land late. Etags on actor state are a follow-up.

Signed-off-by: Cassandra Coyle <cassie@diagrid.io>
Signed-off-by: Cassandra Coyle <cassie@diagrid.io>
Copilot AI balanced review requested due to automatic review settings October 26, 2026 20:18
@cicoyle
cicoyle requested review from a team as code owners October 26, 2026 20:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

…ctivation

Signed-off-by: Cassandra Coyle <cassie@diagrid.io>
Copilot AI balanced review requested due to automatic review settings October 26, 2026 21:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@codecov

codecov Bot commented Oct 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.46835% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.71%. Comparing base (ca2a930) to head (c5ee38a).
⚠️ Report is 14 commits behind head on master.

Files with missing lines Patch % Lines
...main/java/io/dapr/actors/runtime/ActorManager.java 96.77% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1876      +/-   ##
============================================
+ Coverage     79.69%   80.71%   +1.02%     
- Complexity     2687     2876     +189     
============================================
  Files           267      274       +7     
  Lines          7933     8467     +534     
  Branches        842      911      +69     
============================================
+ Hits           6322     6834     +512     
- Misses         1255     1266      +11     
- Partials        356      367      +11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nelson-parente nelson-parente left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

#1840 added the gRPC actor event stream after this branched. ActorEventDispatcher on master calls runtime.invoke, invokeReminder and invokeTimer without the activation id, so stream-hosted actors keep the stale object. Can you rebase and pass the id from request.getMetadata() there?

}

/** Logs errors so a failed cleanup doesn't fail the call. */
private Mono<Void> deactivateDiscarded(ActorId actorId, T actor) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The discarded actor's onDeactivate can still save state, which brings back the overwrite this PR is meant to stop.

onDeactivateInternal clears the state cache and then calls the user's onDeactivate. Nothing stops that override from calling getActorStateManager().set(...) and then saveState(). A common pattern is to keep a counter in a field and write it on deactivation. After a daprd restart another host may have committed a newer value, and when the new activation id arrives (line 86), the stale actor writes its old value over it.

Suggest making a discarded actor refuse saves, for example a flag set here before onDeactivateInternal and checked in ActorStateManager.save() (which covers both saveState() and a direct getActorStateManager().save()). That keeps onDeactivate available for releasing resources without letting it write.

Signed-off-by: Cassandra Coyle <cassie@diagrid.io>
Copilot AI balanced review requested due to automatic review settings October 27, 2026 15:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants