Skip to content
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

Actor TryGetStateAsync()/ GetStateAsync()` is returning stale / previous state #8538

Open
olitomlinson opened this issue Feb 22, 2025 · 13 comments · May be fixed by dapr/dotnet-sdk#1473
Open

Comments

@olitomlinson
Copy link
Contributor

olitomlinson commented Feb 22, 2025

Runtime 1.15.0-rc15
SDK 1.15.0-rc.04

Expected Behavior

Update : I think this is related to state operations that are performed within the context of a Reminder.

When a call is made to this.StateManager.TryGetStateAsync() then I expect the newest/current data to be retrieved.

Actual Behavior

When a call is made to this.StateManager.TryGetStateAsync() then it is retrieving a cached/previous/stale version of the state.

Image

However, if I use the Actor HTTP API to retrieve the same state key, I can see the new/current state returned as expected

Image

If I then kill the dapr/app process, and restart it, the Actor will now log out the correct state

Image

Steps to Reproduce the Problem

Release Note

RELEASE NOTE:

@olitomlinson olitomlinson added the kind/bug Something isn't working label Feb 22, 2025
@WhitWaldo WhitWaldo self-assigned this Feb 22, 2025
@WhitWaldo WhitWaldo removed the kind/bug Something isn't working label Feb 22, 2025
@WhitWaldo WhitWaldo linked a pull request Feb 22, 2025 that will close this issue
2 tasks
@olitomlinson
Copy link
Contributor Author

Just tested this on runtime 1.14.4 (scheduler reminders feature off) and this is working as expected.

Could this be a runtime regression in 1.15?

@WhitWaldo
Copy link
Contributor

@olitomlinson I don't think so.. to my knowledge, unless it's implementing its own cache for batch pooling or the like, the runtime has no knowledge of the SDK cache. It's implemented entirely at the SDK level and on the SDK, it hasn't seen updates since last year (to add TTL support) and again for another year or two before then (for reentrancy support).

@yaron2
Copy link
Member

yaron2 commented Feb 26, 2025

The runtime doesn't do any sort of caching

@olitomlinson
Copy link
Contributor Author

olitomlinson commented Feb 26, 2025

@yaron2

Without changing the dotnet SDK version (from latest 1.15 RC), if I downgrade the runtime from 1.15 RC18 to 1.14.4 and turn off scheduled reminders features, it works as expected.

That points to this being a runtime issue, no?

@yaron2
Copy link
Member

yaron2 commented Feb 26, 2025

@yaron2

Without changing the dotnet SDK version (from latest 1.15 RC), if I downgrade the runtime from 1.15 RC18 to 1.14.4 and turn off scheduled reminders features, it works as expected.

That points to this being a runtime issue, no?

Not necessarily. Can you try to use the API directly (not through the SDK) and report what state you're getting? Also, scheduler reminders data in 1.15 is incompatible with 1.14 so any test should start with a clean state store.

@olitomlinson
Copy link
Contributor Author

olitomlinson commented Feb 27, 2025

@yaron2

  1. Here is a log from my app (note that there is 2 unique entries in sev 3) ✅

Important : this log is generated within the context of an Actor Reminder, every 15 seconds.

Image
  1. Here is the state from the API directly ✅
Image
  1. Now I add a 3rd entry into the system. beginning with 39. ✅

  2. Notice how it is NOT picked up in the log ❌

Image
  1. However the 3rd entry (beginning 39) is picked up by calling the API directly ✅
Image
  1. Important : Stop the compose session (killing dapr and the app) (do not delete any state)

  2. Important : Restart the compose session

  3. Observe the the 3rd entry is in the log message (as is correct / expected) : ✅

Image

@olitomlinson olitomlinson changed the title Actor TryGetStateAsync() is returning stale / previous state Actor TryGetStateAsync()/ GetStateAsync()` is returning stale / previous state Feb 27, 2025
@yaron2
Copy link
Member

yaron2 commented Feb 27, 2025

@olitomlinson do you have actor reentrance enabled?

@olitomlinson
Copy link
Contributor Author

@yaron2 nothing explicit in my code, so what ever is the OOTB default

@yaron2
Copy link
Member

yaron2 commented Feb 27, 2025

@yaron2 nothing explicit in my code, so what ever is the OOTB default

Can you provide a snippet of your code here?

@yaron2
Copy link
Member

yaron2 commented Feb 27, 2025

We tested this and couldn't reproduce.

public async Task ReceiveReminderAsync(string reminderName, byte[] state, TimeSpan dueTime, TimeSpan period)
        {
            // This method is invoked when an actor reminder is fired.
            var actorState = await this.StateManager.GetStateAsync<MyData>(StateName);
            if (actorState != null)
            {
                Console.WriteLine($"This is Actor reminder {reminderName} with state {actorState}.");
            }
            actorState.PropertyB = $"Reminder triggered at '{DateTime.Now:yyyy-MM-ddTHH:mm:ss}'";
            await this.StateManager.SetStateAsync<MyData>(StateName, actorState, ttl: TimeSpan.FromMinutes(5));
        }

cc @artursouza

@artursouza
Copy link
Member

artursouza commented Feb 28, 2025

Summary: the bug is that invocation of Actor method contains the reentrancy id header when reentrancy is not enabled making the use of Actors in .Net broken when using runtime 1.15.

In 1.15, the actor method invocation (incorrectly) includes the Dapr-Reentrancy-Id header but it (correctly) does not include this same header in the reminder invocation. So, the .Net SDK (because it respects the state context based on the reentrancy id) returns different states based on each of those calls. This behavior does not happen in other SDKs since they don't handle reentrancy for actors. This is a bug because reentrancy is not enabled in the actor type but the 1.15 runtime incorrectly adds it to the actor invocations.

The other bug that I found (minor) is that 1.15 does not include the dueTime or period attributes when invoking a reminder. It can be interpreted as a new desired behavior too, I have no strong feelings about this one.

See my screenshots below of using mitmproxy to intercept calls from sidecar to app:

1.15 runtime incorrectly includes Dapr-Reentrancy-Id when invoking the SaveData actor method on the app (root cause):
Image

1.15 runtime correctly does not include Dapr-Reentrancy-Id when invoking the reminder BUT it does not include the dueTime or period attributes (second bug):
Image

1.14 runtime correctly does not include Dapr-Reentrancy-Id when invoking the SaveData actor method on the app:
Image

1.14 runtime correctly does not include Dapr-Reentrancy-Id when invoking the reminder AND it does include the dueTime or period attributes as well:
Image

@artursouza artursouza added the P0 label Feb 28, 2025
@artursouza artursouza transferred this issue from dapr/dotnet-sdk Feb 28, 2025
@JoshVanL
Copy link
Contributor

Hey all!

Thanks for the great write up of the issue 🙂
I've added a fix for this here: #8542

@WhitWaldo
Copy link
Contributor

@JoshVanL Can this be closed as completed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants