Skip to content

Commit

Permalink
BugFix: fs_posix.cc GetFreeSpace uses wrong value non-root users (fac…
Browse files Browse the repository at this point in the history
…ebook#8370)

Summary:
fs_posix.cc GetFreeSpace() calculates free space based upon a call to statvfs().  However, there are two extremely different values in statvfs's returned structure:  f_bfree which is free space for root and f_bavail which is free space for non-root users.  The existing code uses f_bfree.  Many disks have 5 to 10% of the total disk space reserved for root only.  Therefore GetFreeSpace() does not realize that non-root users may not have storage available.

This PR detects whether the effective posix user is root or not, then selects the appropriate available space value.

Pull Request resolved: facebook#8370

Reviewed By: mrambacher

Differential Revision: D29032710

Pulled By: jay-zhuang

fbshipit-source-id: 57feba34ed035615a479956d28f98d85735281c0
  • Loading branch information
matthewvon authored and facebook-github-bot committed Jun 10, 2021
1 parent f44e69c commit 5a2b4ed
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 10 deletions.
2 changes: 2 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
### Behavior Changes
* Added API comments clarifying safe usage of Disable/EnableManualCompaction and EventListener callbacks for compaction.
* Obsolete keys in the bottommost level that were preserved for a snapshot will now be cleaned upon snapshot release in all cases. This form of compaction (snapshot release triggered compaction) previously had an artificial limitation that multiple tombstones needed to be present.
### Bug Fixes
* fs_posix.cc GetFreeSpace() always report disk space available to root even when running as non-root. Linux defaults often have disk mounts with 5 to 10 percent of total space reserved only for root. Out of space could result for non-root users.

## 6.21.0 (2021-05-21)
### Bug Fixes
Expand Down
27 changes: 17 additions & 10 deletions env/fs_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#endif
#include <errno.h>
#include <fcntl.h>

#include <pthread.h>
#include <signal.h>
#include <stdio.h>
Expand All @@ -32,6 +31,7 @@
#include <sys/time.h>
#include <sys/types.h>
#include <time.h>

#include <algorithm>
// Get nano time includes
#if defined(OS_LINUX) || defined(OS_FREEBSD)
Expand Down Expand Up @@ -81,9 +81,7 @@ inline mode_t GetDBFileMode(bool allow_non_owner_access) {
return allow_non_owner_access ? 0644 : 0600;
}

static uint64_t gettid() {
return Env::Default()->GetThreadID();
}
static uint64_t gettid() { return Env::Default()->GetThreadID(); }

// list of pathnames that are locked
// Only used for error message.
Expand Down Expand Up @@ -267,8 +265,7 @@ class PosixFileSystem : public FileSystem {
}

virtual IOStatus OpenWritableFile(const std::string& fname,
const FileOptions& options,
bool reopen,
const FileOptions& options, bool reopen,
std::unique_ptr<FSWritableFile>* result,
IODebugContext* /*dbg*/) {
result->reset();
Expand Down Expand Up @@ -551,8 +548,8 @@ class PosixFileSystem : public FileSystem {
}

IOStatus NewLogger(const std::string& fname, const IOOptions& /*opts*/,
std::shared_ptr<Logger>* result,
IODebugContext* /*dbg*/) override {
std::shared_ptr<Logger>* result,
IODebugContext* /*dbg*/) override {
FILE* f = nullptr;
int fd;
{
Expand Down Expand Up @@ -909,7 +906,17 @@ class PosixFileSystem : public FileSystem {
return IOError("While doing statvfs", fname, errno);
}

*free_space = ((uint64_t)sbuf.f_bsize * sbuf.f_bfree);
// sbuf.bfree is total free space available to root
// sbuf.bavail is total free space available to unprivileged user
// sbuf.bavail <= sbuf.bfree ... pick correct based upon effective user id
if (geteuid()) {
// non-zero user is unprivileged, or -1 if error. take more conservative
// size
*free_space = ((uint64_t)sbuf.f_bsize * sbuf.f_bavail);
} else {
// root user can access all disk space
*free_space = ((uint64_t)sbuf.f_bsize * sbuf.f_bfree);
}
return IOStatus::OK();
}

Expand Down Expand Up @@ -938,7 +945,7 @@ class PosixFileSystem : public FileSystem {
}

FileOptions OptimizeForLogWrite(const FileOptions& file_options,
const DBOptions& db_options) const override {
const DBOptions& db_options) const override {
FileOptions optimized = file_options;
optimized.use_mmap_writes = false;
optimized.use_direct_writes = false;
Expand Down

0 comments on commit 5a2b4ed

Please sign in to comment.