Bankim Bhavsar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17839 )

Change subject: wip [rpc] KUDU-75: refresh DNS entries if proxies hit a network 
error
......................................................................


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/17839/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17839/3//COMMIT_MSG@12
PS3, Line 12: - Also unclear whether we should force all Proxies to abide by 
this new
            :   implementation, or have a separate class or mode that doesn't
            :   re-resolve on failure
I like the idea of a mode using a config that turns on resolving DNS in case of 
error v/s turning it on by default for all proxies.


http://gerrit.cloudera.org:8080/#/c/17839/3/src/kudu/rpc/connection_id.h
File src/kudu/rpc/connection_id.h:

http://gerrit.cloudera.org:8080/#/c/17839/3/src/kudu/rpc/connection_id.h@49
PS3, Line 49: remote
std::move() ?


http://gerrit.cloudera.org:8080/#/c/17839/3/src/kudu/rpc/proxy.cc
File src/kudu/rpc/proxy.cc:

http://gerrit.cloudera.org:8080/#/c/17839/3/src/kudu/rpc/proxy.cc@106
PS3, Line 106: if (PREDICT_TRUE(s.ok() && !addrs.empty())) {
             :     addr = addrs[0];
             :     DCHECK(addr.is_initialized());
             :     addr.set_port(hp_.port());
             :   }
Okay to proceed in case of failure?


http://gerrit.cloudera.org:8080/#/c/17839/3/src/kudu/rpc/proxy.cc@185
PS3, Line 185: successfull
successfully


http://gerrit.cloudera.org:8080/#/c/17839/3/src/kudu/rpc/proxy.cc@201
PS3, Line 201:     // TODO(awong): we should be more specific here -- consider 
having the RPC
             :     // layer set a flag in the controller that warrants a retry.
             :     if (PREDICT_FALSE(controller->status().IsNetworkError())) {
             :       RefreshDnsAndEnqueueRequest(method, req, response, 
controller, callback);
             :       return;
             :     }
             :     // For any other status, OK or otherwise, just run the 
callback.
             :     callback();
What happens if the DNS resolution keeps failing? Will the request be 
indefinitely re-enqueued? Basically when do we give up.



--
To view, visit http://gerrit.cloudera.org:8080/17839
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I777d169bd3a461294e5721f05071b726ced70f7e
Gerrit-Change-Number: 17839
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Bankim Bhavsar <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 14 Sep 2021 22:42:13 +0000
Gerrit-HasComments: Yes

Reply via email to