Fix Duplicate workflow reminder execution in Scheduler mode #7983

JoshVanL · 2024-08-12T22:37:11Z

Workflow reminders must have a period set of repeating forever. This is what fundamentally ensures the durability of Workflows.

PR updates all workflow reminders to have an infinite period.

Workflow reminders repeat execution every 1 minute. In some cases, such as during extremely high load, a workflow reminder may execute whilst a previous execution is still in progress. This leads to situations such as double activity invocation. This patch adds a duplicate internal reminder detector, which silently returns if a duplicate reminder is detected.

Removes the background contexts to Scheduler APIs. The Scheduler API server must respect inbound client contexts.

Removes unused --placement flag from the Scheduler.

Workflow reminders must have a period set of repeating forever. This is what fundamentally ensures the durability of Workflows. PR updates all workflow reminders to have an infinite period. Workflow reminders repeat execution every 1 minute. In some cases, such as during extremely high load, a workflow reminder may execute whilst a previous execution is still in progress. This leads to situations such as double activity invocation. This patch adds a duplicate internal reminder detector, which silently returns if a duplicate reminder is detected. Removes the background contexts to Scheduler APIs. The Scheduler API server must respect inbound client contexts. Removes unused `--placement` flag from the Scheduler. Signed-off-by: joshvanl <me@joshvanl.dev>

cicoyle · 2024-08-12T22:42:11Z

I've confirmed the db is clean after several thousand workflow iterations and the reminders are not triggered indefinitely even while removing the Repeat from 2, should be good with this diff 👍🏻

pkg/scheduler/server/api.go

pkg/runtime/scheduler/streamer.go

pkg/actors/actors.go

cmd/scheduler/options/options.go

pkg/actors/actors.go

Signed-off-by: joshvanl <me@joshvanl.dev>

artursouza · 2024-08-13T13:29:22Z

Approved, pending validation to pass.

Signed-off-by: joshvanl <me@joshvanl.dev>

cicoyle · 2024-08-13T13:57:17Z

LGTM. Just ran my local test and saw the duplicated triggering since the run activity took longer than a minute and saw the log about ignoring it since one is still active, then deleting it. Post workflow run of 4k workflow iterations my db was clean still 👍🏻

scheduler-0-1      | time="2024-08-13T13:49:41.788503301Z" level=debug msg="Triggering job: actorreminder||default||dapr.internal.default.workflow-a.activity||2953-1::0::1||run-activity" instance=bcbc20ef8d3b scope=dapr.scheduler.server type=log ver=edge
scheduler-0-1      | time="2024-08-13T13:50:41.004162967Z" level=debug msg="Triggering job: actorreminder||default||dapr.internal.default.workflow-a.activity||2953-1::0::1||run-activity" instance=bcbc20ef8d3b scope=dapr.scheduler.server type=log ver=edge

workflow-dapr-a-1  | time="2024-08-13T13:50:51.394679014Z" level=debug msg="Executing reminder for internal actor 'dapr.internal.default.workflow-a.activity||2953-1::0::1||run-activity'" app_id=workflow-a instance=e455363315f3 scope=dapr.runtime.actor type=log ver=edge
workflow-dapr-a-1  | time="2024-08-13T13:50:51.394693722Z" level=debug msg="Duplicate concurrent reminder invocation detected for 'dapr.internal.default.workflow-a.activity||2953-1::0::1||run-activity', likely due to long processing time. Ignoring in favour of the active invocation" app_id=workflow-a instance=e455363315f3 scope=dapr.runtime.actor type=log ver=edge
workflow-dapr-a-1  | time="2024-08-13T13:50:51.394715305Z" level=debug msg="Deleting reminder which was cancelled: dapr.internal.default.workflow-a.activity||2953-1::0::1||run-activity" app_id=workflow-a instance=e455363315f3 scope=dapr.runtime.actor type=log ver=edge

JoshVanL requested review from a team as code owners August 12, 2024 22:37

cicoyle previously approved these changes Aug 12, 2024

View reviewed changes