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

Hot Reloading: Actor State Store Reloading #7287

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

JoshVanL
Copy link
Contributor

Part of #1172
Branched from #7260

Adds support for actor state store hot reloading.

PR updates the actors package so that the actor state store is dynamically
retrieved from the component store every time the state it invoked. The
passed static actor state store name previously used to retrieve the
state store has been removed.

The component store has been updated to track the current actor state
store. It is no longer tracked by the state processor, and instead the
state processor updates the latest in the component store.

Like before, the actor state store must be configured on Dapr startup,
else the actor subsystem runtime will not be enabled, even if an actor
state store is hot reloaded later.

Adds actor hotreloading integration tests for the actor state store.

@JoshVanL JoshVanL added the P0 label Dec 11, 2023
@JoshVanL JoshVanL added this to the v1.13 milestone Dec 11, 2023
@JoshVanL JoshVanL removed the P0 label Dec 11, 2023
Part of dapr#1172
Branched from dapr#7260

Adds support for actor state store hot reloading.

PR updates the actors package so that the actor state store is dynamically
retrieved from the component store every time the state it invoked. The
passed static actor state store name previously used to retrieve the
state store has been removed.

The component store has been updated to track the current actor state
store. It is no longer tracked by the state processor, and instead the
state processor updates the latest in the component store.

Like before, the actor state store _must_ be configured on Dapr startup,
else the actor subsystem runtime will not be enabled, even if an actor
state store is hot reloaded later.

Adds actor hotreloading integration tests for the actor state store.

Signed-off-by: joshvanl <me@joshvanl.dev>
@JoshVanL JoshVanL force-pushed the hot-reloading-components-actors branch from fa51653 to b172d75 Compare January 9, 2024 15:06
@JoshVanL JoshVanL marked this pull request as ready for review January 9, 2024 15:23
@JoshVanL JoshVanL requested review from a team as code owners January 9, 2024 15:23
Copy link
Contributor

@ItalyPaleAle ItalyPaleAle left a comment

Choose a reason for hiding this comment

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

I would like us to be very careful with hot-reloading actor state store please, especially as this interacts with the reminder store. The actor state store is not meant to be hot-reloaded today, and the impact on reminders is unclear.

For example, in the current code I do not see the method that triggers reloading of reminders when the actor state store changes. This could have very major impacts on the actor subsystem.

Another thing to consider is that there could be active actors which loaded state, and assume that they can store it back (with etag matching) later. If the state store changes, that could cause some significant issues.

@JoshVanL JoshVanL added the autoupdate DaprBot will keep the Pull Request up to date with master branch label Jan 10, 2024
Copy link

codecov bot commented Jan 10, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 64.33%. Comparing base (51dc3a1) to head (6c3c25a).
Report is 189 commits behind head on master.

Files Patch % Lines
pkg/runtime/processor/state/state.go 30.00% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7287      +/-   ##
==========================================
+ Coverage   64.31%   64.33%   +0.01%     
==========================================
  Files         238      238              
  Lines       21877    21862      -15     
==========================================
- Hits        14070    14064       -6     
+ Misses       6598     6585      -13     
- Partials     1209     1213       +4     

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

@ItalyPaleAle ItalyPaleAle removed the autoupdate DaprBot will keep the Pull Request up to date with master branch label Jan 13, 2024
@mukundansundar mukundansundar removed this from the v1.13 milestone Feb 26, 2024
@mukundansundar mukundansundar added this to the v1.14 milestone Feb 26, 2024
@dapr-bot
Copy link
Collaborator

This pull request has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@dapr-bot dapr-bot added stale Issues and PRs without response and removed stale Issues and PRs without response labels Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants