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

[Workflow] Workflow actor state should be saved with a TTL of -1 #6475

Open
Tracked by #8008
cgillum opened this issue Jun 8, 2023 · 9 comments
Open
Tracked by #8008

[Workflow] Workflow actor state should be saved with a TTL of -1 #6475

cgillum opened this issue Jun 8, 2023 · 9 comments
Labels
kind/bug Something isn't working P2 pinned
Milestone

Comments

@cgillum
Copy link
Contributor

cgillum commented Jun 8, 2023

In what area(s)?

/area runtime

What version of Dapr?

1.11

Expected Behavior

The Dapr Workflow engine should save actor state with a TTL of -1 to ensure any global TTL configuration doesn't cause workflow state to be lost, even if the actor state store has a default TTL specified.

Note that there is some discussion about whether or not TTLs should ever be configured for actor state stores here: #3658. Either way, the internal workflow actors should defend against this.

Actual Behavior

Workflow actor state is saved without specifying any TTL, making it vulnerable to state store configurations with TTLs configured.

Steps to Reproduce the Problem

  1. Configure an actor state store with a short TTL
  2. Run a long-running workflow that will exceed the above TTL

Release Note

RELEASE NOTE: FIX Prevent actor TTL configuration from disrupting workflows

@cgillum cgillum added the kind/bug Something isn't working label Jun 8, 2023
@ItalyPaleAle
Copy link
Contributor

The default behavior of Dapr components is to not set a TTL if the TTL is unset.

@cgillum
Copy link
Contributor Author

cgillum commented Jun 9, 2023

@clintsinger or @olitomlinson would you be able to comment further on this issue? This was originally discussed on Discord, and my understanding was that there could be a default/global TTL configuration placed on state stores. I didn't actually do the work to verify this, though.

@olitomlinson
Copy link

@cgillum

I think the problem here is that TTLs can be set completely external to Dapr, depending on the chosen state technology

I.e. in CosmosDb, you can set a default TTL which applies to any document in the container. So, as @berndverst suggests, setting ttl to -1 will override any default TTL behaviours applied outside of Dapr.

@ItalyPaleAle
Copy link
Contributor

@olitomlinson yes, in the case of Cosmos DB you can set a default TTL and in that case you must set -1 to override it. I do not believe any other database we support has the same behavior.

IMHO this is not a Workflows issue but rather we just need to document that people shouldn't enable TTLs with a default value in Cosmos DB.

@dapr-bot
Copy link
Collaborator

dapr-bot commented Aug 8, 2023

This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted or triaged/resolved) or other activity occurs. Thank you for your contributions.

@dapr-bot dapr-bot added the stale Issues and PRs without response label Aug 8, 2023
@dapr-bot
Copy link
Collaborator

This issue has been automatically closed because it has not had activity in the last 67 days. If this issue is still valid, please ping a maintainer and ask them to label it as pinned, good first issue, help wanted or triaged/resolved. Thank you for your contributions.

@dapr-bot dapr-bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 15, 2023
@rabollin
Copy link
Contributor

/keep-alive

@dapr-bot
Copy link
Collaborator

👋 @rabollin, my apologies but I can't perform this action for you because your username is not in the allowlist in the file .github/scripts/dapr_bot.js.

@mukundansundar mukundansundar added pinned and removed stale Issues and PRs without response labels Sep 26, 2023
@cgillum cgillum added this to the v1.15 milestone Sep 12, 2024
@cgillum cgillum added the P2 label Sep 12, 2024
@cgillum
Copy link
Contributor Author

cgillum commented Sep 12, 2024

Marking as P2 since this may be a corner case. There's also discussion of excluding Azure Cosmos DB from the list of supported workflow state stores, and it seems that may be the only state store potentially impacted.

@yaron2 yaron2 modified the milestones: v1.15, v1.16 Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working P2 pinned
Projects
Status: Done
Status: Backlog
Development

No branches or pull requests

7 participants