Skip to content

Fix duration is zero #1334

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

Closed
wants to merge 1 commit into from
Closed

Conversation

daixiang0
Copy link
Contributor

@daixiang0 daixiang0 commented Apr 17, 2019

When MaxBackoff not set, after one loop, duration is always bigger than
it, so duration set as zero, then following panic occur:

panic: invalid argument to Int63n

goroutine 169 [running]:
math/rand.(*Rand).Int63n(0xc0000cc150, 0x0, 0xc000719b58)
	/usr/lib/golang/src/math/rand/rand.go:111 +0x11e
math/rand.Int63n(...)
	/usr/lib/golang/src/math/rand/rand.go:319

Signed-off-by: Xiang Dai [email protected]

When MaxBackoff not set, after one loop, duration is always bigger than
it, so duration set as zero, then following panic occur:

panic: invalid argument to Int63n

goroutine 169 [running]:
math/rand.(*Rand).Int63n(0xc0000cc150, 0x0, 0xc000719b58)
	/usr/lib/golang/src/math/rand/rand.go:111 +0x11e
math/rand.Int63n(...)
	/usr/lib/golang/src/math/rand/rand.go:319

Signed-off-by: Xiang Dai <[email protected]>
@bboreham
Copy link
Contributor

How did you get into this situation?

Backoff is meant to be used with non-zero parameters, so that it waits an increasing amount of time. I can't see what a zero-time delay would be good for.

@daixiang0
Copy link
Contributor Author

The key is to solve the panic, since i want even without set MaxBackoff, backoff could work without panic.

@bboreham
Copy link
Contributor

I think it would be simpler to just put a check if b.duration == 0 {return} near the top of Wait(), maybe with a comment that this covers the case where the user has configured a zero min or max time.

It's probably also unexpected that a zero minimum time with non-zero max time will result in zero waits.
See also #792

@daixiang0
Copy link
Contributor Author

Do you mean min/max time is required field so no need to change here but check at caller?

@bboreham
Copy link
Contributor

bboreham commented Sep 3, 2019

This has been overtaken by #1599

@bboreham bboreham closed this Sep 3, 2019
@daixiang0 daixiang0 deleted the backoff branch December 19, 2019 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants