top | item 34348248

(no title)

tmerr | 3 years ago

> FWIW, the default concurrency has been changed to "unlimited" since the 0.1.0 release.

Nice! Will that end up on Github?

> Out of curiousity, what do you want

Most often I want to return just the first error. Some reasons: (1) smaller error messages passed across RPC boundaries (2) original errors can be inspected as intended (e.g. error codes) (3) when the semantics are to cancel after the first error, the errors that come after that are just noise.

A couple other thoughts

  - I think the non-conc comparison code is especially hairy since it's using goroutine pools. There's nothing wrong with that and it's fast, just not the easiest to work with. Often goroutine overhead is negligible and I would bound concurrency in dumber ways e.g. by shoving a sync.Semaphore into whatever code I otherwise have
  - I like that errgroup has .Go() block when concurrency limit is reached. Based on a quick skim of the code I think(?) conc does that too, but it could use documentation

discuss

order

camdencheek|3 years ago

Thanks for the feedback!

> Will that end up on Github?

It's already there! I just haven't cut a release since the change.

> Most often I want to return just the first error.

In many cases, I do too, which is why (*pool).WithFirstError() exists :)

> original errors can be inspected as intended

If you're using errors.Is() or errors.As(), error inspection should still work as expected.

> Often goroutine overhead is negligible and I would bound concurrency in dumber ways

Yes, definitely. And that's what I've always done too. However, I've found it's surprisingly easy to get subtly wrong, especially when modifying code I didn't write, and even more especially if I want to propagate panics (which I do, though that seems to be a somewhat controversial opinion in this thread). Conc is intended to be a well-known pattern that I don't have to think about too much when using it.

> I think(?) conc does that too, but it could use documentation

It does! I'll update the docs to make that more clear.