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

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


Patch Set 7:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/17839/6/src/kudu/rpc/proxy-test.cc@77
PS6, Line 77: TestProxyReturnsOnNonTransientError
> nit: how long does it take to run this scenario?  Should it be run only whe
Done


http://gerrit.cloudera.org:8080/#/c/17839/6/src/kudu/rpc/proxy-test.cc@101
PS6, Line 101: _addr));
> Just to make sure my understanding is correct: that's the port part which i
I renamed/reworded this so it's hopefully clearer. The host is fake since there 
is no host called "fakehost", but the port itself is real.


http://gerrit.cloudera.org:8080/#/c/17839/6/src/kudu/rpc/proxy-test.cc@102
PS6, Line 102:
> Could you please extend the comment explaining that the crux here is supply
Done


http://gerrit.cloudera.org:8080/#/c/17839/6/src/kudu/rpc/proxy-test.cc@106
PS6, Line 106:   ASSERT_OK(CreateMessenger("client_messenger", 
&client_messenger));
             :   DnsResolver dns_resolver(1, 1024 * 1024);
             :   Proxy p(client_messe
> Does it make sense to add a testcase to cover this situation of address sup
Done


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

http://gerrit.cloudera.org:8080/#/c/17839/6/src/kudu/rpc/proxy.cc@103
PS6, Line 103:   if (!dns_resolver_) {
             :     return;
             :   }
> Should we return non-OK status here if the object was constructed with the
I opted for this to be a no-op and mentioned in the header what the behavior 
is, and added a test.



--
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: 7
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-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 20 Sep 2021 23:13:11 +0000
Gerrit-HasComments: Yes

Reply via email to