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

Network requests sleep for 10 nanoseconds. Is that intentional? #56

Open
seh opened this issue Dec 5, 2016 · 2 comments
Open

Network requests sleep for 10 nanoseconds. Is that intentional? #56

seh opened this issue Dec 5, 2016 · 2 comments

Comments

@seh
Copy link
Contributor

seh commented Dec 5, 2016

Running staticcheck over fargo reveals one potential problem: in file rpc.go's netReq function, when a request fails with a temporary error, we sleep for ten nanoseconds. Note that the magnitude is specified without any unit, allowing its conversion to a time.Duration with the default unit of nanoseconds.

% staticcheck                                    
[...]/github.com/hudl/fargo/rpc.go:111:15: sleeping for 10 nanoseconds is probably a bug. Be explicit if it isn't: time.Sleep(10 * time.Nanosecond) (SA1004)

Error SA1004 has the following description:

Suspiciously small untyped constant in time.Sleep

What was the intended duration there? @ryansb added that statement in commit 4d053d5. Ryan, do you remember?

@ryansb
Copy link
Contributor

ryansb commented Dec 5, 2016

Oh, whoops. I think I intended 10 seconds, but that may be too long. A smarter option would probably be to back off, trying first 3 seconds later, then 9, then 30 or so.

@seh
Copy link
Contributor Author

seh commented Dec 5, 2016

I agree. There are various backoff libraries out there, if you'd consider introducing a dependency for this. Otherwise, I can implement a simple enough adjustment, so long as we limit how configurable it is. These things tend to devolve into a sampler pack of patterns—strategies, policies, and such.

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

No branches or pull requests

2 participants