David Ribeiro Alves has posted comments on this change.

Change subject: [catalog_manager] categorization of rw operation failures
......................................................................


Patch Set 13:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6170/13/src/kudu/integration-tests/catalog_manager_tsk-itest.cc
File src/kudu/integration-tests/catalog_manager_tsk-itest.cc:

PS13, Line 61:     std::copy(master_flags.begin(), master_flags.end(),
             :         std::back_inserter(cluster_opts_.extra_master_flags))
you're adding these flags twice, once to common and one to the masters. the 
master daemon receives both.


PS13, Line 80:   // Wait while the cluster runs -- there should be multiple TSK 
generated.
             :   const double run_time_seconds = AllowSlowTests() ? 300 : 60;
             :   SleepFor(MonoDelta::FromSeconds(run_time_seconds));
I think we need to have a workload of some sort so that we're sure that 
everything eventually is ok. since we can't use the client as it can't handle 
this situation, could you use the rpc api directly to perform some simple rpcs 
against the masters and make sure that eventually you get a valid token.
I assume such a workload would fail now since latency >> than the heartbeat 
interval.


http://gerrit.cloudera.org:8080/#/c/6170/13/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

Line 605
random formatting changes such as this to parts of the code otherwise unchanged 
are subjective (the previous person probably thought it looked better this way 
and this passed code review) and put more burden on the reviewer, which must 
then make a judgement call on whether these look better or write a comment such 
as this.

please revert this and the changes below.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I826826049e3c08a6c8345949690cbbedaea32ff8
Gerrit-PatchSet: 13
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: Dan Burkert <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to