Skip to content

Skip the state store existence check when setting actor state - #1239

Open
CasperGN wants to merge 7 commits into
dapr:mainfrom
CasperGN:feat/actor-skip-exists-check-on-write
Open

CasperGN wants to merge 7 commits into
dapr:mainfrom
CasperGN:feat/actor-skip-exists-check-on-write

Conversation

@CasperGN

@CasperGN CasperGN commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Based on #1238 (the second PR in this stack, based on #1237 and #1227). Merge it after that. Until then this diff also shows the earlier commits; this PR's own commit is 11f6f53 ("perf(actor): skip the state store read when setting an untracked key").

Description

What was wrong. When set_state or set_state_ttl got a key that was not in the state tracker, it read the state store only to decide whether to record the change as "add" or "update". The state provider saves both as the same upsert, so that read did nothing useful and cost a round trip per new key.

What changed. The read is gone. An untracked key is now recorded as "update" with no store call.

Design choices.

  • Update, not add. We no longer know whether the key is already in the store. If the key is set and then removed in the same turn, an "update" becomes a delete, while an "add" is just dropped from the tracker. Dropping it would silently skip deleting a key that does exist. Sending a delete for a key that turns out not to exist does no harm. The .NET change makes the same choice.
  • Keys the manager knows are absent still use "add", so add-then-remove still sends nothing. That covers try_add_state after its read, and a cached "not found" entry from the previous PR that is then set.
  • try_add_state still reads the store and returns False when the key exists. try_remove_state still reads the store to report whether anything was removed. contains_state is unchanged.
  • The default-tracker refresh after a reentrant save only looks at remove versus not-remove, so a reentrant set is saved as an upsert and refreshes the default entry, and a reentrant set followed by a remove sends a delete and evicts the default entry.

Behaviour change. Setting a key that is neither tracked nor known to be absent, then removing it in the same turn, now sends a delete to the store. Before, it sent a delete only if the store read had found the key. The delete still carries the value and ttl from the set (this was already true for set-then-remove on existing keys); the store ignores them, and I left that alone to keep the scope small.

Tests. Existing set_state / set_state_ttl tests now expect "update", and test_get_state_names asserts no store read. New tests cover: set and set_ttl with zero store reads and the exact upserts saved; set-then-remove sending deletes; add-then-remove sending nothing; a reentrant set-then-remove sending a delete and evicting the default entry; and try_add_state / try_remove_state still checking the store. Reverting the change fails 9 tests; keeping the skipped read but recording "add" fails 10, including the delete tests.

This ports dapr/dotnet-sdk#1914. Thanks to @olitomlinson for pointing out the .NET changes on #1227.

Issue reference

Related to dapr/dapr#10532 (reminders miss state saved by reentrant calls) and #1227.

Checklist

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation (no public API change)

Ran: pytest tests/actor (210 passed), ruff check, ruff format --check, mypy (all clean).

🤖 Generated with Claude Code

JoshVanL and others added 7 commits September 22, 2026 15:10
With reentrancy enabled, each dispatched method call gets its own
state change tracker, but activation, reminders and timers run on the
default tracker because no reentrancy id reaches them. A key read
during activation stays cached there with change kind none forever,
while method calls write the same key through their own trackers.

A reminder callback that later reads that key is served the stale
activation value. An app that skips its write because the value looks
unchanged loses that write silently: nothing is logged anywhere,
because no write is ever issued.

Drop the default tracker's clean copies of keys written through a
reentrancy-scoped tracker, so the next read reloads them from the
runtime.

Reported in dapr/dapr#10532, where a reminder callback's
read-modify-write of an actor state key never persisted while the
identical write from an ordinary method call did, and only with
reentrancy enabled.

Should be backported.

Signed-off-by: joshvanl <me@joshvanl.dev>
…ant save

Instead of dropping the default tracker's clean copy of a key that a
reentrant call saved, replace it with the saved value and ttl so the next
read from activation, a reminder or a timer is served from cache rather
than costing an extra state store read. Removed keys are still dropped and
entries with pending changes are left alone.

The cached value is passed through the state serializer first, so it has
the same shape a fresh read would return (for example a tuple comes back
as a list). This mirrors dapr/dotnet-sdk#1912.

Signed-off-by: Casper Nielsen <casper@diagrid.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tch a fresh read

Two cases fall back to dapr#1227's eviction instead of an in-place refresh:
a saved None value, which the state provider leaves out of the write, and
a state serializer that fails to decode the value after the save has
already committed. The save no longer raises after a successful write.

Signed-off-by: Casper Nielsen <casper@diagrid.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reading a key that does not exist went back to the state store every time,
because a miss was not cached. try_get_state now records a miss as a clean
"not found" entry in the default tracker, so later reads, contains_state,
try_remove_state and remove_state answer without I/O, and try_add_state,
set_state, set_state_ttl, get_or_add_state and add_or_update_state turn it
into an add without asking the store again. get_state_names skips it and
save_state never sends it.

Misses are not cached in reentrant trackers. Nothing refreshes an outer
reentrant call's tracker when a nested reentrant call (A -> B -> A) saves,
so a cached miss there would hide the nested write and then overwrite it
through get_or_add_state or try_add_state.

The entry is a private StateMetadata subclass that reports the existing
"none" change kind, so the public StateChangeKind enum, ActorStateChange and
subclasses of ActorStateManager are unchanged.

A reentrant save refreshes a "not found" default entry the same way as a
clean one: a write replaces it with the saved value and a remove evicts it.
Without that, a reminder that cached a miss would keep seeing the key as
absent after a reentrant call created it.

Ports dapr/dotnet-sdk#1913 and dapr/dotnet-sdk#1916.

Signed-off-by: Casper Nielsen <casper@diagrid.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
create_task(context=...) only exists from Python 3.11. A task already runs
in a copy of the caller's context, so the argument isn't needed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
set_state and set_state_ttl read the state store for a key that was not in
the tracker only to choose between add and update, but the state provider
saves both as the same upsert. The key is now recorded as an update without
that read.

Update rather than add is deliberate: the key may already exist in the
store, so a remove later in the same turn has to send a delete instead of
dropping the pending entry. Deleting a key that turns out not to exist is
harmless; skipping the delete of one that does is not.

Keys the manager knows are absent keep the add kind, so add-then-remove
still sends nothing: try_add_state still reads the store and fails when the
key exists, and a cached "not found" entry still turns into an add.
try_remove_state still reads the store to report whether anything was
removed. A reentrant save refreshes the default tracker the same way
whatever kind the saved change had.

Ports dapr/dotnet-sdk#1914.

Signed-off-by: Casper Nielsen <casper@diagrid.io>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.03%. Comparing base (fb229bc) to head (11f6f53).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1239      +/-   ##
==========================================
+ Coverage   83.89%   84.03%   +0.13%     
==========================================
  Files         123      123              
  Lines       10265    10298      +33     
==========================================
+ Hits         8612     8654      +42     
+ Misses       1653     1644       -9     

☔ 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.

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.

2 participants