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

Allow to disable workflow deadlock detector #6546

Open
itayd opened this issue Sep 24, 2024 · 3 comments
Open

Allow to disable workflow deadlock detector #6546

itayd opened this issue Sep 24, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@itayd
Copy link

itayd commented Sep 24, 2024

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

We need to run some function that might block for a while (up to 10s, usually) during a Temporal workflow. That function cannot receive a Temporal workflow context. It also has to run every time the workflow runs, including replays. Further operations during this workflow rely on this function being run first.

Currently when we do this, the deadlock detection mechanism panics.

Describe the solution you'd like

Allow to disable the deadlock detection for a specific scope. Possible just making https://github.com/temporalio/sdk-go/blob/master/internal/workflow_deadlock.go#L51 and its friend public.

Describe alternatives you've considered

We tried running the function in a separate go routine, and while it's running sleeping on Temporal. The problem is that this is non-deterministic, and we don't know how much Sleep we need to get.

Additional context

I initially really really really didn't want to open this issue. I really think that the deadlock detection is a Good Thing. I discussed this with @mfateev and Chad (no idea what's his alias here), and at the end came to a conclusion that there's no avoiding this.

@mfateev suggested (ab)using the DataConverterWithoutDeadlockDetection, but I'd rather do it in a more polite fashion.

@itayd itayd added the enhancement New feature or request label Sep 24, 2024
@itayd
Copy link
Author

itayd commented Sep 24, 2024

Just for fun, this is what I can do to abuse the data convertor:

import (
	"context"

	commonpb "go.temporal.io/api/common/v1"
	"go.temporal.io/sdk/workflow"
)

type deadlockDetectorAbuser func()

func (deadlockDetectorAbuser) ToPayload(interface{}) (*commonpb.Payload, error)      { return nil, nil }
func (deadlockDetectorAbuser) FromPayload(*commonpb.Payload, interface{}) error      { return nil }
func (deadlockDetectorAbuser) ToPayloads(...interface{}) (*commonpb.Payloads, error) { return nil, nil }
func (deadlockDetectorAbuser) FromPayloads(*commonpb.Payloads, ...interface{}) error { return nil }
func (deadlockDetectorAbuser) ToString(*commonpb.Payload) string                     { return "" }

func (dd deadlockDetectorAbuser) ToStrings(*commonpb.Payloads) []string {
	dd()
	return nil
}

func WithoutDeadlockDetection(ctx workflow.Context, f func())  {
	cvt := workflow.DataConverterWithoutDeadlockDetection(deadlockDetectorAbuser(f))
	cvt = cvt.(workflow.ContextAware).WithWorkflowContext(ctx)
	_ = cvt.ToStrings(nil)
}

edit: need to supply workflow context to the converter.

@itayd
Copy link
Author

itayd commented Sep 24, 2024

tbh, it's so ugly i kinda like it.

@itayd
Copy link
Author

itayd commented Sep 24, 2024

Proposed temporalio/sdk-go#1647.

itayd added a commit to autokitteh/autokitteh that referenced this issue Sep 24, 2024
pashafateev pushed a commit to autokitteh/autokitteh that referenced this issue Sep 24, 2024
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

1 participant