Adar Dembo has posted comments on this change.

Change subject: Allow for reserving disk space for non-Kudu processes

Patch Set 8:

File src/kudu/fs/

PS8, Line 968: // Let the disk-full cache expire.
             :   SleepFor(MonoDelta::FromMilliseconds(100));
Do we still need this sleep now that we're using a FINE clock?
File src/kudu/fs/

Line 1292:     }
> Yes, preallocation sounds a little bit broken.
I see your point (regarding soft vs. hard limits). So let's proceed with what 
you have, provided:
1. Add a note to the help for data_dirs_reserved_bytes explaining that it'll 
only work when log_container_preallocate_bytes is non-zero.
2. Add a comment here explaining the "preallocation size as number of bytes we 
think we're about to consume" inaccuracy.
File src/kudu/fs/

PS8, Line 1234: Initialize the
              :   // indexes from the FullDiskCache.
Nit: it's not using indexes anymore.

Line 1240:       full_root_paths.insert(root_paths_[i]);
Why not InsertOrDie() here and below? I don't think root_paths_ can have 
duplicates, can it?

PS8, Line 1302: "Log block manager: Insufficient disk space under path "
              :                    << container->root_path() << ": Creation of 
new data blocks under this path "
              :                    << "can be retried after " << 
              :                    << " seconds: " << s.ToString();
Nit: I think getting the gist of the entire line would be easier if you used 
Substitute() to build it.
File src/kudu/util/

Line 46: TAG_FLAG(disk_reserved_prefixes_with_bytes_free_for_testing, unsafe);
> I'm changing them in the integration tests.
Ah, didn't notice that.

To view, visit
To unsubscribe, visit

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

Reply via email to