{"action":"created","comment":{"url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/comments/33803593","id":33803593,"diff_hunk":"@@ -91,219 +95,230 @@ type Client struct {\n //\n // The Client.Ready channel is closed after the client has connected\n // and completed one successful heartbeat. The Closed channel is\n-// closed if the client fails to connect or if the client's Close()\n-// method is invoked.\n-func NewClient(addr net.Addr, opts *retry.Options, context *Context) *Client {\n+// closed if the client's Close() method is invoked.\n+func NewClient(addr net.Addr, context *Context) (*Client, error) {\n \tclientMu.Lock()\n+\tdefer clientMu.Unlock()\n+\n+\tnetwork := addr.Network()\n+\taddress := addr.String()\n+\n+\tkey := network + address\n+\n \tif !context.DisableCache {\n-\t\tif c, ok := clients[addr.String()]; ok {\n-\t\t\tclientMu.Unlock()\n-\t\t\treturn c\n+\t\tif c, ok := clients[key]; ok {\n+\t\t\treturn c, nil\n \t\t}\n \t}\n+\n+\ttlsConfig, err := context.GetClientTLSConfig()\n+\tif err != nil {\n+\t\treturn nil, err\n+\t}\n+\n \tc := &Client{\n-\t\taddr:         addr,\n-\t\tReady:        make(chan struct{}),\n-\t\tClosed:       make(chan struct{}),\n-\t\tclock:        context.localClock,\n-\t\tremoteClocks: context.RemoteClocks,\n-\t\tcached:       !context.DisableCache,\n+\t\tClosed:        make(chan struct{}),\n+\t\tkey:           key,\n+\t\tnetwork:       network,\n+\t\tremoteAddress: address,\n+\t\ttlsConfig:     tlsConfig,\n+\t\tclock:         context.localClock,\n+\t\tremoteClocks:  context.RemoteClocks,\n \t}\n-\tif !context.DisableCache {\n-\t\tclients[c.Addr().String()] = c\n+\n+\tc.healthy.Store(make(chan struct{}))\n+\n+\tif err = c.connect(); err != nil {\n+\t\treturn nil, err\n \t}\n-\tclientMu.Unlock()\n+\n+\tclients[key] = c\n+\n+\tretryOpts := clientRetryOptions\n+\tretryOpts.Stopper = context.Stopper\n \n \tcontext.Stopper.RunWorker(func() {\n-\t\tif err := c.connect(opts, context); err != nil {\n-\t\t\tlog.Errorf(\"client %s failed to connect: %v\", c.addr, err)\n-\t\t} else {\n-\t\t\t// Signal client is ready by closing Ready channel.\n-\t\t\tlog.Infof(\"client %s connected\", c.addr)\n-\t\t\tclose(c.Ready)\n+\t\t<-context.Stopper.ShouldStop()\n \n-\t\t\t// Launch periodic heartbeat. This blocks until the client is to be closed.\n-\t\t\tc.runHeartbeat(context.Stopper)\n-\t\t}\n \t\tc.Close()\n \t})\n-\treturn c\n+\n+\tgo func() {\n+\t\tc.runHeartbeat(retryOpts)\n+\t\tc.rpcClient.Close()\n+\t}()\n+\n+\treturn c, nil\n }\n \n-// connect dials the connection in a backoff/retry loop.\n-func (c *Client) connect(opts *retry.Options, context *Context) error {\n-\t// Attempt to dial connection.\n-\tretryOpts := clientRetryOptions\n-\tif opts != nil {\n-\t\tretryOpts = *opts\n+// Go wraps net/rpc.Client.Go and provides additional cockroach-specific error\n+// handling.\n+func (c *Client) Go(serviceMethod string, args interface{}, reply interface{}, done chan *rpc.Call) *rpc.Call {\n+\tcall := c.rpcClient.Go(serviceMethod, args, reply, nil)\n+\tnewCall := &rpc.Call{}\n+\tif done == nil {\n+\t\tnewCall.Done = make(chan *rpc.Call, 1)\n+\t} else {\n+\t\tnewCall.Done = done\n \t}\n-\tretryOpts.Stopper = context.Stopper\n \n-\tfor r := retry.Start(retryOpts); r.Next(); {\n-\t\ttlsConfig, err := context.GetClientTLSConfig()\n-\t\tif err != nil {\n-\t\t\t// Problem loading the TLS config. Retrying will not help.\n-\t\t\treturn err\n-\t\t}\n+\tgo func() {\n+\t\tfinishedCall := <-call.Done\n+\t\tnewCall.ServiceMethod = finishedCall.ServiceMethod\n+\t\tnewCall.Args = finishedCall.Args\n+\t\tnewCall.Reply = finishedCall.Reply\n+\t\tnewCall.Error = finishedCall.Error\n \n-\t\tconn, err := tlsDialHTTP(c.addr.Network(), c.addr.String(), tlsConfig)\n-\t\tif err != nil {\n-\t\t\t// Retry if the error is temporary, otherwise fail fast.\n-\t\t\tif t, ok := err.(net.Error); ok && t.Temporary() {\n-\t\t\t\tif log.V(1) {\n-\t\t\t\t\tlog.Warning(err)\n+\t\tif newCall.Error != nil {","path":"rpc/client.go","position":194,"original_position":177,"commit_id":"9e83cef0575e7ca4b05cc51e1bc3e99fb31081f8","original_commit_id":"7bb676085729bfed9931b9b5cf6836e79dbf69c6","user":{"login":"bdarnell","id":160562,"avatar_url":"https://avatars.githubusercontent.com/u/160562?v=3","gravatar_id":"","url":"https://api.github.com/users/bdarnell","html_url":"https://github.com/bdarnell","followers_url":"https://api.github.com/users/bdarnell/followers","following_url":"https://api.github.com/users/bdarnell/following{/other_user}","gists_url":"https://api.github.com/users/bdarnell/gists{/gist_id}","starred_url":"https://api.github.com/users/bdarnell/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bdarnell/subscriptions","organizations_url":"https://api.github.com/users/bdarnell/orgs","repos_url":"https://api.github.com/users/bdarnell/repos","events_url":"https://api.github.com/users/bdarnell/events{/privacy}","received_events_url":"https://api.github.com/users/bdarnell/received_events","type":"User","site_admin":false},"body":"This feels like the wrong place for this error wrapping. I'd rather have the code in send.go that checks for Retryable be aware of io.ErrUnexpectedEOF as a special case.","created_at":"2015-07-02T17:47:15Z","updated_at":"2015-07-02T17:47:15Z","html_url":"https://github.com/cockroachdb/cockroach/pull/1523#discussion_r33803593","pull_request_url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/1523","_links":{"self":{"href":"https://api.github.com/repos/cockroachdb/cockroach/pulls/comments/33803593"},"html":{"href":"https://github.com/cockroachdb/cockroach/pull/1523#discussion_r33803593"},"pull_request":{"href":"https://api.github.com/repos/cockroachdb/cockroach/pulls/1523"}}},"pull_request":{"url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/1523","id":38691437,"html_url":"https://github.com/cockroachdb/cockroach/pull/1523","diff_url":"https://github.com/cockroachdb/cockroach/pull/1523.diff","patch_url":"https://github.com/cockroachdb/cockroach/pull/1523.patch","issue_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/1523","number":1523,"state":"open","locked":false,"title":"rpc client tweaks","user":{"login":"tamird","id":1535036,"avatar_url":"https://avatars.githubusercontent.com/u/1535036?v=3","gravatar_id":"","url":"https://api.github.com/users/tamird","html_url":"https://github.com/tamird","followers_url":"https://api.github.com/users/tamird/followers","following_url":"https://api.github.com/users/tamird/following{/other_user}","gists_url":"https://api.github.com/users/tamird/gists{/gist_id}","starred_url":"https://api.github.com/users/tamird/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tamird/subscriptions","organizations_url":"https://api.github.com/users/tamird/orgs","repos_url":"https://api.github.com/users/tamird/repos","events_url":"https://api.github.com/users/tamird/events{/privacy}","received_events_url":"https://api.github.com/users/tamird/received_events","type":"User","site_admin":false},"body":"Addresses #1521.\r\n\r\nLast two commits need tests, but I thought I'd get some feedback first. @MBerhault and @petermattis in particular, PTAL.\r\n\r\nSomewhat orthogonally, this adds a 3-second TLS timeout.","created_at":"2015-06-26T21:32:09Z","updated_at":"2015-07-02T17:47:15Z","closed_at":null,"merged_at":null,"merge_commit_sha":"a9bb33cbe98e512fafcb26975f383dd89c5643d5","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/1523/commits","review_comments_url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/1523/comments","review_comment_url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/comments{/number}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/1523/comments","statuses_url":"https://api.github.com/repos/cockroachdb/cockroach/statuses/9e83cef0575e7ca4b05cc51e1bc3e99fb31081f8","head":{"label":"tamird:rpc-client-work","ref":"rpc-client-work","sha":"9e83cef0575e7ca4b05cc51e1bc3e99fb31081f8","user":{"login":"tamird","id":1535036,"avatar_url":"https://avatars.githubusercontent.com/u/1535036?v=3","gravatar_id":"","url":"https://api.github.com/users/tamird","html_url":"https://github.com/tamird","followers_url":"https://api.github.com/users/tamird/followers","following_url":"https://api.github.com/users/tamird/following{/other_user}","gists_url":"https://api.github.com/users/tamird/gists{/gist_id}","starred_url":"https://api.github.com/users/tamird/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tamird/subscriptions","organizations_url":"https://api.github.com/users/tamird/orgs","repos_url":"https://api.github.com/users/tamird/repos","events_url":"https://api.github.com/users/tamird/events{/privacy}","received_events_url":"https://api.github.com/users/tamird/received_events","type":"User","site_admin":false},"repo":{"id":35114205,"name":"cockroach","full_name":"tamird/cockroach","owner":{"login":"tamird","id":1535036,"avatar_url":"https://avatars.githubusercontent.com/u/1535036?v=3","gravatar_id":"","url":"https://api.github.com/users/tamird","html_url":"https://github.com/tamird","followers_url":"https://api.github.com/users/tamird/followers","following_url":"https://api.github.com/users/tamird/following{/other_user}","gists_url":"https://api.github.com/users/tamird/gists{/gist_id}","starred_url":"https://api.github.com/users/tamird/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tamird/subscriptions","organizations_url":"https://api.github.com/users/tamird/orgs","repos_url":"https://api.github.com/users/tamird/repos","events_url":"https://api.github.com/users/tamird/events{/privacy}","received_events_url":"https://api.github.com/users/tamird/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/tamird/cockroach","description":"A Scalable, Geo-Replicated, Transactional Datastore","fork":true,"url":"https://api.github.com/repos/tamird/cockroach","forks_url":"https://api.github.com/repos/tamird/cockroach/forks","keys_url":"https://api.github.com/repos/tamird/cockroach/keys{/key_id}","collaborators_url":"https://api.github.com/repos/tamird/cockroach/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/tamird/cockroach/teams","hooks_url":"https://api.github.com/repos/tamird/cockroach/hooks","issue_events_url":"https://api.github.com/repos/tamird/cockroach/issues/events{/number}","events_url":"https://api.github.com/repos/tamird/cockroach/events","assignees_url":"https://api.github.com/repos/tamird/cockroach/assignees{/user}","branches_url":"https://api.github.com/repos/tamird/cockroach/branches{/branch}","tags_url":"https://api.github.com/repos/tamird/cockroach/tags","blobs_url":"https://api.github.com/repos/tamird/cockroach/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/tamird/cockroach/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/tamird/cockroach/git/refs{/sha}","trees_url":"https://api.github.com/repos/tamird/cockroach/git/trees{/sha}","statuses_url":"https://api.github.com/repos/tamird/cockroach/statuses/{sha}","languages_url":"https://api.github.com/repos/tamird/cockroach/languages","stargazers_url":"https://api.github.com/repos/tamird/cockroach/stargazers","contributors_url":"https://api.github.com/repos/tamird/cockroach/contributors","subscribers_url":"https://api.github.com/repos/tamird/cockroach/subscribers","subscription_url":"https://api.github.com/repos/tamird/cockroach/subscription","commits_url":"https://api.github.com/repos/tamird/cockroach/commits{/sha}","git_commits_url":"https://api.github.com/repos/tamird/cockroach/git/commits{/sha}","comments_url":"https://api.github.com/repos/tamird/cockroach/comments{/number}","issue_comment_url":"https://api.github.com/repos/tamird/cockroach/issues/comments{/number}","contents_url":"https://api.github.com/repos/tamird/cockroach/contents/{+path}","compare_url":"https://api.github.com/repos/tamird/cockroach/compare/{base}...{head}","merges_url":"https://api.github.com/repos/tamird/cockroach/merges","archive_url":"https://api.github.com/repos/tamird/cockroach/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/tamird/cockroach/downloads","issues_url":"https://api.github.com/repos/tamird/cockroach/issues{/number}","pulls_url":"https://api.github.com/repos/tamird/cockroach/pulls{/number}","milestones_url":"https://api.github.com/repos/tamird/cockroach/milestones{/number}","notifications_url":"https://api.github.com/repos/tamird/cockroach/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/tamird/cockroach/labels{/name}","releases_url":"https://api.github.com/repos/tamird/cockroach/releases{/id}","created_at":"2015-05-05T17:34:04Z","updated_at":"2015-05-06T22:06:29Z","pushed_at":"2015-07-02T16:48:08Z","git_url":"git://github.com/tamird/cockroach.git","ssh_url":"git@github.com:tamird/cockroach.git","clone_url":"https://github.com/tamird/cockroach.git","svn_url":"https://github.com/tamird/cockroach","homepage":null,"size":23586,"stargazers_count":0,"watchers_count":0,"language":"Go","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"cockroachdb:master","ref":"master","sha":"b64b512f83bd368e9bf6018820ec38bea92c62ac","user":{"login":"cockroachdb","id":6748139,"avatar_url":"https://avatars.githubusercontent.com/u/6748139?v=3","gravatar_id":"","url":"https://api.github.com/users/cockroachdb","html_url":"https://github.com/cockroachdb","followers_url":"https://api.github.com/users/cockroachdb/followers","following_url":"https://api.github.com/users/cockroachdb/following{/other_user}","gists_url":"https://api.github.com/users/cockroachdb/gists{/gist_id}","starred_url":"https://api.github.com/users/cockroachdb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cockroachdb/subscriptions","organizations_url":"https://api.github.com/users/cockroachdb/orgs","repos_url":"https://api.github.com/users/cockroachdb/repos","events_url":"https://api.github.com/users/cockroachdb/events{/privacy}","received_events_url":"https://api.github.com/users/cockroachdb/received_events","type":"Organization","site_admin":false},"repo":{"id":16563587,"name":"cockroach","full_name":"cockroachdb/cockroach","owner":{"login":"cockroachdb","id":6748139,"avatar_url":"https://avatars.githubusercontent.com/u/6748139?v=3","gravatar_id":"","url":"https://api.github.com/users/cockroachdb","html_url":"https://github.com/cockroachdb","followers_url":"https://api.github.com/users/cockroachdb/followers","following_url":"https://api.github.com/users/cockroachdb/following{/other_user}","gists_url":"https://api.github.com/users/cockroachdb/gists{/gist_id}","starred_url":"https://api.github.com/users/cockroachdb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cockroachdb/subscriptions","organizations_url":"https://api.github.com/users/cockroachdb/orgs","repos_url":"https://api.github.com/users/cockroachdb/repos","events_url":"https://api.github.com/users/cockroachdb/events{/privacy}","received_events_url":"https://api.github.com/users/cockroachdb/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/cockroachdb/cockroach","description":"A Scalable, Geo-Replicated, Transactional Datastore","fork":false,"url":"https://api.github.com/repos/cockroachdb/cockroach","forks_url":"https://api.github.com/repos/cockroachdb/cockroach/forks","keys_url":"https://api.github.com/repos/cockroachdb/cockroach/keys{/key_id}","collaborators_url":"https://api.github.com/repos/cockroachdb/cockroach/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/cockroachdb/cockroach/teams","hooks_url":"https://api.github.com/repos/cockroachdb/cockroach/hooks","issue_events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/events{/number}","events_url":"https://api.github.com/repos/cockroachdb/cockroach/events","assignees_url":"https://api.github.com/repos/cockroachdb/cockroach/assignees{/user}","branches_url":"https://api.github.com/repos/cockroachdb/cockroach/branches{/branch}","tags_url":"https://api.github.com/repos/cockroachdb/cockroach/tags","blobs_url":"https://api.github.com/repos/cockroachdb/cockroach/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/cockroachdb/cockroach/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/cockroachdb/cockroach/git/refs{/sha}","trees_url":"https://api.github.com/repos/cockroachdb/cockroach/git/trees{/sha}","statuses_url":"https://api.github.com/repos/cockroachdb/cockroach/statuses/{sha}","languages_url":"https://api.github.com/repos/cockroachdb/cockroach/languages","stargazers_url":"https://api.github.com/repos/cockroachdb/cockroach/stargazers","contributors_url":"https://api.github.com/repos/cockroachdb/cockroach/contributors","subscribers_url":"https://api.github.com/repos/cockroachdb/cockroach/subscribers","subscription_url":"https://api.github.com/repos/cockroachdb/cockroach/subscription","commits_url":"https://api.github.com/repos/cockroachdb/cockroach/commits{/sha}","git_commits_url":"https://api.github.com/repos/cockroachdb/cockroach/git/commits{/sha}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/comments{/number}","issue_comment_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/comments{/number}","contents_url":"https://api.github.com/repos/cockroachdb/cockroach/contents/{+path}","compare_url":"https://api.github.com/repos/cockroachdb/cockroach/compare/{base}...{head}","merges_url":"https://api.github.com/repos/cockroachdb/cockroach/merges","archive_url":"https://api.github.com/repos/cockroachdb/cockroach/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/cockroachdb/cockroach/downloads","issues_url":"https://api.github.com/repos/cockroachdb/cockroach/issues{/number}","pulls_url":"https://api.github.com/repos/cockroachdb/cockroach/pulls{/number}","milestones_url":"https://api.github.com/repos/cockroachdb/cockroach/milestones{/number}","notifications_url":"https://api.github.com/repos/cockroachdb/cockroach/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/labels{/name}","releases_url":"https://api.github.com/repos/cockroachdb/cockroach/releases{/id}","created_at":"2014-02-06T00:18:47Z","updated_at":"2015-07-02T16:05:12Z","pushed_at":"2015-07-02T16:52:30Z","git_url":"git://github.com/cockroachdb/cockroach.git","ssh_url":"git@github.com:cockroachdb/cockroach.git","clone_url":"https://github.com/cockroachdb/cockroach.git","svn_url":"https://github.com/cockroachdb/cockroach","homepage":null,"size":42505,"stargazers_count":4824,"watchers_count":4824,"language":"Go","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":355,"mirror_url":null,"open_issues_count":110,"forks":355,"open_issues":110,"watchers":4824,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/cockroachdb/cockroach/pulls/1523"},"html":{"href":"https://github.com/cockroachdb/cockroach/pull/1523"},"issue":{"href":"https://api.github.com/repos/cockroachdb/cockroach/issues/1523"},"comments":{"href":"https://api.github.com/repos/cockroachdb/cockroach/issues/1523/comments"},"review_comments":{"href":"https://api.github.com/repos/cockroachdb/cockroach/pulls/1523/comments"},"review_comment":{"href":"https://api.github.com/repos/cockroachdb/cockroach/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/cockroachdb/cockroach/pulls/1523/commits"},"statuses":{"href":"https://api.github.com/repos/cockroachdb/cockroach/statuses/9e83cef0575e7ca4b05cc51e1bc3e99fb31081f8"}}}}