You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2017/07/10 19:15:22 UTC

[kudu-CR] disk failure: make DataDirManager failure-aware

Todd Lipcon has posted comments on this change.

Change subject: disk failure: make DataDirManager failure-aware
......................................................................


Patch Set 13:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7028/13/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:

PS13, Line 58: DEFINE_int32(fs_target_data_dirs_per_tablet, 0,
             :               "Indicates the target number of data dirs to spread each "
             :               "tablet's data across. If greater than the number of data dirs "
             :               "available, data will be striped across those available. The "
             :               "default value 0 indicates striping should occur across all "
             :               "data directories.");
             : DEFINE_validator(fs_target_data_dirs_per_tablet,
             :     [](const char* /*n*/, int32_t v) { return v >= -1; });
if -1 is a valid value, can you document what it means?


Line 488:         DCHECK_GE(group_target_size, 0);
this assertion probably belongs one line up, right? otherwise it's a no-op


PS13, Line 497: DataDirGroup for tablet $0 is smaller than targeted
for a user-facing message, can we make this a bit clearer and with less exposure of implementation details like class names? eg something like:

Only able to allocate <N> of requested <M> directories for tablet <T>. (<A> dirs total, <B> full, <C> failed)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iee212793152de5de5198751d649ab34fb97f6aa2
Gerrit-PatchSet: 13
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes