Todd Lipcon 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?
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?

Line 40:   int fd = open(argv[1], O_EXCL | O_WRONLY, 0644);
O_EXCL without O_CREAT is undefined

Line 54:   for (block_num = start_block; block_num < end_block; block_num += 
stride) {
any reason not to use C99 here?
File src/kudu/experiments/KUDU-1508/

PS1, Line 70: 1
instead of '1' how about using a string like 'fail_ok' so it's clearer

alternatively what about just using something like 'sudo umount || :'

PS1, Line 77: -l 
can you use the longer form (--length) here and elsewhere for flags that aren't 
well-known (eg I think mkfs -t or mount -o are commonly used)

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I710918a153a9e8e05e989fe63281891c9ebc7178
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to