Andrew Wong 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?
-1 shouldn't be valid.


Line 488:         DCHECK_GE(group_target_size, 0);
> this assertion probably belongs one line up, right? otherwise it's a no-op
Ah you're right, my mistake.


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 exp
Done


-- 
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 <[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-HasComments: Yes

Reply via email to