Skip to content

Commit

Permalink
Make InMemoryEnv more consistent with filesystem based Env's.
Browse files Browse the repository at this point in the history
Env's (like the POSIX Env) which use an actual filesystem behave
differently than InMemoryEnv with regards to writing data to a currently
open file.

InMemoryEnv::NewWritableFile would previously delete that file,
if it was open, before creating a new file so any previously
open file would be unlinked. This change truncates an open file
so that subsequent reads will read that new data.

This should have no impact on leveldb as it never has the same
file open for both read and write access. This change is only
being made for tests (specifically a future change to corruption_test)
to allow them to be decoupled from the underlying platform and
allow them to use an Env.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=237858231
  • Loading branch information
cmumford committed Mar 11, 2019
1 parent cf1d1ab commit dd90626
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 16 deletions.
46 changes: 30 additions & 16 deletions helpers/memenv/memenv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,22 @@ class FileState {
}
}

uint64_t Size() const { return size_; }
uint64_t Size() const {
MutexLock lock(&blocks_mutex_);
return size_;
}

void Truncate() {
MutexLock lock(&blocks_mutex_);
for (char*& block : blocks_) {
delete[] block;
}
blocks_.clear();
size_ = 0;
}

Status Read(uint64_t offset, size_t n, Slice* result, char* scratch) const {
MutexLock lock(&blocks_mutex_);
if (offset > size_) {
return Status::IOError("Offset greater than file size.");
}
Expand Down Expand Up @@ -100,6 +113,7 @@ class FileState {
const char* src = data.data();
size_t src_len = data.size();

MutexLock lock(&blocks_mutex_);
while (src_len > 0) {
size_t avail;
size_t offset = size_ % kBlockSize;
Expand Down Expand Up @@ -128,10 +142,7 @@ class FileState {
private:
// Private since only Unref() should be used to delete it.
~FileState() {
for (std::vector<char*>::iterator i = blocks_.begin(); i != blocks_.end();
++i) {
delete [] *i;
}
Truncate();
}

// No copying allowed.
Expand All @@ -141,11 +152,9 @@ class FileState {
port::Mutex refs_mutex_;
int refs_ GUARDED_BY(refs_mutex_);

// The following fields are not protected by any mutex. They are only mutable
// while the file is being written, and concurrent access is not allowed
// to writable files.
std::vector<char*> blocks_;
uint64_t size_;
mutable port::Mutex blocks_mutex_;
std::vector<char*> blocks_ GUARDED_BY(blocks_mutex_);
uint64_t size_ GUARDED_BY(blocks_mutex_);

enum { kBlockSize = 8 * 1024 };
};
Expand Down Expand Up @@ -269,13 +278,18 @@ class InMemoryEnv : public EnvWrapper {
virtual Status NewWritableFile(const std::string& fname,
WritableFile** result) {
MutexLock lock(&mutex_);
if (file_map_.find(fname) != file_map_.end()) {
DeleteFileInternal(fname);
}
FileSystem::iterator it = file_map_.find(fname);

FileState* file = new FileState();
file->Ref();
file_map_[fname] = file;
FileState* file;
if (it == file_map_.end()) {
// File is not currently open.
file = new FileState();
file->Ref();
file_map_[fname] = file;
} else {
file = it->second;
file->Truncate();
}

*result = new WritableFileImpl(file);
return Status::OK();
Expand Down
23 changes: 23 additions & 0 deletions helpers/memenv/memenv_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,29 @@ TEST(MemEnvTest, LargeWrite) {
delete [] scratch;
}

TEST(MemEnvTest, OverwriteOpenFile) {
const char kWrite1Data[] = "Write #1 data";
const size_t kFileDataLen = sizeof(kWrite1Data) - 1;
const std::string kTestFileName = test::TmpDir() + "/leveldb-TestFile.dat";

ASSERT_OK(WriteStringToFile(env_, kWrite1Data, kTestFileName));

RandomAccessFile* rand_file;
ASSERT_OK(env_->NewRandomAccessFile(kTestFileName, &rand_file));

const char kWrite2Data[] = "Write #2 data";
ASSERT_OK(WriteStringToFile(env_, kWrite2Data, kTestFileName));

// Verify that overwriting an open file will result in the new file data
// being read from files opened before the write.
Slice result;
char scratch[kFileDataLen];
ASSERT_OK(rand_file->Read(0, kFileDataLen, &result, scratch));
ASSERT_EQ(0, result.compare(kWrite2Data));

delete rand_file;
}

TEST(MemEnvTest, DBTest) {
Options options;
options.create_if_missing = true;
Expand Down

0 comments on commit dd90626

Please sign in to comment.