Mike Percy has posted comments on this change. Change subject: mini_cluster: support multiple data dirs ......................................................................
Patch Set 14: (5 comments) http://gerrit.cloudera.org:8080/#/c/7211/11/src/kudu/integration-tests/internal_mini_cluster.cc File src/kudu/integration-tests/internal_mini_cluster.cc: PS11, Line 60: opts_(st > Done, but it came at the cost of making opts_ non-const. great - seems alright to me http://gerrit.cloudera.org:8080/#/c/7211/14/src/kudu/master/mini_master.cc File src/kudu/master/mini_master.cc: Line 60: CHECK_OK(env_util::CreateDirIfMissing(Env::Default(), fs_root_, &created)); Are we sure this step is required now but not previously? If it is required now, perhaps this should be RETURN_NOT_OK() in Start() instead of a CHECK() in the constructor since it's not there to enforce a particular API, ensure correctness, or run on a non-main thread. Line 102: Shutdown(); Shutdown() still here in Restart() http://gerrit.cloudera.org:8080/#/c/7211/14/src/kudu/tserver/mini_tablet_server.cc File src/kudu/tserver/mini_tablet_server.cc: Line 65: CHECK_OK(env_util::CreateDirIfMissing(Env::Default(), fs_root, &created)); See comment in mini_master.cc http://gerrit.cloudera.org:8080/#/c/7211/14/src/kudu/tserver/tablet_server-test-base.h File src/kudu/tserver/tablet_server-test-base.h: Line 103: tablet_replica_.reset(); Why is this needed now but not before? -- To view, visit http://gerrit.cloudera.org:8080/7211 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I52c9352f1a3565d58149cf2c63d37246c6b39c23 Gerrit-PatchSet: 14 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@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-HasComments: Yes
