Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate pool in favor of errgroup? #2556

Closed
imjasonh opened this issue Jul 22, 2022 · 3 comments · Fixed by #2568
Closed

Deprecate pool in favor of errgroup? #2556

imjasonh opened this issue Jul 22, 2022 · 3 comments · Fixed by #2568
Labels
area/API kind/question triage/accepted Issues which should be fixed (post-triage)

Comments

@imjasonh
Copy link
Member

AIUI, pkg/pool has nearly the same interface and use case as errgroup:

  • run some functions in parallel using goroutines
  • stop and return an error when any goroutine fails
  • take a context.Context so the whole group can be stopped if the context is cancelled
  • be able to limit concurrency (typically to runtime.NumCPU)

It might have been that pool provided more functionality than errgroup at one point in time, but with errgroup getting SetLimit recently, I don't think that's the case anymore.

Maintaining this code yourself is an option, but concurrency can get tricky, and if possible it'd probably be worth making it someone else's problem. Future performance improvements may get made to errgroup which we could just pick up.

As far as external users, cosign depends on pkg/pool, until sigstore/cosign#2092, and it looks like mink uses it too, but I can send a PR for that too.

/kind question
/area API

btw if this sounds good I'm happy to do the work to migrate knative's own usages, and help anybody else using it to migrate.

@dprotaso
Copy link
Member

Let's drop the uses here:
https://pkg.go.dev/knative.dev/pkg/pool?tab=importedby

and just delete this package

@dprotaso
Copy link
Member

errgroup getting SetLimit is great

@dprotaso
Copy link
Member

/triage accepted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API kind/question triage/accepted Issues which should be fixed (post-triage)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants