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) 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; : } > 1. FLAGS_refresh_is_full_with_expired_only_for_testing I see. What if we took a slightly different approach and: - continue to _always_ use EXPIRED_ONLY here. That way, we hit the disk error when we expect to in the test, i.e. we won't check for available space when we create blocks, but we _will_ hit a disk error when we write to those blocks. - update the EXPIRED_ONLY behavior to not take into account fullness, i.e. remove the is_full_ check for EXPIRED_ONLY. That way, we're not constantly running this space check, which might be expensive, especially if run for every block creation. AFAICT the RefreshIsFull() was originally written with fullness only in mind. Since we're trying to use it for more than just fullness, updating it to be "RefreshAvailableSpace()" or something might be worth doing. BTW another way to avoid this specific check for disk_failure-itest would be to have disk_failure-itest fail the glob "/data/*data" so it matches only to *.data and *.metadata files, rather than the entire directory. That said, I would prefer we consider the proposal above. Also curious if Adar agrees here. -- 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 <triplesheep0...@gmail.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Reviewer: ZhangYao <triplesheep0...@gmail.com> Gerrit-Comment-Date: Wed, 07 Aug 2019 00:43:31 +0000 Gerrit-HasComments: Yes
