Todd Lipcon has posted comments on this change.

Change subject: KUDU-1508: enforce block limit on lbm containers
......................................................................


Patch Set 2:

(3 comments)

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

Line 270:       // 1. el6.9: 
https://bugzilla.redhat.com/show_bug.cgi?id=1351798.
sounds like kernel-2.6.32-674.el6 will have the fix for this stream


Line 357:     RETURN_NOT_OK(FindMountForPath(env_, mounts, p, &found_mount));
instead of parsing the mtab and doing this stuff, why not just use statfs() on 
the container file and check f_type == EXT4_SUPER_MAGIC? It's slightly less 
precise since ext3 and ext2 share the same magic number, but i dont think that 
really matters, since ext3 and ext2 wouldn't support hole punching anyway

I think that could simplify the patch a lot: you could just use fstatfs() on 
the container file when you first open it, and it'll give you both the block 
size as well as the FS type, so you can set the limit right there and not have 
to plumb it around through Data Dirs


http://gerrit.cloudera.org:8080/#/c/5403/2/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:

Line 1277:     FILE* f = setmntent(MOUNTED, "r");
apparently this is deprecated, and should use _PATH_MOUNTED instead 
(https://www.gnu.org/software/libc/manual/html_node/Mount-Information.html)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I63576d2bf2cc61b2deb70f7e166f08e0d4c66543
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to