Attila Bukor has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10293 )

Change subject: KUDU-2426 Fix WRONG_SERVER_UUID case in ksck
......................................................................


Patch Set 9:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10293/8/src/kudu/tools/ksck.cc
File src/kudu/tools/ksck.cc:

http://gerrit.cloudera.org:8080/#/c/10293/8/src/kudu/tools/ksck.cc@296
PS8, Line 296: ts->FetchInfo(&health).AndThen([&ts,&health]() {
             :                 if (FLAGS_consensus) {
             :                   return ts->FetchConsensusState(&h
> Ah, looking here, I don't think `health` gets set if `FetchConsensusState()
Updated FetchConsensusState() to FetchConsensusState(&health), this felt more 
natural, + this way we can introduce other statuses later, for example 
CONSENSUS_FETCH_FAILED instead of the more generic UNAVAILABLE. This felt out 
of scope though.


http://gerrit.cloudera.org:8080/#/c/10293/8/src/kudu/tools/ksck_remote.h
File src/kudu/tools/ksck_remote.h:

http://gerrit.cloudera.org:8080/#/c/10293/8/src/kudu/tools/ksck_remote.h@28
PS8, Line 28: #include "kudu/util/net/net_util.h"
> We should be able to forward declare KsckServerHealth, no?
yep, declared it in ksck.h, thanks for the tip



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b4f50fe4dd94450b4f2e34dbad315bd761b071f
Gerrit-Change-Number: 10293
Gerrit-PatchSet: 9
Gerrit-Owner: Attila Bukor <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Will Berkeley <[email protected]>
Gerrit-Comment-Date: Wed, 16 May 2018 11:01:22 +0000
Gerrit-HasComments: Yes

Reply via email to