ReusableGoroutinesPool: Add optional action when Go is called after Close

[julienduchesne]

Default behavior remains the same

There is a data race in mimir where the pool can be closed and Go is still called on it:
https://github.com/grafana/mimir/blob/0c6070552517bda1ccb97b8fc84ca50c591a71f7/pkg/distributor/distributor.go#L528-L532

Rather than handling this in Mimir, this can be handled in the util directly

In Mimir, we'll be able to define this pool as wp := concurrency.NewReusableGoroutinesPool(cfg.ReusableIngesterPushWorkers).WithClosedAction(concurrency.ErrorWhenClosed) and we can check for errors wherever we call Go on it.

Note: By expanding this utility a bit more, I can also reuse it for the memberlist TCP transport (#525). It's a very similar worker pool

[colega]

I feel it's too much logic just to handle the fact that our service shutdown process doesn't have proper dependencies. Can we just fix it in mimir, and instead of this being a subservice, close the pool when distributor's service is closed?

It feels wrong to me that this component now has almost more logic to handle the fact that someone can't properly handle it's lifecycle than it's main business logic.

[colega]

OTOH, can you explain what's the data race here? I would assume this component would just spawn goroutines instead of using workers once closed. How that causes a data race?

[julienduchesne]

OTOH, can you explain what's the data race here? I would assume this component would just spawn goroutines instead of using workers once closed. How that causes a data race?

Seems to be on the close sequence. Closing the channel before the distributors are done pushing. Seems to be an issue with closing the services in the correct order

I feel it's too much logic just to handle the fact that our service shutdown process doesn't have proper dependencies. Can we just fix it in mimir, and instead of this being a subservice, close the pool when distributor's service is closed?

Yeah, definitely possible. I thought that making this component more robust would be nice, rather than playing with dependency orders in Mimir (making sure this service is closed after the distributor), but either is possible

[julienduchesne]

I think this just needs a mutex, not all the new logic (like this: grafana/mimir#9585). I redid this PR here: #607