Adar Dembo has posted comments on this change. Change subject: KUDU-1508: script for testing presence of bug and finding upper bounds ......................................................................
Patch Set 1: (5 comments) > can you write in the commit message or in the test header something > about what the finding here means? if I'm interpreting it > correctly, we're trying to find the max number of extents in the > tree before it becomes multi-level, and that's dependent on the > file system's block size? Yes, I'll note that in the commit description. http://gerrit.cloudera.org:8080/#/c/4730/1/src/kudu/experiments/KUDU-1508/hole_punch_range.c File src/kudu/experiments/KUDU-1508/hole_punch_range.c: Line 32: fprintf(stderr, "usage: %s <path> <start block> <end block> <stride>\n", argv[0]); > clarify whether the file should exist or not, and how large it should be? Clarified the existence and the arguments. Size doesn't matter; fallocate() will no-op if you punch a hole beyond the size of the file. Line 40: int fd = open(argv[1], O_EXCL | O_WRONLY, 0644); > O_EXCL without O_CREAT is undefined Whoops, dropped O_EXCL. Line 54: for (block_num = start_block; block_num < end_block; block_num += stride) { > any reason not to use C99 here? Not a huge deal, but I didn't want to have to pass -std=c99 with older compilers (i.e. gcc 4.4 on el6). Given that this is the only C99ism, it seemed like a reasonable trade-off. http://gerrit.cloudera.org:8080/#/c/4730/1/src/kudu/experiments/KUDU-1508/run_test.sh File src/kudu/experiments/KUDU-1508/run_test.sh: PS1, Line 70: 1 > instead of '1' how about using a string like 'fail_ok' so it's clearer Sure, will use "fail_ok". I couldn't get "... || :" to work with the run() method, though maybe there's some quoting that would make it work. PS1, Line 77: -l > can you use the longer form (--length) here and elsewhere for flags that ar Sure, though I think this is the only place I could use a longopt. -- To view, visit http://gerrit.cloudera.org:8080/4730 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I710918a153a9e8e05e989fe63281891c9ebc7178 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
