Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13975 )

Change subject: Consider the available space when selecting data dirs for 
blocks.
......................................................................


Patch Set 6:

(1 comment)

The itself code looks fine, but I don't love the idea of having that test-only 
flag change the behavior like that. Could you explain a bit more why it's 
necessary?

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

http://gerrit.cloudera.org:8080/#/c/13975/6/src/kudu/fs/data_dirs.cc@991
PS6, Line 991:
             :   if (FLAGS_refresh_is_full_with_expired_only_for_testing) {
             :     // This currently should only be reached by 
disk_failure-itest.
             :     refresh_mode = DataDir::RefreshMode::EXPIRED_ONLY;
             :   }
I'm confused about why this is necessary. Why do we need this here?

Also, I'm curious, in most tests that use a minicluster, the data directories 
all share a single disk, even though they are different directories. If that's 
the case, do they all have the same available space? If so, I wonder if we 
should add a way to break ties. For example, select two, and if they have the 
same space, select the one that has fewer tablets. What do you think?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I194c4965ee64aed728e3b84e684c04d445cbe529
Gerrit-Change-Number: 13975
Gerrit-PatchSet: 6
Gerrit-Owner: ZhangYao <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <[email protected]>
Gerrit-Reviewer: ZhangYao <[email protected]>
Gerrit-Comment-Date: Tue, 06 Aug 2019 04:52:44 +0000
Gerrit-HasComments: Yes

Reply via email to