Mike Percy has posted comments on this change.
Change subject: Allow for reserving disk space for non-Kudu processes
Patch Set 4:
Line 13: reservation limit then a crash will result.
> Why did you choose for the behavior to be a crash instead of just returning
The problem is that if this guy is a leader then he can maintain his leadership
and never make progress. So it doesn't seem to make sense not to crash.
Line 55: // Note: This functionality is not implemented in the file block
: // this test is disabled on non-Linux platforms.
> Can't we compile the code on all platforms but, at runtime, test the kind o
What is the benefit of that? I just basically copied what we do for similar
Line 212: // Returns information about a mounted filesystem.
> No unit test in env-test for StatVfs?
It's literally a pass-through for statvfs().
Line 214: virtual Status StatVfs(const std::string& path, struct statvfs*
buf) = 0;
> Not a fan of leaking a POSIX-specific struct into the rest of Kudu. AFAICT
This was done for expediency; Since this is not an external API it's a decision
that can be redone later if we need to. However I don't think we have plans to
support non-POSIX systems anyway so it doesn't seem to be a big problem.
Line 997: return Status::IOError("statvfs: " + path,
> You should use the IOError() function so that FLAGS_suicide_on_eio will be
wasn't aware of that, ok
Line 43: Number of inodes to reserve on each filesystem for non-Kudu usage"
> Why would an operator care to reserve the number of inodes? Do other system
This is something Todd suggested we add. I wasn't sure what the default should
be so I defaulted it to 0.
I can imagine a case where we'd want to avoid taking up all the inodes,
resulting in a difficult to administer system (potentially preventing login?)
Line 125: if (PREDICT_FALSE(FLAGS_disk_reserved_bytes_free_for_testing > -1))
> If these for_testing flags are set, why bother with the StatVfs() call at a
It keeps the code simple.
To view, visit http://gerrit.cloudera.org:8080/3135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Owner: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>