microsoft / durabletask-java

Java SDK for Durable Functions and the Durable Task Framework
MIT License
13 stars 7 forks source link

[Eternal Orchestrator] Timers more than 7days giving error #114

Closed kanupriya15025 closed 1 year ago

kanupriya15025 commented 1 year ago

I have implemented a scheduler out of an eternal orchestrator. This scheduler evaluates a cron expression and calculates the next execution time based on it. This eternal orchestrator now needs to sleep till this next execution time. I was reading the documentation on timers in durable framework, and according to this there is no limitation on timers for java, however when I tried it I am getting this error : Microsoft.WindowsAzure.Storage: The argument 'initialVisibilityDelay' is larger than maximum of '7.00:00:00' (Parameter 'initialVisibilityDelay').

Can you help me understand if this is a bug or am I doing something wrong. Here the delay I am generating is of 44 days.

cgillum commented 1 year ago

This sounds like a bug that needs to be investigated.

cgillum commented 1 year ago

@kanupriya15025 in the meantime, you can work around this issue by scheduling a series of shorter timers, like this (which is untested, but hopefully illustrates the idea):

private static Task<Void> createLongTimer(TaskOrchestrationContext ctx, Duration duration) {
    Instant startTime = ctx.getCurrentInstant();
    Instant fireAt = startTime.minus(duration);

    Duration maxInterval = Duration.ofDays(3);
    Duration remainingTime = duration;
    while (remainingTime.compareTo(maxInterval) > 0) {
        ctx.createTimer(maxInterval).await();
        remainingTime = Duration.between(ctx.getCurrentInstant(), fireAt);
    }

    return ctx.createTimer(remainingTime);
}

The code above breaks up a large duration into intervals of no longer than 3 days.

/cc @kaibocai @shreyas-gopalakrishna I think we should bake this logic into createTimer() implementation.

kanupriya15025 commented 1 year ago

@cgillum Do you have an idea on when this fix can be made available?

cgillum commented 1 year ago

@kanupriya15025 I don’t, unfortunately, since I’m not directly involved in the Java SDK release process, but it’s a simple enough fix that I don’t expect it will take the team too long.

kanupriya15025 commented 1 year ago

@ChrisRomp Can we create an ADO ticket to track this bug?

cgillum commented 1 year ago

@kamperiadis this might be a good one for you to take a look at if you have the bandwidth.

ChrisRomp commented 1 year ago

@kanupriya15025 does setting a timer based on timestamp help as a workaround? #103 #106

kanupriya15025 commented 1 year ago

I still the documentation and sdk still working with just duration. Can you share the usage or sdk version?

kanupriya15025 commented 1 year ago

@ChrisRomp just a bump on the issue

kanupriya15025 commented 1 year ago

@ChrisRomp Looks like that feature isn't available yet as confirmed on the ticket. Can we look at a way to expedite this?

ChrisRomp commented 1 year ago

@kanupriya15025 - looking at the doc you linked, it mentions limitations of long timers depending on the storage provider used. By default that's Azure Storage queues which uses the visibilityTimeout parameter to control when the message is picked up for processing. That parameter has a limit of 7 days (docs) which is what we're seeing here.

I'd check out @cgillum's suggestion around multiple, shorter timers. Once #106 is released, you could determine a specific time based on your longer duration and use that instead of a delay.

It's also possible the SDK could be extended to abstract the creation of multiple, shorter timers to create timers longer than 7d (other language SDKs might do this already?). I'll leave this to the eng team to prioritize that as a feature request.