| ▲ | Waitgroups: What they are, how to use them and what changed with Go 1.25(mfbmina.dev) |
| 61 points by mfbmina 2 days ago | 47 comments |
| |
|
| ▲ | tombert 2 days ago | parent | next [-] |
| Forgive a bit of ignorance, it's been a bit since I've touched Go, but this looks awfully similar to a Java CountdownLatch [1]. Is this just a glorified Go port of that or am I missing something vital here? [1] https://docs.oracle.com/javase/8/docs/api/java/util/concurre... |
| |
| ▲ | arccy 2 days ago | parent | next [-] | | CountDownLatch looks like it can only count down? the go one you can add/remove at will | | | |
| ▲ | xh-dude 2 days ago | parent | prev [-] | | Pretty much. It’s a counting semaphore underneath. |
|
|
| ▲ | stefanos82 2 days ago | parent | prev | next [-] |
| Personally I wished they had it backported to previous versions too, because it's rather convenient! What is quite sad is that we cannot add it ourselves as it's so simple of what they have done: func (wg *WaitGroup) Go(f func()) {
wg.Add(1)
go func() {
defer wg.Done()
f()
}()
}
|
| |
| ▲ | evanelias 2 days ago | parent | next [-] | | You can just use golang.org/x/sync/errgroup instead, which has always provided this style of use. errgroup also has other niceties like error propagation, context cancellation, and concurrency limiting. | | |
| ▲ | Cyph0n 2 days ago | parent | next [-] | | Context cancellation is not always desirable. I personally have been bitten multiple times by the default behavior of errgroup. | | |
| ▲ | CamouflagedKiwi 2 days ago | parent [-] | | You have to explicitly propagate the group's context if you want it to cancel. You can just not do that if you don't want - there certainly are cases for that. | | |
| ▲ | Cyph0n a day ago | parent [-] | | But if you’re looking at the package API, there is no alternative constructor for Group, which makes it seem as if the most common default is to construct a Group using WithContext. Also, 2/3 of the examples use WithContext. My recommendation would be to have a NewGroup function or equivalent that returns an empty group to surface it as an alternative to WithContext. | | |
| ▲ | evanelias 20 hours ago | parent [-] | | > My recommendation would be to have a NewGroup function or equivalent that returns an empty group That goes against common practices in Golang, articulated in the second paragraph of https://go.dev/doc/effective_go#allocation_new among many other places. Also the errgroup documentation specifically says "A zero Group is valid, has no limit on the number of active goroutines, and does not cancel on error." And, as you noted, one of the examples doesn't use WithContext. | | |
| ▲ | Cyph0n 20 hours ago | parent [-] | | That’s one of my Go pet peeves - zero/default structs are sometimes valid and sometimes not, and the only way to know is to dig into the docs. I prefer the API to speak for itself, ideally enforced by the language/compiler. |
|
|
|
| |
| ▲ | porridgeraisin 2 days ago | parent | prev [-] | | errgroup cancels the whole task if even one subtask fails however. That is not desirable always. | | |
| ▲ | Groxx 2 days ago | parent | next [-] | | It does not, which is easy to verify from the source. Every func passed in is always run (with the exception of TryGo which is explicitly "maybe"). At best, using the optional, higher-effort errgroup.WithContext will cancel the context but still run all of your funcs. If you don't want that for one of the funcs, or some component of them, just don't use the context. | |
| ▲ | evanelias 2 days ago | parent | prev [-] | | If the context cancellation is undesirable, you just choose not to use WithContext, as the sibling comment mentions. You could also just make your subtask function return nil always, if you just want to get the automatic bookkeeping call pattern (like WaitGroup.Go from Golang 1.25), plus optional concurrency limiting. Also note, even if a subtask function returns an error, the errgroup Wait blocking semantics are identical to those of a WaitGroup. Wait will return the first error when it returns, but it doesn't unblock early on first error. |
|
| |
| ▲ | CamouflagedKiwi 2 days ago | parent | prev | next [-] | | They basically don't backport anything for Go, but the quid pro quo for that is that the backwards compatibility is pretty strong so upgrades should be safe. I have seen one serious issue from it, but still it's the language I'm the most confident to do an upgrade and expect things to Just Work afterwards. | |
| ▲ | cedws 2 days ago | parent | prev [-] | | You can wrap WaitGroup if you really want to. | | |
| ▲ | stefanos82 2 days ago | parent [-] | | Can you provide an example please? | | |
| ▲ | listeria 2 days ago | parent [-] | | something like this would do it: package main
import (
"sync"
"time"
)
type WaitGroup struct {
sync.WaitGroup
}
func (wg *WaitGroup) Go(fn func()) {
wg.Add(1)
go func() {
defer wg.Done()
fn()
}()
}
func main() {
var wg WaitGroup
wg.Go(func() { time.Sleep(1 * time.Second) })
wg.Wait()
}
| | |
|
|
|
|
| ▲ | porridgeraisin 2 days ago | parent | prev | next [-] |
| Love this. Majority of concurrency in a usual web service is implemented using waitgroups IME (see below) This will greatly simplify it. var wg sync.WaitGroup
wg.Add(1)
go func(){
callService1(inputs, outParameter)
wg.Done()
}
// Repeat for services 2 through N
wg.Wait()
// Combine all outputs
BTW, this can already be done with a wrapper type type WaitGroup struct { sync.WaitGroup }
func (wg *WaitGroup) Go(fn func()) {
wg.Add(1)
go func() {
fn()
wg.Done()
}()
}
Since you're doing struct embedding you can call methods of sync.WaitGroup on the new WaitGroup type as well. |
|
| ▲ | danenania 2 days ago | parent | prev | next [-] |
| I like WaitGroup as a concept, but I often end up using a channel instead for clearer error handling. Something like: errCh := make(chan error)
for _, url := range urls {
go func(url string){
errCh <- http.Get(url)
}(url)
}
for range urls {
err := <-errCh
if err != nil {
// handle error
}
}
Should I be using WaitGroup instead? If I do, don't I still need an error channel anyway—in which case it feels redundant? Or am I thinking about this wrong? I rarely encounter concurrency situations that the above pattern doesn't seem sufficient for. |
| |
| ▲ | c0balt 2 days ago | parent | next [-] | | You would probably benefit from errgroup, https://pkg.go.dev/golang.org/x/sync/errgroup But channels already do the waiting part for you. | | | |
| ▲ | javier2 2 days ago | parent | prev [-] | | How you handle err here? If you return, the go routines will leak | | |
| ▲ | danenania 2 days ago | parent [-] | | Ah, good point—should be using a buffered channel to avoid that: errCh := make(chan error, len(urls))
| | |
| ▲ | unsnap_biceps 2 days ago | parent [-] | | buffered channels won't help here. That's just how many results can be buffered before the remaining results can be added to the channel. It doesn't wait until all of them are done before returning a result to the consumer. | | |
| ▲ | danenania 2 days ago | parent [-] | | > It doesn't wait until all of them are done before returning a result to the consumer. Right, but it prevents goroutine leaks. In these situations I'm usually fine with bailing on the first error, but I grant that's not always desirable. If it's not, I would collect and join errors and return those along with partial results (if those are useful). |
|
|
|
|
|
| ▲ | nikolayasdf123 2 days ago | parent | prev | next [-] |
| > wg := sync.WaitGroup{} just `var wg sync.WaitGroup`, it is cleaner this way |
| |
| ▲ | fozdenn 2 days ago | parent | next [-] | | doesn't this point to a bigger problem that there are two ways of doing the same thing? | | |
| ▲ | nikolayasdf123 2 days ago | parent | next [-] | | no. it is different thing. container-agnostic zero value vs struct init. | |
| ▲ | unsnap_biceps 2 days ago | parent | prev [-] | | multiple ways of doing something isn't inherently bad. For example, if you want to set a variable to the number of seconds in seven hours, you could just set the variable to 25200, or you could set it to 60 * 60 * 7. The expanded version might be clearer in the code context, but in the end they do exactly the same thing. | | |
| ▲ | pests 2 days ago | parent | next [-] | | Your math equation turned the asterisks into italics. | | | |
| ▲ | nikolayasdf123 2 days ago | parent | prev [-] | | hold on, why would you have 7 hours? dont you mean a week? 60 x 60 x 24 x 7 ? or at lest 8 hours? 7 hours is just odd | | |
| ▲ | unsnap_biceps 15 hours ago | parent [-] | | The goal was to pick an arbitrary number that wasn't well known. There was no intention behind the specific value chosen. |
|
|
| |
| ▲ | dwb 2 days ago | parent | prev | next [-] | | Why? | | |
| ▲ | nikolayasdf123 2 days ago | parent [-] | | zero value. container-agnostic initialization. say your type is not struct anymore, you would not have to change the way you intialize it. what you care here is zero value, and let the type figure out that it is zero and use methods appropriately. and it is just more clean this way here is google guideline: https://google.github.io/styleguide/go/best-practices#declar... | | |
| ▲ | dwb 2 days ago | parent [-] | | That is a much better argument than saying it is "more clean", which doesn't mean anything. I don't necessarily agree, because I don't think zero values are a good feature of the language, and even if they were this is a completely trivial case. But at least I don't have to work out what "cleanliness" is. |
|
| |
| ▲ | mr90210 2 days ago | parent | prev [-] | | Oh you are one of those. The nit picker. This is not at a PR review mate. | | |
| ▲ | nikolayasdf123 2 days ago | parent [-] | | "one of those", name calling, telling me what to say, cool it down a little. touch some grass. and hopefully you will see beauty in Go zero-values :P |
|
|
|
| ▲ | a-poor 2 days ago | parent | prev [-] |
| This means you can't pass variables in as function arguments. Even the example in the official go docs doesn't handle the scope correctly: func main() {
var wg sync.WaitGroup
var urls = []string{
"http://www.golang.org/",
"http://www.google.com/",
"http://www.example.com/",
}
for _, url := range urls {
// Launch a goroutine to fetch the URL.
wg.Go(func() {
// Fetch the URL.
http.Get(url)
})
}
// Wait for all HTTP fetches to complete.
wg.Wait()
}
https://pkg.go.dev/sync#example-WaitGroupYou need to use this pattern instead: for _, url := range urls {
url := url
// ...
|
| |
| ▲ | jeremyloy_wt 2 days ago | parent | next [-] | | This isn’t necessary anymore as of Go 1.22 https://go.dev/blog/loopvar-preview | |
| ▲ | 9rx 2 days ago | parent | prev [-] | | > This means you can't pass variables in as function arguments. Well, you could... for _, url := range urls {
wg.Go(func(u string) func() {
return func() {
http.Get(u)
}
}(url))
}
> You need to use this pattern insteadWhy? Seems rather redundant. It is not like WaitGroup.Go exists in earlier versions. |
|