util: Refactor Backoff to return the next step rather than sleeping #71088

smarterclayton · 2018-11-15T19:43:15Z

Allows consumers to use Backoff as a generator rather than have to
call ExponentialBackoff. Extracted from #69890 at reviewer request.

Many other consumers want to cap exponential backoff and this now
allows it.

/kind cleanup

NONE

fedebongio · 2018-11-15T21:48:24Z

/cc @logicalhan

k8s-ci-robot · 2018-11-15T21:48:26Z

@fedebongio: GitHub didn't allow me to request PR reviews from the following users: logicalhan.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @logicalhan

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

smarterclayton · 2018-11-16T15:36:09Z

@sttts can you take a peek at this?

staging/src/k8s.io/apimachinery/pkg/util/wait/wait.go

liggitt · 2018-11-29T04:31:16Z

staging/src/k8s.io/apimachinery/pkg/util/wait/wait.go

+	}
+	b.Steps--
+	if b.Jitter > 0 {
+		b.Duration = Jitter(b.Duration, b.Jitter)


this compounds jitter into future iterations rather than compounding duration consistently and jittering each iteration without affecting future steps, which is a change

We compounded before

Nm, I see what you mean. Not sure it’s better or worse.

staging/src/k8s.io/apimachinery/pkg/util/wait/wait.go

liggitt · 2018-11-29T04:36:35Z

staging/src/k8s.io/apimachinery/pkg/util/wait/wait.go

+	if b.Cap > 0 && b.Duration > b.Cap {
+		b.Duration = b.Cap
+		b.Steps = 0
+		if b.Jitter > 0 {


shouldn't this be outside the if b.Cap > 0 && b.Duration > b.Cap block?

liggitt · 2018-11-29T04:36:59Z

staging/src/k8s.io/apimachinery/pkg/util/wait/wait.go

@@ -177,6 +177,32 @@ type Backoff struct {
 	Factor   float64       // Duration is multiplied by factor each iteration
 	Jitter   float64       // The amount of jitter applied each iteration
 	Steps    int           // Exit with error after this many steps
+	Cap      time.Duration // The maximum amount of time to wait if set


is Jitter allowed to exceed Cap?

In this, yes. We wouldn’t want to line up again on cap.

makes sense, just document that the cap is +/- jitter

liggitt · 2018-11-29T04:39:03Z

staging/src/k8s.io/apimachinery/pkg/util/wait/wait_test.go

+		{initial: &Backoff{Duration: time.Second, Steps: 0}, want: []time.Duration{time.Second, time.Second, time.Second}},
+		{initial: &Backoff{Duration: time.Second, Steps: 1}, want: []time.Duration{time.Second, time.Second, time.Second}},
+		{initial: &Backoff{Duration: time.Second, Factor: 1.0, Steps: 1}, want: []time.Duration{time.Second, time.Second, time.Second}},
+		{initial: &Backoff{Duration: time.Second, Factor: 2, Steps: 2}, want: []time.Duration{2 * time.Second, 4 * time.Second, 4 * time.Second}},


I expected the first duration to be 1 second as before this PR

staging/src/k8s.io/apimachinery/pkg/util/wait/wait_test.go

smarterclayton · 2018-12-01T21:31:26Z

Fixes made. It's easier to read now.

smarterclayton · 2018-12-01T23:09:12Z

/retest

k8s-ci-robot · 2018-12-02T05:12:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smarterclayton

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~staging/src/k8s.io/apimachinery/pkg/OWNERS~~ [smarterclayton]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

smarterclayton · 2018-12-02T06:36:39Z

/retest

…

On Sun, Dec 2, 2018 at 1:10 AM Kubernetes Prow Robot < ***@***.***> wrote: @smarterclayton <https://github.com/smarterclayton>: The following test *failed*, say /retest to rerun them all: Test name Commit Details Rerun command pull-kubernetes-e2e-kops-aws a2a6637 <a2a6637> link <https://gubernator.k8s.io/build/kubernetes-jenkins/pr-logs/pull/71088/pull-kubernetes-e2e-kops-aws/115568/> /test pull-kubernetes-e2e-kops-aws Full PR test history <https://gubernator.k8s.io/pr/71088>. Your PR dashboard <https://gubernator.k8s.io/pr/smarterclayton>. Please help us cut down on flakes by linking to <https://git.k8s.io/community/contributors/devel/flaky-tests.md#filing-issues-for-flaky-tests> an open issue <https://github.com/kubernetes/kubernetes/issues?q=is:issue+is:open> when you hit one in your PR. Instructions for interacting with me using PR comments are available here <https://git.k8s.io/community/contributors/guide/pull-requests.md>. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra <https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:> repository. I understand the commands that are listed here <https://go.k8s.io/bot-commands>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#71088 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p5Ethx3iNrwK7L-8pWQGcvkbJxDBks5u026-gaJpZM4YiGHw> .

staging/src/k8s.io/apimachinery/pkg/util/wait/wait.go

Allows consumers to use Backoff as a generator rather than have to call ExponentialBackoff

smarterclayton · 2018-12-04T04:56:01Z

/retest

liggitt · 2018-12-04T04:59:12Z

/lgtm

smarterclayton · 2018-12-04T07:19:36Z

/retest

…

On Tue, Dec 4, 2018 at 2:15 AM Kubernetes Prow Robot < ***@***.***> wrote: @smarterclayton <https://github.com/smarterclayton>: The following test *failed*, say /retest to rerun them all: Test name Commit Details Rerun command pull-kubernetes-e2e-gce-100-performance e62d389 <e62d389> link <https://gubernator.k8s.io/build/kubernetes-jenkins/pr-logs/pull/71088/pull-kubernetes-e2e-gce-100-performance/28662/> /test pull-kubernetes-e2e-gce-100-performance Full PR test history <https://gubernator.k8s.io/pr/71088>. Your PR dashboard <https://gubernator.k8s.io/pr/smarterclayton>. Please help us cut down on flakes by linking to <https://git.k8s.io/community/contributors/devel/flaky-tests.md#filing-issues-for-flaky-tests> an open issue <https://github.com/kubernetes/kubernetes/issues?q=is:issue+is:open> when you hit one in your PR. Instructions for interacting with me using PR comments are available here <https://git.k8s.io/community/contributors/guide/pull-requests.md>. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra <https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:> repository. I understand the commands that are listed here <https://go.k8s.io/bot-commands>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#71088 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p6FzZF8SwiHSOF4UAlfKhM0UZ3Ngks5u1iEYgaJpZM4YiGHw> .

k8s-ci-robot requested review from gmarek and sttts November 15, 2018 19:43

k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 15, 2018

This was referenced Nov 15, 2018

Make bootstrap client cert loading part of rotation #69890

Merged

Use a timeout loop rather than a retry backoff in bootstrap #71089

Closed

liggitt reviewed Nov 29, 2018

View reviewed changes

staging/src/k8s.io/apimachinery/pkg/util/wait/wait.go Show resolved Hide resolved

liggitt reviewed Nov 29, 2018

View reviewed changes

staging/src/k8s.io/apimachinery/pkg/util/wait/wait.go Show resolved Hide resolved

liggitt reviewed Nov 29, 2018

View reviewed changes

staging/src/k8s.io/apimachinery/pkg/util/wait/wait_test.go Show resolved Hide resolved

smarterclayton force-pushed the wait_cap branch from 91a1e03 to a824e30 Compare December 1, 2018 21:31

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 1, 2018

smarterclayton force-pushed the wait_cap branch from a824e30 to a2a6637 Compare December 2, 2018 05:11

liggitt reviewed Dec 3, 2018

View reviewed changes

staging/src/k8s.io/apimachinery/pkg/util/wait/wait.go Show resolved Hide resolved

util: Refactor Backoff to return the next step rather than sleeping

e62d389

Allows consumers to use Backoff as a generator rather than have to call ExponentialBackoff

smarterclayton force-pushed the wait_cap branch from a2a6637 to e62d389 Compare December 3, 2018 21:43

k8s-ci-robot assigned liggitt Dec 4, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 4, 2018

k8s-ci-robot merged commit 3a83f29 into kubernetes:master Dec 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

util: Refactor Backoff to return the next step rather than sleeping #71088

util: Refactor Backoff to return the next step rather than sleeping #71088

smarterclayton commented Nov 15, 2018

fedebongio commented Nov 15, 2018

k8s-ci-robot commented Nov 15, 2018

smarterclayton commented Nov 16, 2018

liggitt Nov 29, 2018 •

edited

smarterclayton Nov 29, 2018

smarterclayton Nov 29, 2018

liggitt Nov 29, 2018

liggitt Nov 29, 2018

smarterclayton Nov 29, 2018

liggitt Nov 29, 2018

liggitt Nov 29, 2018

smarterclayton commented Dec 1, 2018

smarterclayton commented Dec 1, 2018

k8s-ci-robot commented Dec 2, 2018

smarterclayton commented Dec 2, 2018 via email

smarterclayton commented Dec 4, 2018

liggitt commented Dec 4, 2018

smarterclayton commented Dec 4, 2018 via email

util: Refactor Backoff to return the next step rather than sleeping #71088

util: Refactor Backoff to return the next step rather than sleeping #71088

Conversation

smarterclayton commented Nov 15, 2018

fedebongio commented Nov 15, 2018

k8s-ci-robot commented Nov 15, 2018

smarterclayton commented Nov 16, 2018

liggitt Nov 29, 2018 • edited

Choose a reason for hiding this comment

smarterclayton Nov 29, 2018

Choose a reason for hiding this comment

smarterclayton Nov 29, 2018

Choose a reason for hiding this comment

liggitt Nov 29, 2018

Choose a reason for hiding this comment

liggitt Nov 29, 2018

Choose a reason for hiding this comment

smarterclayton Nov 29, 2018

Choose a reason for hiding this comment

liggitt Nov 29, 2018

Choose a reason for hiding this comment

liggitt Nov 29, 2018

Choose a reason for hiding this comment

smarterclayton commented Dec 1, 2018

smarterclayton commented Dec 1, 2018

k8s-ci-robot commented Dec 2, 2018

smarterclayton commented Dec 2, 2018 via email

smarterclayton commented Dec 4, 2018

liggitt commented Dec 4, 2018

smarterclayton commented Dec 4, 2018 via email

liggitt Nov 29, 2018 •

edited