Skip to content

Commit

Permalink
Fixed style of snapshot compaction bug fix.
Browse files Browse the repository at this point in the history 
Simple formatting and variable name fixes to a prior
fix for snapshot compactions to conform to the Google
C++ style guide.
  • Loading branch information
@cmumford
cmumford committed Mar 27, 2019
1 parent 48a9a9a commit 783fcff
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 145 deletions.
76 changes: 41 additions & 35 deletions db/version_set.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

#include "db/version_set.h"

#include <algorithm>
#include <stdio.h>
#include <algorithm>
#include "db/filename.h"
#include "db/log_reader.h"
#include "db/log_writer.h"
Expand Down Expand Up @@ -1347,38 +1347,44 @@ Compaction* VersionSet::PickCompaction() {
return c;
}

// find the largest key in a vector of files. returns true if files it not empty
bool FindLargestKey(const InternalKeyComparator & icmp, const std::vector<FileMetaData*> & files, InternalKey *largestKey) {
// find the largest key in a vector of files. returns true if files it not
// empty.
bool FindLargestKey(const InternalKeyComparator& icmp,
const std::vector<FileMetaData*>& files,
InternalKey* largest_key) {
if (files.empty()) {
return false;
}
*largestKey = files[0]->largest;
*largest_key = files[0]->largest;
for (size_t i = 1; i < files.size(); ++i) {
FileMetaData* f = files[i];
if (icmp.Compare(f->largest, *largestKey) > 0) {
*largestKey = f->largest;
if (icmp.Compare(f->largest, *largest_key) > 0) {
*largest_key = f->largest;
}
}
return true;
}

// find minimum file b2=(l2, u2) in level file for which l2 > u1 and user_key(l2) = user_key(u1)
FileMetaData* FindSmallestBoundaryFile(const InternalKeyComparator & icmp,
const std::vector<FileMetaData*> & levelFiles,
const InternalKey & largestKey) {
// find minimum file b2=(l2, u2) in level file for which l2 > u1 and
// user_key(l2) = user_key(u1)
FileMetaData* FindSmallestBoundaryFile(
const InternalKeyComparator& icmp,
const std::vector<FileMetaData*>& level_files,
const InternalKey& largest_key) {
const Comparator* user_cmp = icmp.user_comparator();
FileMetaData* smallestBoundaryFile = NULL;
for (size_t i = 0; i < levelFiles.size(); ++i) {
FileMetaData* f = levelFiles[i];
if (icmp.Compare(f->smallest, largestKey) > 0 &&
user_cmp->Compare(f->smallest.user_key(), largestKey.user_key()) == 0) {
if (smallestBoundaryFile == NULL ||
icmp.Compare(f->smallest, smallestBoundaryFile->smallest) < 0) {
smallestBoundaryFile = f;
FileMetaData* smallest_boundary_file = NULL;
for (size_t i = 0; i < level_files.size(); ++i) {
FileMetaData* f = level_files[i];
if (icmp.Compare(f->smallest, largest_key) > 0 &&
user_cmp->Compare(f->smallest.user_key(), largest_key.user_key()) ==
0) {
if (smallest_boundary_file == NULL ||
icmp.Compare(f->smallest, smallest_boundary_file->smallest) < 0) {
smallest_boundary_file = f;
}
}
}
return smallestBoundaryFile;
return smallest_boundary_file;
}

// If there are two blocks, b1=(l1, u1) and b2=(l2, u2) and
Expand All @@ -1388,35 +1394,35 @@ FileMetaData* FindSmallestBoundaryFile(const InternalKeyComparator & icmp,
// i rather than from b1 because it searches level by level
// for records matching the supplied user key.
//
// This function extracts the largest file b1 from compactionFiles
// and then searches for a b2 in levelFiles for which user_key(u1) =
// This function extracts the largest file b1 from compaction_files
// and then searches for a b2 in level_files for which user_key(u1) =
// user_key(l2). If it finds such a file b2 (known as a boundary file)
// it adds it to compactionFiles and then searches again using this
// it adds it to compaction_files and then searches again using this
// new upper bound.
//
// parameters:
// in levelFiles: list of files to search for boundary files
// in/out compactionFiles: list of files to extend by adding boundary files
// in level_files: list of files to search for boundary files.
// in/out compaction_files: list of files to extend by adding boundary files.
void AddBoundaryInputs(const InternalKeyComparator& icmp,
const std::vector<FileMetaData*>& levelFiles,
std::vector<FileMetaData*>* compactionFiles) {
InternalKey largestKey;
const std::vector<FileMetaData*>& level_files,
std::vector<FileMetaData*>* compaction_files) {
InternalKey largest_key;

// find largestKey in compactionFiles, quick return if compactionFiles is
// find largest_key in compaction_files, quick return if compaction_files is
// empty
if (!FindLargestKey(icmp, *compactionFiles, &largestKey)) {
if (!FindLargestKey(icmp, *compaction_files, &largest_key)) {
return;
}

bool continueSearching = true;
while (continueSearching) {
FileMetaData* smallestBoundaryFile =
FindSmallestBoundaryFile(icmp, levelFiles, largestKey);
FileMetaData* smallest_boundary_file =
FindSmallestBoundaryFile(icmp, level_files, largest_key);

// if a boundary file was found advance largestKey, otherwise we're done
if (smallestBoundaryFile != NULL) {
compactionFiles->push_back(smallestBoundaryFile);
largestKey = smallestBoundaryFile->largest;
// if a boundary file was found advance largest_key, otherwise we're done.
if (smallest_boundary_file != NULL) {
compaction_files->push_back(smallest_boundary_file);
largest_key = smallest_boundary_file->largest;
} else {
continueSearching = false;
}
Expand Down
162 changes: 85 additions & 77 deletions db/version_set_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,153 +172,161 @@ TEST(FindFileTest, OverlappingFiles) {
ASSERT_TRUE(Overlaps("600", "700"));
}

void AddBoundaryInputs(const InternalKeyComparator &icmp,
const std::vector<FileMetaData *> &levelFiles,
std::vector<FileMetaData *> *compactionFiles);
void AddBoundaryInputs(const InternalKeyComparator& icmp,
const std::vector<FileMetaData*>& level_files,
std::vector<FileMetaData*>* compaction_files);

class AddBoundaryInputsTest {
public:
std::vector<FileMetaData *> levelFiles_;
std::vector<FileMetaData *> compactionFiles_;
std::vector<FileMetaData *> allFiles_;
std::vector<FileMetaData*> level_files_;
std::vector<FileMetaData*> compaction_files_;
std::vector<FileMetaData*> all_files_;
InternalKeyComparator icmp_;

AddBoundaryInputsTest() : icmp_(BytewiseComparator()){};

~AddBoundaryInputsTest() {
for (size_t i = 0; i < allFiles_.size(); ++i) {
delete allFiles_[i];
for (size_t i = 0; i < all_files_.size(); ++i) {
delete all_files_[i];
}
allFiles_.clear();
all_files_.clear();
};

FileMetaData *CreateFileMetaData(uint64_t number, InternalKey smallest,
FileMetaData* CreateFileMetaData(uint64_t number, InternalKey smallest,
InternalKey largest) {
FileMetaData *f = new FileMetaData();
FileMetaData* f = new FileMetaData();
f->number = number;
f->smallest = smallest;
f->largest = largest;
allFiles_.push_back(f);
all_files_.push_back(f);
return f;
}
};

TEST(AddBoundaryInputsTest, TestEmptyFileSets) {
AddBoundaryInputs(icmp_, levelFiles_, &compactionFiles_);
ASSERT_TRUE(compactionFiles_.empty());
ASSERT_TRUE(levelFiles_.empty());
AddBoundaryInputs(icmp_, level_files_, &compaction_files_);
ASSERT_TRUE(compaction_files_.empty());
ASSERT_TRUE(level_files_.empty());
}

TEST(AddBoundaryInputsTest, TestEmptyLevelFiles) {
FileMetaData *f1 =
FileMetaData* f1 =
CreateFileMetaData(1, InternalKey("100", 2, kTypeValue),
InternalKey(InternalKey("100", 1, kTypeValue)));
compactionFiles_.push_back(f1);
compaction_files_.push_back(f1);

AddBoundaryInputs(icmp_, levelFiles_, &compactionFiles_);
ASSERT_EQ(1, compactionFiles_.size());
ASSERT_EQ(f1, compactionFiles_[0]);
ASSERT_TRUE(levelFiles_.empty());
AddBoundaryInputs(icmp_, level_files_, &compaction_files_);
ASSERT_EQ(1, compaction_files_.size());
ASSERT_EQ(f1, compaction_files_[0]);
ASSERT_TRUE(level_files_.empty());
}

TEST(AddBoundaryInputsTest, TestEmptyCompactionFiles) {
FileMetaData *f1 =
FileMetaData* f1 =
CreateFileMetaData(1, InternalKey("100", 2, kTypeValue),
InternalKey(InternalKey("100", 1, kTypeValue)));
levelFiles_.push_back(f1);
level_files_.push_back(f1);

AddBoundaryInputs(icmp_, levelFiles_, &compactionFiles_);
ASSERT_TRUE(compactionFiles_.empty());
ASSERT_EQ(1, levelFiles_.size());
ASSERT_EQ(f1, levelFiles_[0]);
AddBoundaryInputs(icmp_, level_files_, &compaction_files_);
ASSERT_TRUE(compaction_files_.empty());
ASSERT_EQ(1, level_files_.size());
ASSERT_EQ(f1, level_files_[0]);
}

TEST(AddBoundaryInputsTest, TestNoBoundaryFiles) {
FileMetaData *f1 =
FileMetaData* f1 =
CreateFileMetaData(1, InternalKey("100", 2, kTypeValue),
InternalKey(InternalKey("100", 1, kTypeValue)));
FileMetaData *f2 =
FileMetaData* f2 =
CreateFileMetaData(1, InternalKey("200", 2, kTypeValue),
InternalKey(InternalKey("200", 1, kTypeValue)));
FileMetaData *f3 =
FileMetaData* f3 =
CreateFileMetaData(1, InternalKey("300", 2, kTypeValue),
InternalKey(InternalKey("300", 1, kTypeValue)));

levelFiles_.push_back(f3);
levelFiles_.push_back(f2);
levelFiles_.push_back(f1);
compactionFiles_.push_back(f2);
compactionFiles_.push_back(f3);
level_files_.push_back(f3);
level_files_.push_back(f2);
level_files_.push_back(f1);
compaction_files_.push_back(f2);
compaction_files_.push_back(f3);

AddBoundaryInputs(icmp_, levelFiles_, &compactionFiles_);
ASSERT_EQ(2, compactionFiles_.size());
AddBoundaryInputs(icmp_, level_files_, &compaction_files_);
ASSERT_EQ(2, compaction_files_.size());
}

TEST(AddBoundaryInputsTest, TestOneBoundaryFiles) {
FileMetaData *f1 =
FileMetaData* f1 =
CreateFileMetaData(1, InternalKey("100", 3, kTypeValue),
InternalKey(InternalKey("100", 2, kTypeValue)));
FileMetaData *f2 =
FileMetaData* f2 =
CreateFileMetaData(1, InternalKey("100", 1, kTypeValue),
InternalKey(InternalKey("200", 3, kTypeValue)));
FileMetaData *f3 =
FileMetaData* f3 =
CreateFileMetaData(1, InternalKey("300", 2, kTypeValue),
InternalKey(InternalKey("300", 1, kTypeValue)));

levelFiles_.push_back(f3);
levelFiles_.push_back(f2);
levelFiles_.push_back(f1);
compactionFiles_.push_back(f1);
level_files_.push_back(f3);
level_files_.push_back(f2);
level_files_.push_back(f1);
compaction_files_.push_back(f1);

AddBoundaryInputs(icmp_, levelFiles_, &compactionFiles_);
ASSERT_EQ(2, compactionFiles_.size());
ASSERT_EQ(f1, compactionFiles_[0]);
ASSERT_EQ(f2, compactionFiles_[1]);
AddBoundaryInputs(icmp_, level_files_, &compaction_files_);
ASSERT_EQ(2, compaction_files_.size());
ASSERT_EQ(f1, compaction_files_[0]);
ASSERT_EQ(f2, compaction_files_[1]);
}

TEST(AddBoundaryInputsTest, TestTwoBoundaryFiles) {
FileMetaData *f1 =
FileMetaData* f1 =
CreateFileMetaData(1, InternalKey("100", 6, kTypeValue),
InternalKey(InternalKey("100", 5, kTypeValue)));
FileMetaData *f2 =
FileMetaData* f2 =
CreateFileMetaData(1, InternalKey("100", 2, kTypeValue),
InternalKey(InternalKey("300", 1, kTypeValue)));
FileMetaData *f3 =
FileMetaData* f3 =
CreateFileMetaData(1, InternalKey("100", 4, kTypeValue),
InternalKey(InternalKey("100", 3, kTypeValue)));

levelFiles_.push_back(f2);
levelFiles_.push_back(f3);
levelFiles_.push_back(f1);
compactionFiles_.push_back(f1);
level_files_.push_back(f2);
level_files_.push_back(f3);
level_files_.push_back(f1);
compaction_files_.push_back(f1);

AddBoundaryInputs(icmp_, levelFiles_, &compactionFiles_);
ASSERT_EQ(3, compactionFiles_.size());
ASSERT_EQ(f1, compactionFiles_[0]);
ASSERT_EQ(f3, compactionFiles_[1]);
ASSERT_EQ(f2, compactionFiles_[2]);
AddBoundaryInputs(icmp_, level_files_, &compaction_files_);
ASSERT_EQ(3, compaction_files_.size());
ASSERT_EQ(f1, compaction_files_[0]);
ASSERT_EQ(f3, compaction_files_[1]);
ASSERT_EQ(f2, compaction_files_[2]);
}

TEST(AddBoundaryInputsTest, TestDisjoinFilePointers) {
FileMetaData *f1 = CreateFileMetaData(1, InternalKey("100", 6, kTypeValue), InternalKey(InternalKey("100", 5, kTypeValue)));
FileMetaData *f2 = CreateFileMetaData(1, InternalKey("100", 6, kTypeValue), InternalKey(InternalKey("100", 5, kTypeValue)));
FileMetaData *f3 = CreateFileMetaData(1, InternalKey("100", 2, kTypeValue), InternalKey(InternalKey("300", 1, kTypeValue)));
FileMetaData *f4 = CreateFileMetaData(1, InternalKey("100", 4, kTypeValue), InternalKey(InternalKey("100", 3, kTypeValue)));

levelFiles_.push_back(f2);
levelFiles_.push_back(f3);
levelFiles_.push_back(f4);

compactionFiles_.push_back(f1);

AddBoundaryInputs(icmp_, levelFiles_, &compactionFiles_);
ASSERT_EQ(3, compactionFiles_.size());
ASSERT_EQ(f1, compactionFiles_[0]);
ASSERT_EQ(f4, compactionFiles_[1]);
ASSERT_EQ(f3, compactionFiles_[2]);
FileMetaData* f1 =
CreateFileMetaData(1, InternalKey("100", 6, kTypeValue),
InternalKey(InternalKey("100", 5, kTypeValue)));
FileMetaData* f2 =
CreateFileMetaData(1, InternalKey("100", 6, kTypeValue),
InternalKey(InternalKey("100", 5, kTypeValue)));
FileMetaData* f3 =
CreateFileMetaData(1, InternalKey("100", 2, kTypeValue),
InternalKey(InternalKey("300", 1, kTypeValue)));
FileMetaData* f4 =
CreateFileMetaData(1, InternalKey("100", 4, kTypeValue),
InternalKey(InternalKey("100", 3, kTypeValue)));

level_files_.push_back(f2);
level_files_.push_back(f3);
level_files_.push_back(f4);

compaction_files_.push_back(f1);

AddBoundaryInputs(icmp_, level_files_, &compaction_files_);
ASSERT_EQ(3, compaction_files_.size());
ASSERT_EQ(f1, compaction_files_[0]);
ASSERT_EQ(f4, compaction_files_[1]);
ASSERT_EQ(f3, compaction_files_[2]);
}

} // namespace leveldb
} // namespace leveldb

int main(int argc, char** argv) {
return leveldb::test::RunAllTests();
Expand Down
Loading

0 comments on commit 783fcff

Please sign in to comment.