Skip to content

Commit

Permalink
Improved FileExists API
Browse files Browse the repository at this point in the history
Summary: Add new CheckFileExists method.  Considered changing the FileExists api but didn't want to break anyone's builds.

Test Plan: unit tests

Reviewers: yhchiang, igor, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D42003
  • Loading branch information
agiardullo committed Jul 21, 2015
1 parent 9f1de95 commit 0642940
Show file tree
Hide file tree
Showing 23 changed files with 191 additions and 96 deletions.
1 change: 1 addition & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* Deprecated purge_redundant_kvs_while_flush option.
* Removed BackupEngine::NewBackupEngine() and NewReadOnlyBackupEngine() that were deprecated in RocksDB 3.8. Please use BackupEngine::Open() instead.
* Deprecated Compaction Filter V2. We are not aware of any existing use-cases. If you use this filter, your compile will break with RocksDB 3.13. Please let us know if you use it and we'll put it back in RocksDB 3.14.
* Env::FileExists now returns a Status instead of a boolean

## 3.12.0 (7/2/2015)
### New Features
Expand Down
2 changes: 1 addition & 1 deletion db/compact_files_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ TEST_F(CompactFilesTest, ObsoleteFiles) {

// verify all compaction input files are deleted
for (auto fname : l0_files) {
ASSERT_TRUE(!env_->FileExists(fname));
ASSERT_EQ(Status::NotFound(), env_->FileExists(fname));
}
delete db;
}
Expand Down
15 changes: 12 additions & 3 deletions db/db_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,8 @@ Status DBImpl::Recover(
return s;
}

if (!env_->FileExists(CurrentFileName(dbname_))) {
s = env_->FileExists(CurrentFileName(dbname_));
if (s.IsNotFound()) {
if (db_options_.create_if_missing) {
s = NewDB();
is_new_db = true;
Expand All @@ -879,18 +880,26 @@ Status DBImpl::Recover(
return Status::InvalidArgument(
dbname_, "does not exist (create_if_missing is false)");
}
} else {
} else if (s.ok()) {
if (db_options_.error_if_exists) {
return Status::InvalidArgument(
dbname_, "exists (error_if_exists is true)");
}
} else {
// Unexpected error reading file
assert(s.IsIOError());
return s;
}
// Check for the IDENTITY file and create it if not there
if (!env_->FileExists(IdentityFileName(dbname_))) {
s = env_->FileExists(IdentityFileName(dbname_));
if (s.IsNotFound()) {
s = SetIdentityFile(env_, dbname_);
if (!s.ok()) {
return s;
}
} else if (!s.ok()) {
assert(s.IsIOError());
return s;
}
}

Expand Down
6 changes: 3 additions & 3 deletions db/db_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8450,13 +8450,13 @@ TEST_F(DBTest, DeleteMovedFileAfterCompaction) {
ASSERT_EQ("0,0,2", FilesPerLevel(0));

// iterator is holding the file
ASSERT_TRUE(env_->FileExists(dbname_ + moved_file_name));
ASSERT_OK(env_->FileExists(dbname_ + moved_file_name));

listener->SetExpectedFileName(dbname_ + moved_file_name);
iterator.reset();

// this file should have been compacted away
ASSERT_TRUE(!env_->FileExists(dbname_ + moved_file_name));
ASSERT_EQ(Status::NotFound(), env_->FileExists(dbname_ + moved_file_name));
listener->VerifyMatchedCount(1);
}
}
Expand Down Expand Up @@ -8703,7 +8703,7 @@ TEST_F(DBTest, DeleteObsoleteFilesPendingOutputs) {
ASSERT_EQ(metadata.size(), 2U);

// This file should have been deleted during last compaction
ASSERT_TRUE(!env_->FileExists(dbname_ + file_on_L2));
ASSERT_EQ(Status::NotFound(), env_->FileExists(dbname_ + file_on_L2));
listener->VerifyMatchedCount(1);
}

Expand Down
12 changes: 6 additions & 6 deletions db/deletefile_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -269,11 +269,11 @@ TEST_F(DeleteFileTest, DeleteLogFiles) {
// Should not succeed because live logs are not allowed to be deleted
std::unique_ptr<LogFile> alive_log = std::move(logfiles.back());
ASSERT_EQ(alive_log->Type(), kAliveLogFile);
ASSERT_TRUE(env_->FileExists(options_.wal_dir + "/" + alive_log->PathName()));
ASSERT_OK(env_->FileExists(options_.wal_dir + "/" + alive_log->PathName()));
fprintf(stdout, "Deleting alive log file %s\n",
alive_log->PathName().c_str());
ASSERT_TRUE(!db_->DeleteFile(alive_log->PathName()).ok());
ASSERT_TRUE(env_->FileExists(options_.wal_dir + "/" + alive_log->PathName()));
ASSERT_OK(env_->FileExists(options_.wal_dir + "/" + alive_log->PathName()));
logfiles.clear();

// Call Flush to bring about a new working log file and add more keys
Expand All @@ -287,13 +287,13 @@ TEST_F(DeleteFileTest, DeleteLogFiles) {
ASSERT_GT(logfiles.size(), 0UL);
std::unique_ptr<LogFile> archived_log = std::move(logfiles.front());
ASSERT_EQ(archived_log->Type(), kArchivedLogFile);
ASSERT_TRUE(env_->FileExists(options_.wal_dir + "/" +
archived_log->PathName()));
ASSERT_OK(
env_->FileExists(options_.wal_dir + "/" + archived_log->PathName()));
fprintf(stdout, "Deleting archived log file %s\n",
archived_log->PathName().c_str());
ASSERT_OK(db_->DeleteFile(archived_log->PathName()));
ASSERT_TRUE(!env_->FileExists(options_.wal_dir + "/" +
archived_log->PathName()));
ASSERT_EQ(Status::NotFound(), env_->FileExists(options_.wal_dir + "/" +
archived_log->PathName()));
CloseDB();
}

Expand Down
8 changes: 6 additions & 2 deletions db/fault_injection_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,14 @@ class FaultInjectionTestEnv : public EnvWrapper {
return Status::Corruption("Not Active");
}
// Not allow overwriting files
if (target()->FileExists(fname)) {
Status s = target()->FileExists(fname);
if (s.ok()) {
return Status::Corruption("File already exists.");
} else if (!s.IsNotFound()) {
assert(s.IsIOError());
return s;
}
Status s = target()->NewWritableFile(fname, result, soptions);
s = target()->NewWritableFile(fname, result, soptions);
if (s.ok()) {
result->reset(new TestWritableFile(fname, std::move(*result), this));
// WritableFileWriter* file is opened
Expand Down
14 changes: 9 additions & 5 deletions db/wal_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,15 @@ Status WalManager::GetSortedWalFiles(VectorLogPtr& files) {
files.clear();
// list wal files in archive dir.
std::string archivedir = ArchivalDirectory(db_options_.wal_dir);
if (env_->FileExists(archivedir)) {
Status exists = env_->FileExists(archivedir);
if (exists.ok()) {
s = GetSortedWalsOfType(archivedir, files, kArchivedLogFile);
if (!s.ok()) {
return s;
}
} else if (!exists.IsNotFound()) {
assert(s.IsIOError());
return s;
}

uint64_t latest_archived_log_number = 0;
Expand Down Expand Up @@ -313,9 +317,9 @@ Status WalManager::GetSortedWalsOfType(const std::string& path,
// re-try in case the alive log file has been moved to archive.
std::string archived_file = ArchivedLogFileName(path, number);
if (!s.ok() && log_type == kAliveLogFile &&
env_->FileExists(archived_file)) {
env_->FileExists(archived_file).ok()) {
s = env_->GetFileSize(archived_file, &size_bytes);
if (!s.ok() && !env_->FileExists(archived_file)) {
if (!s.ok() && env_->FileExists(archived_file).IsNotFound()) {
// oops, the file just got deleted from archived dir! move on
s = Status::OK();
continue;
Expand Down Expand Up @@ -380,7 +384,7 @@ Status WalManager::ReadFirstRecord(const WalFileType type,
if (type == kAliveLogFile) {
std::string fname = LogFileName(db_options_.wal_dir, number);
s = ReadFirstLine(fname, sequence);
if (env_->FileExists(fname) && !s.ok()) {
if (env_->FileExists(fname).ok() && !s.ok()) {
// return any error that is not caused by non-existing file
return s;
}
Expand All @@ -394,7 +398,7 @@ Status WalManager::ReadFirstRecord(const WalFileType type,
// maybe the file was deleted from archive dir. If that's the case, return
// Status::OK(). The caller with identify this as empty file because
// *sequence == 0
if (!s.ok() && !env_->FileExists(archived_file)) {
if (!s.ok() && env_->FileExists(archived_file).IsNotFound()) {
return Status::OK();
}
}
Expand Down
6 changes: 4 additions & 2 deletions hdfs/env_hdfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class HdfsEnv : public Env {
virtual Status NewDirectory(const std::string& name,
std::unique_ptr<Directory>* result);

virtual bool FileExists(const std::string& fname);
virtual Status FileExists(const std::string& fname);

virtual Status GetChildren(const std::string& path,
std::vector<std::string>* result);
Expand Down Expand Up @@ -279,7 +279,9 @@ class HdfsEnv : public Env {
return notsup;
}

virtual bool FileExists(const std::string& fname) override { return false; }
virtual Status FileExists(const std::string& fname) override {
return notsup;
}

virtual Status GetChildren(const std::string& path,
std::vector<std::string>* result) override {
Expand Down
10 changes: 7 additions & 3 deletions include/rocksdb/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,12 @@ class Env {
virtual Status NewDirectory(const std::string& name,
unique_ptr<Directory>* result) = 0;

// Returns true iff the named file exists.
virtual bool FileExists(const std::string& fname) = 0;
// Returns OK if the named file exists.
// NotFound if the named file does not exist,
// the calling process does not have permission to determine
// whether this file exists, or if the path is invalid.
// IOError if an IO Error was encountered
virtual Status FileExists(const std::string& fname) = 0;

// Store in *result the names of the children of the specified directory.
// The names are relative to "dir".
Expand Down Expand Up @@ -764,7 +768,7 @@ class EnvWrapper : public Env {
unique_ptr<Directory>* result) override {
return target_->NewDirectory(name, result);
}
bool FileExists(const std::string& f) override {
Status FileExists(const std::string& f) override {
return target_->FileExists(f);
}
Status GetChildren(const std::string& dir,
Expand Down
10 changes: 10 additions & 0 deletions include/rocksdb/status.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ class Status {
// Copy the specified status.
Status(const Status& s);
void operator=(const Status& s);
bool operator==(const Status& rhs) const;
bool operator!=(const Status& rhs) const;

// Return a success status.
static Status OK() { return Status(); }
Expand Down Expand Up @@ -164,6 +166,14 @@ inline void Status::operator=(const Status& s) {
}
}

inline bool Status::operator==(const Status& rhs) const {
return (code_ == rhs.code_);
}

inline bool Status::operator!=(const Status& rhs) const {
return !(*this == rhs);
}

} // namespace rocksdb

#endif // STORAGE_ROCKSDB_INCLUDE_STATUS_H_
5 changes: 3 additions & 2 deletions port/win/env_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1663,10 +1663,11 @@ class WinEnv : public Env {
return s;
}

virtual bool FileExists(const std::string& fname) override {
virtual Status FileExists(const std::string& fname) override {
// F_OK == 0
const int F_OK_ = 0;
return _access(fname.c_str(), F_OK_) == 0;
return _access(fname.c_str(), F_OK_) == 0 ? Status::OK()
: Status::NotFound();
}

virtual Status GetChildren(const std::string& dir,
Expand Down
3 changes: 2 additions & 1 deletion table/cuckoo_table_reader_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,8 @@ TEST_F(CuckooReaderTest, TestReadPerformance) {
"WARNING: Not compiled with DNDEBUG. Performance tests may be slow.\n");
#endif
for (uint64_t num : nums) {
if (FLAGS_write || !Env::Default()->FileExists(GetFileName(num))) {
if (FLAGS_write ||
Env::Default()->FileExists(GetFileName(num)).IsNotFound()) {
std::vector<std::string> all_keys;
GetKeys(num, &all_keys);
WriteFile(all_keys, num, hash_ratio);
Expand Down
6 changes: 3 additions & 3 deletions util/auto_roll_logger_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,9 @@ TEST_F(AutoRollLoggerTest, RollLogFileByTime) {

InitTestDb();
// -- Test the existence of file during the server restart.
ASSERT_TRUE(!env->FileExists(kLogFile));
ASSERT_EQ(Status::NotFound(), env->FileExists(kLogFile));
AutoRollLogger logger(Env::Default(), kTestDir, "", log_size, time);
ASSERT_TRUE(env->FileExists(kLogFile));
ASSERT_OK(env->FileExists(kLogFile));

RollLogFileByTimeTest(&logger, time, kSampleMessage + ":RollLogFileByTime");
}
Expand Down Expand Up @@ -397,7 +397,7 @@ TEST_F(AutoRollLoggerTest, LogFileExistence) {
options.max_log_file_size = 100 * 1024 * 1024;
options.create_if_missing = true;
ASSERT_OK(rocksdb::DB::Open(options, kTestDir, &db));
ASSERT_TRUE(env->FileExists(kLogFile));
ASSERT_OK(env->FileExists(kLogFile));
delete db;
}

Expand Down
12 changes: 5 additions & 7 deletions util/env_hdfs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -448,20 +448,18 @@ Status HdfsEnv::NewDirectory(const std::string& name,
}
}

bool HdfsEnv::FileExists(const std::string& fname) {

Status HdfsEnv::FileExists(const std::string& fname) {
int value = hdfsExists(fileSys_, fname.c_str());
switch (value) {
case HDFS_EXISTS:
return true;
return Status::OK();
case HDFS_DOESNT_EXIST:
return false;
return Status::NotFound();
default: // anything else should be an error
Log(InfoLogLevel::FATAL_LEVEL,
mylog, "FileExists hdfsExists call failed");
throw HdfsFatalException("hdfsExists call failed with error " +
ToString(value) + " on path " + fname +
".\n");
return Status::IOError("hdfsExists call failed with error " +
ToString(value) + " on path " + fname + ".\n");
}
}

Expand Down
28 changes: 24 additions & 4 deletions util/env_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "util/posix_logger.h"
#include "util/random.h"
#include "util/iostats_context_imp.h"
#include "util/string_util.h"
#include "util/sync_point.h"
#include "util/thread_status_updater.h"
#include "util/thread_status_util.h"
Expand Down Expand Up @@ -1055,8 +1056,25 @@ class PosixEnv : public Env {
return Status::OK();
}

virtual bool FileExists(const std::string& fname) override {
return access(fname.c_str(), F_OK) == 0;
virtual Status FileExists(const std::string& fname) override {
int result = access(fname.c_str(), F_OK);

if (result == 0) {
return Status::OK();
}

switch (errno) {
case EACCES:
case ELOOP:
case ENAMETOOLONG:
case ENOENT:
case ENOTDIR:
return Status::NotFound();
default:
assert(result == EIO || result == ENOMEM);
return Status::IOError("Unexpected error(" + ToString(result) +
") accessing file `" + fname + "' ");
}
}

virtual Status GetChildren(const std::string& dir,
Expand Down Expand Up @@ -1772,9 +1790,11 @@ void PosixEnv::WaitForJoin() {

std::string Env::GenerateUniqueId() {
std::string uuid_file = "/proc/sys/kernel/random/uuid";
if (FileExists(uuid_file)) {

Status s = FileExists(uuid_file);
if (s.ok()) {
std::string uuid;
Status s = ReadFileToString(this, uuid_file, &uuid);
s = ReadFileToString(this, uuid_file, &uuid);
if (s.ok()) {
return uuid;
}
Expand Down
8 changes: 6 additions & 2 deletions util/memenv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,14 @@ class InMemoryEnv : public EnvWrapper {
return Status::OK();
}

virtual bool FileExists(const std::string& fname) override {
virtual Status FileExists(const std::string& fname) override {
std::string nfname = NormalizeFileName(fname);
MutexLock lock(&mutex_);
return file_map_.find(nfname) != file_map_.end();
if (file_map_.find(nfname) != file_map_.end()) {
return Status::OK();
} else {
return Status::NotFound();
}
}

virtual Status GetChildren(const std::string& dir,
Expand Down
Loading

0 comments on commit 0642940

Please sign in to comment.