Andrew Wong has posted comments on this change. Change subject: mini_cluster: support multiple data dirs ......................................................................
Patch Set 7: (12 comments) http://gerrit.cloudera.org:8080/#/c/7211/6//COMMIT_MSG Commit Message: Line 7: mini_cluster: support multiple data dirs > Agreed Done Line 7: mini_cluster: support multiple data dirs > I think this would be better defined in MiniClusterOptions and then plumbed Done PS6, Line 18: be generated for tservers (a similar one is created for masters): : /test_dir/test_tserver/wal : /test_dir/test_tserver/data0 : /test_dir/test_tserver/data1 > Hmm, what do you think about: Done PS6, Line 18: be generated for tservers (a similar one is created for masters): : /test_dir/test_tserver/wal : /test_dir/test_tserver/data0 : /test_dir/test_tserver/data1 > +1 on this Done http://gerrit.cloudera.org:8080/#/c/7211/7/src/kudu/integration-tests/internal_mini_cluster.h File src/kudu/integration-tests/internal_mini_cluster.h: Line 201: const int num_data_dirs_; > Hmm, doesn't seem like we need this (or fs_root_) since we have a copy of o Interestingly we weren't copying opts_. I've removed it in favor of keeping these private members. http://gerrit.cloudera.org:8080/#/c/7211/7/src/kudu/master/mini_master.cc File src/kudu/master/mini_master.cc: Line 110: > Not your fault, but could you make 'opts' a class member and set it up in t Done http://gerrit.cloudera.org:8080/#/c/7211/6/src/kudu/tserver/mini_tablet_server.cc File src/kudu/tserver/mini_tablet_server.cc: Line 59: opts_.webserver_opts.port = 0; > Don't need std:: prefix. Done PS6, Line 60: _data_di > int here Done http://gerrit.cloudera.org:8080/#/c/7211/7/src/kudu/tserver/mini_tablet_server.cc File src/kudu/tserver/mini_tablet_server.cc: Line 21: #include <utility> > warning: #includes are not sorted properly [llvm-include-order] Done Line 32: #include "kudu/util/path_util.h" > warning: #includes are not sorted properly [llvm-include-order] Done http://gerrit.cloudera.org:8080/#/c/7211/6/src/kudu/tserver/mini_tablet_server.h File src/kudu/tserver/mini_tablet_server.h: PS6, Line 43: > An int would be fine here too. Done http://gerrit.cloudera.org:8080/#/c/7211/6/src/kudu/tserver/tablet_server-test-base.h File src/kudu/tserver/tablet_server-test-base.h: PS6, Line 100: > Don't use default arguments with virtual functions. They don't really work. Done -- 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: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
