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

Change subject: [tools] ksck improvements [2/n]: KUDU-2306: ksck master health 
check
......................................................................


Patch Set 5: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9882/3/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:

http://gerrit.cloudera.org:8080/#/c/9882/3/src/kudu/tools/ksck.h@240
PS3, Line 240: enum class KsckFetchState {
             :   // Information has not yet been fetched.
             :   UNINITIALIZED,
             :   // The attempt to fetch information failed.
             :   FETCH_FAILED,
             :   // Infor
> UNINITIALIZED means we haven't tried to fetch. FETCH_FAILED means we tried
Right, but does it matter whether we're FETCH_FAILED vs UNINITIALIZED? It seems 
like we won't know anything unless we're at FETCHED, and if that's not the case 
then <unknown> is appropriate regardless. Although this is probably useful for 
catching programming errors.


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

http://gerrit.cloudera.org:8080/#/c/9882/3/src/kudu/tools/ksck.cc@126
PS3, Line 126: std::ostream& operator<<(std::ostream& lhs, KsckFetchState 
state) {
             :   switch (state) {
             :     case KsckFetchState::UNINITIALIZED:
             :       lhs << "UNINITIALIZED";
             :       break;
             :     case KsckFetchState::FETCH_FAILED:
             :       lhs << "FETCH_FAILED";
             :       break;
             :     case KsckFetchState::FETCHED:
             :       lhs << "FETCHED";
             :       break;
             :     default:
             :       LOG(FATAL) << "unknown KsckFetchState";
             :   }
             :   return lhs;
             : }
> There's a comment on its declaration in ksck.h. It's required for using Ksc
Ack


http://gerrit.cloudera.org:8080/#/c/9882/3/src/kudu/tools/ksck_remote-test.cc
File src/kudu/tools/ksck_remote-test.cc:

http://gerrit.cloudera.org:8080/#/c/9882/3/src/kudu/tools/ksck_remote-test.cc@a217
PS3, Line 217:
> Mind if I pass on this one? If I do another rebase pass and remember I will
Yea, np



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0066468a0adc563979dcd2c61cd65d66978420c
Gerrit-Change-Number: 9882
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <[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: Thu, 05 Apr 2018 20:55:39 +0000
Gerrit-HasComments: Yes

Reply via email to