-
Notifications
You must be signed in to change notification settings - Fork 40.1k
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
Change defaults, retry on errors and report stats for serve_hostnames #5881
Conversation
Here is a representative run. Worryingly one of the nodes never fields any requests.
|
6aaa5f7
to
05bd6dd
Compare
}, | ||
}) | ||
}) | ||
glog.Infof("Service create %s/server-hostnames took %v", ns, time.Since(t)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be outside the loop? Otherwise it will be printed on every failed attempt as well as successful attempts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to print the failed attempts. In the next iteration I plan to add a --verbose
flag to make the default output quieter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. You're trying to measure the latency of the create call here. When you add the verbose flag, you might also consider making the output reflect whether the call was successful or not.
Lots of nits, overall looks good. |
05bd6dd
to
32909a3
Compare
Thank you! PTAL. |
Oh, I also plan to add an option to use an external load balancer. That way we can compare proxy vs. ELB performance. |
LGTM. Will merge on green. |
Change defaults, retry on errors and report stats for serve_hostnames
No description provided.