[sync] x/sync/errgroup: make note of the default errgroup goroutine limit (unlimited)
Jean de Klerk (Gerrit)
Jean de Klerk has uploaded this change for review.
x/sync/errgroup: make note of the default errgroup goroutine limit (unlimited)
This is a question in the form of a CL. If I'm right about my guess, yay! If
not, I'll change the answer. Either way, we should document it.
Change-Id: I1926e6faf821a7adb6c1365d18432abb0f856367
---
M errgroup/errgroup.go
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/errgroup/errgroup.go b/errgroup/errgroup.go
index 1eab2fd..da2d40d 100644
--- a/errgroup/errgroup.go
+++ b/errgroup/errgroup.go
@@ -113,7 +113,7 @@
}
// SetLimit limits the number of active goroutines in this group to at most n.
-// A negative value indicates no limit.
+// A negative value indicates no limit. The default is unlimited.
//
// Any subsequent call to the Go method will block until it can add an active
// goroutine without exceeding the configured limit.
To view, visit change 409774. To unsubscribe, or for help writing mail filters, visit settings.
Jean de Klerk (Gerrit)
Attention is currently required from: Bryan Mills, Changkun Ou.
Patch set 1:Run-TryBot +1
Changkun Ou (Gerrit)
Attention is currently required from: Bryan Mills, Jean de Klerk.
1 comment:
File errgroup/errgroup.go:
Patch Set #1, Line 116:
defaultI don't know what "default" means here. Is default referring to `SetLimit(0)`?
I think we might want to clarify the difference between SetLimit is never called or called with a given number. But this clarification shouldn't be here.
Suppose we want to clarify SetLimit(0). In that case, it seems to me that the existing doc already covered all behavior: SetLimit(0) means "SetLimit limits the number of active goroutines in this group to at most 0."
To view, visit change 409774. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Attention is currently required from: Changkun Ou, Jean de Klerk.
Patch set 1:Code-Review +1
1 comment:
File errgroup/errgroup.go:
Patch Set #1, Line 116:
default
I don't know what "default" means here. Is default referring to `SetLimit(0)`? […]
From the commit message, I'm guessing that this refers to the behavior if `SetLimit` is not called.
In that case, perhaps we should update the comment on line 20 instead:
```
// A zero Group is valid, has no limit on the number of active goroutines,
// and does not cancel on error.
```
To view, visit change 409774. To unsubscribe, or for help writing mail filters, visit settings.
Jean de Klerk (Gerrit)
Attention is currently required from: Changkun Ou, Jean de Klerk.
Jean de Klerk uploaded patch set #2 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Jean de Klerk, TryBot-Result+1 by Gopher Robot
x/sync/errgroup: make note of the default errgroup goroutine limit (unlimited)
Change-Id: I1926e6faf821a7adb6c1365d18432abb0f856367
---
M errgroup/errgroup.go
1 file changed, 10 insertions(+), 1 deletion(-)
To view, visit change 409774. To unsubscribe, or for help writing mail filters, visit settings.
Jean de Klerk (Gerrit)
Attention is currently required from: Bryan Mills, Changkun Ou.
1 comment:
File errgroup/errgroup.go:
Patch Set #1, Line 116:
default
From the commit message, I'm guessing that this refers to the behavior if `SetLimit` is not called. […]
Done
To view, visit change 409774. To unsubscribe, or for help writing mail filters, visit settings.
Jean de Klerk (Gerrit)
Patchset:
errr, sorry, having trouble uploading. one moment...
To view, visit change 409774. To unsubscribe, or for help writing mail filters, visit settings.
Jean de Klerk (Gerrit)
Attention is currently required from: Bryan Mills, Changkun Ou.
Jean de Klerk uploaded patch set #4 to this change.
x/sync/errgroup: make note of the default errgroup goroutine limit (unlimited)
Change-Id: I1926e6faf821a7adb6c1365d18432abb0f856367
---
M errgroup/errgroup.go
1 file changed, 11 insertions(+), 1 deletion(-)
To view, visit change 409774. To unsubscribe, or for help writing mail filters, visit settings.
Jean de Klerk (Gerrit)
Attention is currently required from: Bryan Mills, Changkun Ou.
1 comment:
Patchset:
errr, sorry, having trouble uploading. one moment...
there we go - done properly now =)
To view, visit change 409774. To unsubscribe, or for help writing mail filters, visit settings.
Changkun Ou (Gerrit)
Attention is currently required from: Bryan Mills, Jean de Klerk.
Patch set 4:Run-TryBot +1Code-Review +1
Bryan Mills (Gerrit)
Attention is currently required from: Jean de Klerk.
Patch set 4:Run-TryBot +1Code-Review +2
Jean de Klerk (Gerrit)
Jean de Klerk submitted this change.
x/sync/errgroup: make note of the default errgroup goroutine limit (unlimited)
Change-Id: I1926e6faf821a7adb6c1365d18432abb0f856367
Reviewed-on: https://go-review.googlesource.com/c/sync/+/409774
Run-TryBot: Bryan Mills <bcm...@google.com>
Reviewed-by: Bryan Mills <bcm...@google.com>
Reviewed-by: Changkun Ou <ma...@changkun.de>
Run-TryBot: Changkun Ou <ma...@changkun.de>
TryBot-Result: Gopher Robot <go...@golang.org>
---
M errgroup/errgroup.go
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/errgroup/errgroup.go b/errgroup/errgroup.go
index 1eab2fd..4c0850a 100644
--- a/errgroup/errgroup.go
+++ b/errgroup/errgroup.go
@@ -17,7 +17,8 @@
// A Group is a collection of goroutines working on subtasks that are part of
// the same overall task.
//
-// A zero Group is valid and does not cancel on error.
+// A zero Group is valid, has no limit on the number of active goroutines,
+// and does not cancel on error.
type Group struct {
cancel func()
To view, visit change 409774. To unsubscribe, or for help writing mail filters, visit settings.