Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9027 )
Change subject: KUDU-1489: allow configuration of metadata dir ...................................................................... Patch Set 12: (6 comments) http://gerrit.cloudera.org:8080/#/c/9027/11/src/kudu/fs/fs_manager-test.cc File src/kudu/fs/fs_manager-test.cc: http://gerrit.cloudera.org:8080/#/c/9027/11/src/kudu/fs/fs_manager-test.cc@265 PS11, Line 265: // We should be able to verify that the metadata is in the WAL root. > Nit: is 'explicate' the right choice here? I substitute it with 'analyze' o You're right. Wanted to say that we should be able to _explicitly see_. I'll go with "verify". http://gerrit.cloudera.org:8080/#/c/9027/11/src/kudu/fs/fs_manager.h File src/kudu/fs/fs_manager.h: http://gerrit.cloudera.org:8080/#/c/9027/11/src/kudu/fs/fs_manager.h@268 PS11, Line 268: // Initializes, sanitizes, and canonicalizes the filesystem roots. > Isn't this exactly the same as FRIEND_TEST(MiniTabletServerTest...) above? Done http://gerrit.cloudera.org:8080/#/c/9027/11/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/9027/11/src/kudu/tools/kudu-tool-test.cc@2214 PS11, Line 2214: ASSERT_OK(env_->CreateDir(kTestDir)); > ASSERT_OK Done http://gerrit.cloudera.org:8080/#/c/9027/11/src/kudu/tools/kudu-tool-test.cc@2229 PS11, Line 2229: Status s = RunTool(Substitute("fs dump uuid --fs_wal_dir=$0", opts.wal_root), > ASSERT on the expected status? Ah, yeah. Thought it was void like RunAction*. http://gerrit.cloudera.org:8080/#/c/9027/11/src/kudu/tools/tool_action_fs.cc File src/kudu/tools/tool_action_fs.cc: http://gerrit.cloudera.org:8080/#/c/9027/11/src/kudu/tools/tool_action_fs.cc@764 PS11, Line 764: .AddOptionalParameter("fs_wal_dir") > Since you're adding these, mind reordering the optional parameters to be al Done http://gerrit.cloudera.org:8080/#/c/9027/11/src/kudu/tserver/mini_tablet_server-test.cc File src/kudu/tserver/mini_tablet_server-test.cc: http://gerrit.cloudera.org:8080/#/c/9027/11/src/kudu/tserver/mini_tablet_server-test.cc@60 PS11, Line 60: ot), or > Nit: older Done -- To view, visit http://gerrit.cloudera.org:8080/9027 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I375c6b2eb283db5fa9c956135d98252c8781f5db Gerrit-Change-Number: 9027 Gerrit-PatchSet: 12 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Thu, 18 Jan 2018 02:24:28 +0000 Gerrit-HasComments: Yes
