Adar Dembo has posted comments on this change.
Change subject: KUDU-1508: script for testing presence of bug and finding upper
Patch Set 1:
> 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.
Line 32: fprintf(stderr, "usage: %s <path> <start block> <end block>
> 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, 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 +=
> 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.
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-Owner: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>