Hacker News Reader

Hacker News

Comment by camdencheek | Hacker News Reader

Parent Full thread

camdencheek

> nice work

Thanks!

> The default concurrency GOMAXPROCS is almost never what I want.

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

> Aggregated errors are almost never what I want.

Out of curiousity, what do you want? There is an option to only keep the first error, and it's possible to unwrap the error to an array of errors that compose it if you just want a slice of errors.

> Using it would place a burden on any reader that isn't familiar with the library

Using concurrency in general places a burden on the reader :) I personally find using patterns like this to significantly reduce the read/review burden.

View on HN

> 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

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.

Reply on news.ycombinator.com