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

Make it possible to keep workflows in worker cache also if they're only queried #1716

Open
recht opened this issue Nov 19, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@recht
Copy link
Contributor

recht commented Nov 19, 2024

Is your feature request related to a problem? Please describe.

Currently, when sending a query to a workflow that is in running state but not active on any worker, a worker will receive the request, replay history, process the query, and forget everything again. On next query the same thing happens. This is in contrast to starting the workflow from scratch without restarting the worker. Then the workflow is in the worker cache, ready to serve queries. Same if the worker has restarted, but workflow state has changed so that the workflow got loaded into the cache.

Because replays can get a bit expensive, especially if encryption/offloading codecs are used (which we do), it would be nice if querying a workflow would add it to the worker sticky cache.

Describe the solution you'd like
A config property that allows us to specify that workflows that are queried should also be inserted into the cache.

While debugging locally I did this:

diff --git a/internal/internal_task_handlers.go b/internal/internal_task_handlers.go
index 8312587..342f7f4 100644
--- a/internal/internal_task_handlers.go
+++ b/internal/internal_task_handlers.go
@@ -812,7 +812,7 @@ func (wth *workflowTaskHandlerImpl) GetOrCreateWorkflowContext(
                        return
                }
 
-               if wth.cache.MaxWorkflowCacheSize() > 0 && task.Query == nil {
+               if wth.cache.MaxWorkflowCacheSize() > 0 {
                        workflowContext, _ = wth.cache.putWorkflowContext(runID, workflowContext)
                        workflowContext.Lock()
                        workflowContext.cached = true

It's unclear if that's the right solution or if that has other unintended consequences, but at least it keeps the workflow in the sticky worker cache, making queries much faster.

Describe alternatives you've considered
Doing less queries, or faster codecs. Regardless, it seems wasteful to have to replay on every single query.

@recht recht added the enhancement New feature or request label Nov 19, 2024
@Quinn-With-Two-Ns
Copy link
Contributor

I believe this is a limitation on the server. Query only tasks cannot set the sticky cache attribute so the worker cache would not actually be used even if the SDK did keep it in cache. There is an issue to add this feature on the server side, once this is closed the SDK could keep the workflow in cache.

@recht
Copy link
Contributor Author

recht commented Nov 20, 2024

Ok, thanks - so I guess the only reason why it made any difference for me was that I was only running a single worker so queries always went to that worker?

@Quinn-With-Two-Ns
Copy link
Contributor

There are multiple ways a query can be delivered to a workflow, if the worker was already in the cache it will be used. The problem is direct queries cannot add the workflow to the sticky task queue so the server would always send the whole history down again causing a replay.

@recht
Copy link
Contributor Author

recht commented Dec 2, 2024

Ok, but with the change I tried there was no replay after the first query - it's possible that the worker receives all the data, but it doesn't actually replay (checked by adding print statements in the workflow code and observing with and without the change)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants