Adar Dembo has posted comments on this change. Change subject: Allow for reserving disk space for non-Kudu processes ......................................................................
Patch Set 8: (6 comments) http://gerrit.cloudera.org:8080/#/c/3135/8/src/kudu/fs/block_manager-test.cc File src/kudu/fs/block_manager-test.cc: 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? http://gerrit.cloudera.org:8080/#/c/3135/5/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: 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. http://gerrit.cloudera.org:8080/#/c/3135/8/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: 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 " << FLAGS_log_block_manager_full_disk_cache_seconds : << " seconds: " << s.ToString(); Nit: I think getting the gist of the entire line would be easier if you used Substitute() to build it. http://gerrit.cloudera.org:8080/#/c/3135/5/src/kudu/util/env_util.cc File src/kudu/util/env_util.cc: 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 http://gerrit.cloudera.org:8080/3135 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifd0451d4dbddc1783019a53302de0263080939c7 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
