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

Jump to bottom

x/sync/errgroup: get all errors #23595

Closed
umairidris opened this issue Jan 28, 2018 · 3 comments
Closed

x/sync/errgroup: get all errors #23595

umairidris opened this issue Jan 28, 2018 · 3 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@umairidris
Copy link

umairidris commented Jan 28, 2018 •
edited

errgroup currently only allows the user to get the first error. It is sometimes desirable to get all errors from the goroutines launched.

Below are some recent examples where I was doing some uniform work across multiple machines and needed to use sync.WaitGroup rather than errgroup as I wanted to collect all errors.

Example 1: Running uniform cleanup across multiple machines and wanting to see each failure.

Example 2: Running uniform setup across multiple machines and needing only a subset to succeed in order for the pool to be available.

Proposal: One possible solution would be to add a WaitAll() function which returns []error rather than error.

func Cleanup(string) error {
	return errors.New("uh oh")
}

func main() {
	machs := []string{"mach1", "mach2"}

	var g Group
	for _, m := range machs {
		m := m
		g.Go(func() error {
			if err := Cleanup(m); err != nil {
				return fmt.Errorf("failed to cleanup %v: %v", m, err)
			}
			return nil
		})

	}
	if errs := g.WaitAll(); len(errs) > 0 {
		// do stuff with errs
		fmt.Fprintln(os.Stderr, errs)
	}
}

I am happy to work on sending a patch for this if the feature request and API make sense.

Thanks!
@gopherbot gopherbot added this to the Unreleased milestone Jan 28, 2018
@umairidris
Copy link
Author

/cc @bcmills

@bcmills
Copy link
Contributor

bcmills commented Jan 30, 2018

The purpose of the errgroup package is to coordinate error collection with cancellation, since that's fairly subtle to get right using sync.WaitGroup.
If you cancel the Context on the first error, the subsequent errors tend not to be meaningful (they're often either context.Canceled or other errors that arise as a side-effect of cancellation).

If you want all of the errors (and don't want to cancel the Context), then you may as well just use a sync.WaitGroup. It's not a lot of boilerplate, and not particularly error-prone.

Furthermore, at the moment the errgroup package uses space proportional to the maximum number of goroutines in flight at any time. A WaitAll method, on the other hand, would require space proportional to the total number of goroutines spawned in the group, which may be much larger (or even unbounded).

@umairidris
Copy link
Author

Thanks for your response. I see the library was created explicitly for the cancellation parts, while I was trying to reuse it for other use cases too. Using sync.WaitGroup is not ideal but not painful either so I will continue using it.

@golang golang locked and limited conversation to collaborators Feb 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@bcmills @umairidris @gopherbot