Adar Dembo has posted comments on this change.

Change subject: Add Env::GetBytesFree() method

Patch Set 1:

File src/kudu/util/

Line 753: TEST_F(TestEnv, TestGetBytesFree) {
I appreciate the intent behind this test (i.e. every Env method should be 
tested), but it's a little flaky in that if some files are deleted in between 
the two GetBytesFree() calls, the ASSERT may fail.

To preserve the test and mitigate this issue, perhaps run the test body in a 
loop (up to some number of times), and just make sure the ASSERT succeeds at 
least once?
File src/kudu/util/env.h:

PS1, Line 173: // The implementation is likely to allocate bytes in sizes of 
blocks, so
             :   // the value returned in 'bytes_free' will likely not smoothly 
decrease as
             :   // bytes are written to the filesystem.
True, but the level of detail in the explanation is high enough to be a little 
confusing. Is it actually necessary? That is, do you anticipate GetBytesFree() 
callers who would care about the function returning an updated value after e.g. 
a single byte is written?
File src/kudu/util/

Line 918:     RETRY_ON_EINTR(ret, statvfs(path.c_str(), buf));
Could you make sure statvfs() exists on OS X?

Line 929:     *bytes_free = buf.f_frsize * buf.f_bavail;
Do you think we should check if we're running as uid 0, and if we are, use 
buf.f_bfree instead of buf.f_bavail?

To view, visit
To unsubscribe, visit

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

Reply via email to