Comment

Comment by knorker

The downside to that more "not idiomatic Go" example is that you lose useful stack traces, and it complicates contexts (timeouts, credentials, tracing, etc… etc…).

For this reason the article I would say is also suboptimal, and is non-idiomatic. It's something you'd do in other languages, but should be avoided in Go.

In my opinion you should just start a goroutine. The goroutine could block on a semaphore/channel to limit concurrency, but there's nothing inherently more costly about having the goroutines themselves be the queue instead of like the article having a list.

Yes, it'll probably take a bit more memory to create a goroutine than to add to a list, but almost always I'll take that to get more correct behavior.

But also, your suggestion doesn't handle the requirement "Calling the service should not block the caller", does it?

The article overengineered, by far. You don't need infrastructure for this. It's just:

  for work := range workGenerator() {
     go process(work)
  }
to limit concurrency, create a semaphore and just block either before starting goroutine (thereby blocking caller):

  sem := sync.NewSemaphore(runtime.NumCPU())
  for work := range workGenerator() {
    work:=work
    sem.Acquire(1)
    go func() {
      defer sem.Release()
      process(work)
    }()
  }
Or in the goroutine, to not block the caller:

  sem := sync.NewSemaphore(runtime.NumCPU())
  for work := range workGenerator() {
    work:=work
    go func() {
      sem.Acquire(1)
      defer sem.Release()
      process(work)
    }()
  }
You don't need the infrastructure from the article and, as I described, it's actually hurting.
parent knorker 5 years ago

Discussion

Live from the official Hacker News API

A modern reader for Hacker News · SvelteKit + Skeleton