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

add support for Task with context as the function argument #6

Open
emar-kar opened this issue Jun 7, 2024 · 3 comments
Open

add support for Task with context as the function argument #6

emar-kar opened this issue Jun 7, 2024 · 3 comments

Comments

@emar-kar
Copy link
Contributor

emar-kar commented Jun 7, 2024

In current realisation in case if any of the tasks is a long running function, (s *Scheduler) Wait() might hang for a long period of time, before exiting.
If the job function hangs, it will consume the resources, staying as a running goroutine forever. It will not hang the worker, since it will return on timeout, but it will still be in scheduler and consume resources.

I suggest to add TaskWithContext instead of the regular Task and make it a standard. It will allow to add such parameters for scheduler as the real timeout. Give caller more flexible controls of the task execution and will allow to actually gracefully stop the task.

@WangYihang
Copy link
Owner

@emar-kar Thanks for your contribution! I will check it out soon.

@WangYihang
Copy link
Owner

You are right, when f hangs and the timeout signal arrived, the resource in f won't be release. We need a method to pass the cancel signal to the task object.

Maybe we should change the signature of func Do() error in the Task interface?

// Task is an interface that defines a task
type Task interface {
	// Do starts the task, returns error if failed
	// If an error is returned, the task will be retried until MaxRetries
	// You can set MaxRetries by calling SetMaxRetries on the scheduler
	Do(ctx context.Context) error
}

And the RunWithTimeout may becomes like this.

func RunWithTimeout(f func(ctx context.Context) error, timeout time.Duration) error {
	ctx, cancel := context.WithTimeout(context.Background(), timeout)
	defer cancel()

	done := make(chan error, 1)

	go func() {
		done <- f(ctx)
	}()

	select {
	case <-ctx.Done():
		return ctx.Err()
	case err := <-done:
		return err
	}
}

Or we might need to add additional context for RunWithTimeout? Or just embed the context into the concrete Task struct?

What's your option on this change?

@emar-kar
Copy link
Contributor Author

Hello there, I'm currently looking into the code, to see what I can suggest here. I'll introduce my ideas for you a little bit later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants