9

I am trying to track down some occasional Non-Deterministic workflow detected: TaskScheduledEvent: 0 TaskScheduled ... errors in a durable function project of ours. It is infrequent (3 times in 10,000 or so instances).

When comparing the orchestrator code to the constraints documented here there is one pattern we use that I am not clear on. In an effort to make the orchestrator code more clean/readable we use some private async helper functions to make the actual CallActivityWithRetryAsync call, sometimes wrapped in an exception handler for logging, then the main orchestrator function awaits on this helper function.

Something like this simplified sample:

[FunctionName(Name)]
public static async Task RunPipelineAsync(
    [OrchestrationTrigger]
    DurableOrchestrationContextBase context,

    ILogger log)
{
    // other steps

    await WriteStatusAsync(context, "Started", log);

    // other steps

    await WriteStatusAsync(context, "Completed", log);
}

private static async Task WriteStatusAsync(
    DurableOrchestrationContextBase context,
    string status,
    ILogger log
)
{
    log.LogInformationOnce(context, "log message...");
    try
    {
        var request = new WriteAppDocumentStatusRequest 
        {
            //...
        };

        await context.CallActivityWithRetryAsync(
            "WriteAppStatus",
            RetryPolicy,
            request
        );
    }
    catch(Exception e)
    {
        // "optional step" will log errors but not take down the orchestrator
        // log here
    }
}

In reality these tasks are combined and used with Task.WhenAll. Is it valid to be calling these async functions despite the fact that they are not directly on the context?

Gavin H
  • 10,274
  • 3
  • 35
  • 42

1 Answers1

6

Yes, what you're doing is perfectly safe because it still results in deterministic behavior. As long as you aren't doing any custom thread scheduling or calling non-durable APIs that have their own separate async callbacks (for example, network APIs typically have callbacks running on a separate thread), you are fine.

If you are ever unsure, I highly recommend you use our Durable Functions C# analyzer to analyzer your code for coding errors. This will help flag any coding mistakes that could result in Non-deterministic workflow errors.

UPDATE

Note: the current version of the analyzer will require you to add a [Deterministic] attribute to your private async function, like this:

[Deterministic]
private static async Task WriteStatusAsync(
    DurableOrchestrationContextBase context,
    string status,
    ILogger log
)
{
   // ...
}

This lets it know that the private async method is being used by your orchestrator function and that it also needs to be analyzed. If you're using Durable Functions 1.8.3 or below, the [Deterministic] attribute will not exist. However, you can create your own custom attribute with the same name and the analyzer will honor it. For example:

[Deterministic]
private static async Task WriteStatusAsync(
    DurableOrchestrationContextBase context,
    string status,
    ILogger log
)
{
   // ...
}

// Needed for the Durable Functions analyzer
class Deterministic : Attribute { }

Note, however, that we are planning on removing the need for the [Deterministic] attribute in a future release, as we're finding it may not actually be necessary.

Chris Gillum
  • 14,526
  • 5
  • 48
  • 61
  • Thank you for the link to that tool. I've installed that package and it has a list of concerns that I am looking at now. Interestingly enough, however, it is complaining when I `await` on a private async function from the orchestrator with a ` DF0107 'RunPipelineAsync(context, source, log)' violates the orchestrator deterministic code constraint. ` – Gavin H Nov 15 '19 at 18:47
  • I also tried doing `await DoNothingAsync();` with that function being ``` private static Task DoNothingAsync() { return Task.CompletedTask; } ``` and I still get that error – Gavin H Nov 15 '19 at 18:51
  • 1
    Ah, yes, I forgot to mention that the analyzer will require the use of a `[Deterministic]` attribute on your private async function. It's just a flag that tells the analyzer "please analyze this for orchestration code constraints". Unfortunately the latest 1.x releases of Durable Functions don't define a `[Deterministic]` attribute. However, you can define your own in your project and the analyzer will honor it. This is something we still need to properly document. Sorry about the confusion. – Chris Gillum Nov 15 '19 at 19:02
  • 1
    Just as a follow-up - I did find one potential issue that may have caused it. I had a function that would check for required app settings to be defined and throw an exception if not. We have before seen intermittent cases where app settings are null when they shouldn't be (may be https://github.com/Azure/azure-functions-host/issues/1577) so I'm thinking perhaps on replay that function threw when it didn't previously which could explain why it may be considered non-deterministic. I removed that check from the orchestrator and switched to using `Environment` for settings instead. – Gavin H Nov 17 '19 at 01:25
  • 2
    If you're checking for app settings or environment variables, make sure you do so in an activity function and not in your orchestrator function. Those do fall into the "non-deterministic" category (and the analyzer should be flagging those APIs). – Chris Gillum Nov 19 '19 at 01:07
  • Do you know if using something like MediatR package is considered safe? – sebsmgzz Sep 30 '22 at 19:20
  • @sebsmgzz sorry, I'm not familiar with MediatR. You may need to do your own analysis and testing using those packages to know if you're safe. I've definitely used some 3rd party packages which aren't safe. For example, anything that internally does `.ConfigureAwait(false)` is not safe. – Chris Gillum Oct 03 '22 at 21:46