-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Enhancements to loadbalancer #90
Conversation
Having Publishers return a set of endpoints directly (synchronously) allows us to eliminate a lot of boilerplate related to pub/sub semantics, as well as the cache type. Thanks, @rogpeppe!
@rogpeppe Thanks for your suggestions at GopherCon. I've changed the package and API and it feels a lot smoother. Have a few moments for a quick code review? |
} | ||
``` | ||
|
||
It's also possible to wrap a load balancer with a retry strategy, so that it | ||
can used as an endpoint directly. This may make load balancers more convenient |
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.
s/can used/can be used/
If the publisher has been stopped, return an appropriate error message from Endpoints. Thanks, @ChrisHines!
// Get a new endpoint from the load balancer. | ||
endpoint, err := lb.Endpoint() | ||
if err != nil { | ||
panic(err) |
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.
shouldn't just loop until we have a valid endpoint after waiting few seconds ?
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.
If your DNS name has no instances, I believe it's much better to crash your program on startup, than to successfully start up, but leave you in a state where you can't complete any requests.
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've been thinking about this and may revisit this decision...
Enhancements to loadbalancer
Enhancements to loadbalancer
Addresses #73 and #83.