Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9027 )
Change subject: KUDU-1489: allow configuration of metadata dir ...................................................................... Patch Set 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/9027/10/src/kudu/fs/data_dirs.cc File src/kudu/fs/data_dirs.cc: http://gerrit.cloudera.org:8080/#/c/9027/10/src/kudu/fs/data_dirs.cc@728 PS10, Line 728: DCHECK_NE(first_healthy, -1); // Guaranteed by LoadInstances(). maybe just make this a CHECK since it'll avoid some ugly stack overwriting on the next line, and this isn't any sort of perf-critical code http://gerrit.cloudera.org:8080/#/c/9027/10/src/kudu/fs/fs_manager.cc File src/kudu/fs/fs_manager.cc: http://gerrit.cloudera.org:8080/#/c/9027/10/src/kudu/fs/fs_manager.cc@94 PS10, Line 94: compatability typo http://gerrit.cloudera.org:8080/#/c/9027/10/src/kudu/fs/fs_manager.cc@96 PS10, Line 96: exist nit: exists http://gerrit.cloudera.org:8080/#/c/9027/10/src/kudu/fs/fs_manager.cc@98 PS10, Line 98: TAG_FLAG(fs_metadata_dir, advanced); maybe this should be stable out of the gate? -- 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: 10 Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Wed, 17 Jan 2018 06:07:00 +0000 Gerrit-HasComments: Yes