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

[Bug] Workflow Update hangs on time skipping environment #179

Open
alrz opened this issue Jan 21, 2024 · 5 comments
Open

[Bug] Workflow Update hangs on time skipping environment #179

alrz opened this issue Jan 21, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@alrz
Copy link

alrz commented Jan 21, 2024

What are you really trying to do?

Describe the bug

Minimal Reproduction

[Workflow]
public class TestWorkflow
{
   [WorkflowUpdate]
   public Task Update() => Task.CompletedTask;
   [WorkflowRun]
   public Task RunAsync() => Workflow.DelayAsync(TimeSpan.FromSeconds(3));
}
public async Task Test() {
    await using var env = await WorkflowEnvironment.StartTimeSkippingAsync();
    using var worker = new TemporalWorker(
        env.Client,
        new TemporalWorkerOptions($"tq-{Guid.NewGuid()}").
            AddWorkflow<TestWorkflow>());
    await worker.ExecuteAsync(async () =>
    {
        var h = await env.Client.StartWorkflowAsync(
            (TestWorkflow wf) => wf.RunAsync(),
            new(id: $"workflow-{Guid.NewGuid()}", taskQueue: worker.Options.TaskQueue!));
        await h.ExecuteUpdateAsync(wf => wf.Update());
        await h.GetResultAsync();
    });
}

Environment/Versions

  • OS and processor: Windows x64
  • Temporal Version: Temporal nuget package 1.0.0
  • Are you using Docker or Kubernetes or building Temporal from source? N/A

Additional context

@cretz
Copy link
Member

cretz commented Jan 22, 2024

I have replicated and we have an outstanding issue in the Java SDK (where our time-skipping test server implementation lives): temporalio/sdk-java#1903. Updates are experimental/unstable currently, but in the meantime you should be able to use a normal local server if you must use them and need to test them. I will leave this issue open pending a fix.

@alrz
Copy link
Author

alrz commented Jan 22, 2024

where our time-skipping test server implementation lives

Good to know, thanks! BTW is there any issue to track progress on workflow updates? If I'm not mistaken it seems like this is not sdk-related and the server implementation is actually experimental for the time being.

I thought signals would handle concurrent ops but turns out I had to use updates for this. The workaround is to use a combination of query/signal but I'm not comfortable doing that because the logic would be outside of the workflow itself and that would be hard to reason about.

@cretz
Copy link
Member

cretz commented Jan 23, 2024

BTW is there any issue to track progress on workflow updates?

I am not aware of an issue to track this. You can see the status at the top of https://docs.temporal.io/workflows#update.

I thought signals would handle concurrent ops but turns out I had to use updates for this.

Signals can handle concurrent pieces, they just don't have results to return

The workaround is to use a combination of query/signal but I'm not comfortable doing that because the logic would be outside of the workflow itself and that would be hard to reason about.

There's also an advanced approach to "push" results to activity workers hosted by the caller. See https://github.com/temporalio/samples-go/tree/main/reqrespactivity for a description of the pattern.

@alrz
Copy link
Author

alrz commented Jan 24, 2024

Signals can handle concurrent pieces, they just don't have results to return

Right, I was confused by "asynchronous" mentioned in the docs. I supposed that means we don't wait for the signal method to complete on the client side, however the call itself is synchronized inside the workflow.

Just one more question, where do we expect the signal to be received by the workflow? If I had to guess, between each await there's a chance that a signal would be received and get executed and then a workflow-run would be scheduled so we need to keep track of the state between each await and check it either in the update validator or via a query, is that correct?

Context: I'm trying to handle a concurrent timeout and signal using WhenAny(Delay,WaitCondition), the happy path is that the signal would be received before the timeout. But if the timeout occurred beforehand we need to run a compensating operation.

@cretz
Copy link
Member

cretz commented Jan 24, 2024

Just one more question, where do we expect the signal to be received by the workflow? If I had to guess, between each await there's a chance that a signal would be received and get executed and then a workflow-run would be scheduled so we need to keep track of the state between each await and check it either in the update validator or via a query, is that correct?

This is deviating from the issue and it may be better for a discussion place like the #dotnet-sdk channel in https://t.mp/slack or our forums.

Yes, signals are received from server at potentially any yield point. Yes, most users simply set a class field from their signal and have their Workflow.WaitConditionAsync wait for that value to get set to a certain value. Your use case of wait-for-signal-or-timeout is a very normal one and the proper approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants