Adar Dembo has posted comments on this change.

Change subject: [client-test] ServiceUnavailable retry scenario
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/5974/3/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:

PS3, Line 4669:   shared_ptr<KuduClient> client;
              :   ASSERT_OK(KuduClientBuilder()
              :       .add_master_server_addr(bound_rpc.ToString())
              :       .Build(&client));
Why can't we reuse client_?


Line 4683:   ASSERT_OK(cluster_->mini_master()->StartAlreadyInitted());
Why is it important to split up Init() and Start()? Don't you still wind up 
with ServiceUnavailable errors if you usurp the election lock belonging to a 
fully initialized catalog manager?

The Init/Start split is a fair amount of mini_master complexity for just one 
test. It also makes our eventual goal of unifying mini/external cluster 
start/stop routines harder. If you really need to slow down the catalog manager 
startup sequence, perhaps you can inject some latency instead? Check out 
util/fault_injection.h.


Line 4691:     gscoped_ptr<KuduTableCreator> 
table_creator(client->NewTableCreator());
unique_ptr


PS3, Line 4706: gscoped_ptr
unique_ptr.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief82add7b36c1f3875ea77e7f7503f6fb552838c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to