Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9137 )

Change subject: internal_mini_cluster: support Cluster/LogVerifier
......................................................................


Patch Set 11: Code-Review+1

(5 comments)

Looks good! Thanks for doing this. I just have a few nits.

http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/consensus/log_reader.cc
File src/kudu/consensus/log_reader.cc:

http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/consensus/log_reader.cc@86
PS11, Line 86: RETURN_NOT_OK
how about: RETURN_NOT_OK_PREPEND(log_reader->Init(tablet_wal_dir), "Unable to 
initialize log reader");


http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/consensus/log_reader.cc@95
PS11, Line 95: metric_entity,
nit: unusual line wrapping


http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/integration-tests/log_verifier.h
File src/kudu/integration-tests/log_verifier.h:

http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/integration-tests/log_verifier.h@74
PS11, Line 74: const cluster::MiniCluster*
nit: const cluster::MiniCluster* const ?


http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/mini-cluster/external_mini_cluster.h
File src/kudu/mini-cluster/external_mini_cluster.h:

http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/mini-cluster/external_mini_cluster.h@267
PS11, Line 267: UUIDforTS
nit: how about UuidForTS() or GetUuidForTabletServer() ... the lowercase "for" 
is a bit unusual


http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/tablet/tablet_bootstrap.cc
File src/kudu/tablet/tablet_bootstrap.cc:

http://gerrit.cloudera.org:8080/#/c/9137/11/src/kudu/tablet/tablet_bootstrap.cc@708
PS11, Line 708: index
nit: mind naming this log_index? Kudu has all kinds of indexes everywhere, 
including loop indexes



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228a6e3ba1a42db4e243ffdc5116f0c60ee04a84
Gerrit-Change-Number: 9137
Gerrit-PatchSet: 11
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Tue, 13 Feb 2018 01:28:13 +0000
Gerrit-HasComments: Yes

Reply via email to