A major bug that was not considering the compaction score of the n-1 level.

Summary:
The method Finalize() recomputes the compaction score of each
level and then sorts these score from largest to smallest. The
idea is that the level with the largest compaction score will
be a better candidate for compaction.  There are usually very
few levels, and a bubble sort code was used to sort these
compaction scores. There existed a bug in the sorting code that
skipped looking at the score for the n-1 level. This meant that
even if the compaction score of the n-1 level is large, it will
not be picked for compaction.

This patch fixes the bug and also introduces "asserts" in the
code to detect any possible inconsistencies caused by future bugs.

This bug existed in the very first code change that introduced
multi-threaded compaction to the leveldb code. That version of
code was committed on Oct 19th via
1ca0584345

Test Plan: make clean check OPT=-g

Reviewers: emayanke, sheki, MarkCallaghan

Reviewed By: sheki

CC: leveldb

Differential Revision: https://reviews.facebook.net/D6837
This commit is contained in:
Dhruba Borthakur 2012-11-20 09:08:11 -08:00
parent dde70898a1
commit 3754f2f4ff

View File

@ -1416,7 +1416,7 @@ void VersionSet::Finalize(Version* v) {
// sort all the levels based on their score. Higher scores get listed
// first. Use bubble sort because the number of entries are small.
for(int i = 0; i < NumberLevels()-2; i++) {
for (int j = i+1; j < NumberLevels()-2; j++) {
for (int j = i+1; j < NumberLevels()-1; j++) {
if (v->compaction_score_[i] < v->compaction_score_[j]) {
double score = v->compaction_score_[i];
int level = v->compaction_level_[i];
@ -1868,6 +1868,8 @@ Compaction* VersionSet::PickCompaction() {
//
// Find the compactions by size on all levels.
for (int i = 0; i < NumberLevels()-1; i++) {
assert(i == 0 || current_->compaction_score_[i] <=
current_->compaction_score_[i-1]);
level = current_->compaction_level_[i];
if ((current_->compaction_score_[i] >= 1)) {
c = PickCompactionBySize(level);
@ -2127,6 +2129,10 @@ bool Compaction::ShouldStopBefore(const Slice& internal_key) {
if (seen_key_) {
overlapped_bytes_ += grandparents_[grandparent_index_]->file_size;
}
assert(grandparent_index_ + 1 >= grandparents_.size() ||
icmp->Compare(grandparents_[grandparent_index_]->largest.Encode(),
grandparents_[grandparent_index_+1]->smallest.Encode())
< 0);
grandparent_index_++;
}
seen_key_ = true;