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

clientv3: add pinned() method to 'balancer' #8659

Merged
merged 1 commit into from
Oct 6, 2017
Merged

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Oct 6, 2017

No description provided.

@gyuho gyuho requested a review from xiang90 October 6, 2017 20:32
@gyuho
Copy link
Contributor Author

gyuho commented Oct 6, 2017

Partially address #8660.

@@ -42,6 +42,7 @@ type balancer interface {

endpoint(host string) string
endpoints() []string
pinned() string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doc string?

err := f(rpcCtx)
if err == nil {
return nil
}
if logger.V(4) {
logger.Infof("clientv3/retry: retry for error %v", err)
logger.Infof("clientv3/retry: retry for error %v (%s)", err, pinned)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

retry on pined endpoint (%s) due to error %v

err := f(rpcCtx)
if err == nil {
return nil
}
if logger.V(4) {
logger.Infof("clientv3/auth-retry: retry for error %v", err)
logger.Infof("clientv3/auth-retry: retry for error %v (%s)", err, pinned)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

retry on pined endpoint (%s) due to error %v

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xiang90 That endpoint is errored, so it's unpinned by this line. So it's possible that we retry on different endpoint?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or let's say ` error %s on pined endpoint (%s))

basically only (%s) is not descriptive.

@xiang90
Copy link
Contributor

xiang90 commented Oct 6, 2017

this is just for debugging, right?

@gyuho
Copy link
Contributor Author

gyuho commented Oct 6, 2017

@xiang90 Yes, this is for balancer debugging. But in the following PR, I will add gray-listing the timed out endpoint.

Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
@gyuho
Copy link
Contributor Author

gyuho commented Oct 6, 2017

@xiang90 PTAL. Fixed. Thanks.

@xiang90
Copy link
Contributor

xiang90 commented Oct 6, 2017

lgtm

@gyuho gyuho merged commit e8e3467 into etcd-io:master Oct 6, 2017
@gyuho gyuho deleted the pinned branch October 6, 2017 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants