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 parallel steps #51

Merged
merged 3 commits into from
Sep 18, 2024
Merged

Add parallel steps #51

merged 3 commits into from
Sep 18, 2024

Conversation

amh4r
Copy link
Contributor

@amh4r amh4r commented Aug 6, 2024

Description

Add parallel steps using a new group.Parallel function. The new group package lives in an experimental directory, where it will remain until we feel it's stable.

func MyFn(ctx context.Context, input inngestgo.Input[any]) (any, error) {
	results := group.Parallel(
		ctx,
		func(ctx context.Context) (any, error) {
			return step.Run(ctx, "1a", func(ctx context.Context) (string, error) {
				return "1a output", nil
			})
		},
		func(ctx context.Context) (any, error) {
			return step.Run(ctx, "1b", func(ctx context.Context) (string, error) {
				return "1b output", nil
			})
		},
	)

	out := make([]string, len(results))
	for i, r := range results {
		if r.Error != nil {
			return nil, r.Error
		}
		out[i] = r.Value.(string)
	}
	return out, nil
}

@amh4r amh4r changed the title Add parallel steps RFC: Add parallel steps Aug 6, 2024
@amh4r amh4r force-pushed the poc-parallel-steps branch from 15d5a0e to d072f8d Compare August 7, 2024 23:57
@amh4r amh4r marked this pull request as ready for review August 7, 2024 23:59
@amh4r amh4r changed the title RFC: Add parallel steps Add parallel steps Aug 8, 2024

results := []Result{}
isPlanned := false
ch := make(chan struct{}, 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used as a waitgroup?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's effectively a waitgroup of 1. We want to run each step sequentially but also in a goroutine so that we can recover the control hijack

Comment on lines 32 to 33
// TODO: What to do here?
fmt.Println("TODO")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just repanic for now? I suppose this might be where we capture the non-Inngest panic and could return it as an error to Inngest in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should recover the panic and repanic it outside the goroutine? I think that'll let our normal non-control-flow panic recovery logic work

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh yeah if we already have something that captures panics then that'd be awesome.

@amh4r amh4r merged commit 3e55d43 into main Sep 18, 2024
7 checks passed
@amh4r amh4r deleted the poc-parallel-steps branch September 18, 2024 19:07
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 this pull request may close these issues.

3 participants