Skip to content

Commit 677fee2

Browse files
committed
Make VersionSet::ReduceNumberOfLevels() static
Summary: A lot of our code implicitly assumes number_levels to be static. ReduceNumberOfLevels() breaks that assumption. For example, after calling ReduceNumberOfLevels(), DBImpl::NumberLevels() will be different from VersionSet::NumberLevels(). This is dangerous. Thankfully, it's not in public headers and is only used from LDB cmd tool. LDB tool is only using it statically, i.e. it never calls it with running DB instance. With this diff, we make it explicitly static. This way, we can assume number_levels to be immutable and not break assumption that lot of our code is relying upon. LDB tool can still use the method. Also, I removed the method from a separate file since it breaks filename completition. version_se<TAB> now completes to "version_set." instead of "version_set" (without the dot). I don't see a big reason that the function should be in a different file. Test Plan: reduce_levels_test Reviewers: dhruba, haobo, kailiu, sdong Reviewed By: kailiu CC: leveldb Differential Revision: https://reviews.facebook.net/D15303
1 parent c583157 commit 677fee2

6 files changed

+77
-107
lines changed

db/compaction_picker.cc

-8
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,7 @@ CompactionPicker::CompactionPicker(const Options* options,
4545
options_(options),
4646
num_levels_(options->num_levels),
4747
icmp_(icmp) {
48-
Init();
49-
}
50-
51-
void CompactionPicker::ReduceNumberOfLevels(int new_levels) {
52-
num_levels_ = new_levels;
53-
Init();
54-
}
5548

56-
void CompactionPicker::Init() {
5749
max_file_size_.reset(new uint64_t[NumberLevels()]);
5850
level_max_bytes_.reset(new uint64_t[NumberLevels()]);
5951
int target_file_size_multiplier = options_->target_file_size_multiplier;

db/compaction_picker.h

-5
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,6 @@ class CompactionPicker {
2727
CompactionPicker(const Options* options, const InternalKeyComparator* icmp);
2828
virtual ~CompactionPicker();
2929

30-
// See VersionSet::ReduceNumberOfLevels()
31-
void ReduceNumberOfLevels(int new_levels);
32-
3330
// Pick level and inputs for a new compaction.
3431
// Returns nullptr if there is no compaction to be done.
3532
// Otherwise returns a pointer to a heap-allocated object that
@@ -120,8 +117,6 @@ class CompactionPicker {
120117

121118
const Options* const options_;
122119
private:
123-
void Init();
124-
125120
int num_levels_;
126121

127122
const InternalKeyComparator* const icmp_;

db/version_set.cc

+69
Original file line numberDiff line numberDiff line change
@@ -1730,6 +1730,75 @@ Status VersionSet::Recover() {
17301730
return s;
17311731
}
17321732

1733+
Status VersionSet::ReduceNumberOfLevels(const std::string& dbname,
1734+
const Options* options,
1735+
const EnvOptions& storage_options,
1736+
int new_levels) {
1737+
if (new_levels <= 1) {
1738+
return Status::InvalidArgument(
1739+
"Number of levels needs to be bigger than 1");
1740+
}
1741+
1742+
const InternalKeyComparator cmp(options->comparator);
1743+
TableCache tc(dbname, options, storage_options, 10);
1744+
VersionSet versions(dbname, options, storage_options, &tc, &cmp);
1745+
Status status;
1746+
1747+
status = versions.Recover();
1748+
if (!status.ok()) {
1749+
return status;
1750+
}
1751+
1752+
Version* current_version = versions.current();
1753+
int current_levels = current_version->NumberLevels();
1754+
1755+
if (current_levels <= new_levels) {
1756+
return Status::OK();
1757+
}
1758+
1759+
// Make sure there are file only on one level from
1760+
// (new_levels-1) to (current_levels-1)
1761+
int first_nonempty_level = -1;
1762+
int first_nonempty_level_filenum = 0;
1763+
for (int i = new_levels - 1; i < current_levels; i++) {
1764+
int file_num = current_version->NumLevelFiles(i);
1765+
if (file_num != 0) {
1766+
if (first_nonempty_level < 0) {
1767+
first_nonempty_level = i;
1768+
first_nonempty_level_filenum = file_num;
1769+
} else {
1770+
char msg[255];
1771+
snprintf(msg, sizeof(msg),
1772+
"Found at least two levels containing files: "
1773+
"[%d:%d],[%d:%d].\n",
1774+
first_nonempty_level, first_nonempty_level_filenum, i,
1775+
file_num);
1776+
return Status::InvalidArgument(msg);
1777+
}
1778+
}
1779+
}
1780+
1781+
std::vector<FileMetaData*>* old_files_list = current_version->files_;
1782+
std::vector<FileMetaData*>* new_files_list =
1783+
new std::vector<FileMetaData*>[new_levels];
1784+
for (int i = 0; i < new_levels - 1; i++) {
1785+
new_files_list[i] = old_files_list[i];
1786+
}
1787+
1788+
if (first_nonempty_level > 0) {
1789+
new_files_list[new_levels - 1] = old_files_list[first_nonempty_level];
1790+
}
1791+
1792+
delete[] current_version->files_;
1793+
current_version->files_ = new_files_list;
1794+
current_version->num_levels_ = new_levels;
1795+
1796+
VersionEdit ve;
1797+
port::Mutex dummy_mutex;
1798+
MutexLock l(&dummy_mutex);
1799+
return versions.LogAndApply(&ve, &dummy_mutex, true);
1800+
}
1801+
17331802
Status VersionSet::DumpManifest(Options& options, std::string& dscname,
17341803
bool verbose, bool hex) {
17351804
struct LogReporter : public log::Reader::Reporter {

db/version_set.h

+7-1
Original file line numberDiff line numberDiff line change
@@ -283,10 +283,16 @@ class VersionSet {
283283
// Try to reduce the number of levels. This call is valid when
284284
// only one level from the new max level to the old
285285
// max level containing files.
286+
// The call is static, since number of levels is immutable during
287+
// the lifetime of a RocksDB instance. It reduces number of levels
288+
// in a DB by applying changes to manifest.
286289
// For example, a db currently has 7 levels [0-6], and a call to
287290
// to reduce to 5 [0-4] can only be executed when only one level
288291
// among [4-6] contains files.
289-
Status ReduceNumberOfLevels(int new_levels, port::Mutex* mu);
292+
static Status ReduceNumberOfLevels(const std::string& dbname,
293+
const Options* options,
294+
const EnvOptions& storage_options,
295+
int new_levels);
290296

291297
// Return the current version.
292298
Version* current() const { return current_; }

db/version_set_reduce_num_levels.cc

-77
This file was deleted.

util/ldb_cmd.cc

+1-16
Original file line numberDiff line numberDiff line change
@@ -1069,23 +1069,8 @@ void ReduceDBLevelsCommand::DoCommand() {
10691069
CloseDB();
10701070

10711071
EnvOptions soptions;
1072-
TableCache tc(db_path_, &opt, soptions, 10);
1073-
const InternalKeyComparator cmp(opt.comparator);
1074-
VersionSet versions(db_path_, &opt, soptions, &tc, &cmp);
1075-
// We rely the VersionSet::Recover to tell us the internal data structures
1076-
// in the db. And the Recover() should never do any change (like LogAndApply)
1077-
// to the manifest file.
1078-
st = versions.Recover();
1079-
if (!st.ok()) {
1080-
exec_state_ = LDBCommandExecuteResult::FAILED(st.ToString());
1081-
return;
1082-
}
1083-
1084-
port::Mutex mu;
1085-
mu.Lock();
1086-
st = versions.ReduceNumberOfLevels(new_levels_, &mu);
1087-
mu.Unlock();
10881072

1073+
st = VersionSet::ReduceNumberOfLevels(db_path_, &opt, soptions, new_levels_);
10891074
if (!st.ok()) {
10901075
exec_state_ = LDBCommandExecuteResult::FAILED(st.ToString());
10911076
return;

0 commit comments

Comments
 (0)