Skip to content

Commit 36a5f8e

Browse files
committed
A number of fixes:
- Replace raw slice comparison with a call to user comparator. Added test for custom comparators. - Fix end of namespace comments. - Fixed bug in picking inputs for a level-0 compaction. When finding overlapping files, the covered range may expand as files are added to the input set. We now correctly expand the range when this happens instead of continuing to use the old range. For example, suppose L0 contains files with the following ranges: F1: a .. d F2: c .. g F3: f .. j and the initial compaction target is F3. We used to search for range f..j which yielded {F2,F3}. However we now expand the range as soon as another file is added. In this case, when F2 is added, we expand the range to c..j and restart the search. That picks up file F1 as well. This change fixes a bug related to deleted keys showing up incorrectly after a compaction as described in Issue 44. (Sync with upstream @25072954)
1 parent 299cced commit 36a5f8e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

102 files changed

+258
-146
lines changed

db/builder.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,4 +85,4 @@ Status BuildTable(const std::string& dbname,
8585
return s;
8686
}
8787

88-
}
88+
} // namespace leveldb

db/builder.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,6 @@ extern Status BuildTable(const std::string& dbname,
2929
Iterator* iter,
3030
FileMetaData* meta);
3131

32-
}
32+
} // namespace leveldb
3333

3434
#endif // STORAGE_LEVELDB_DB_BUILDER_H_

db/corruption_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ TEST(CorruptionTest, UnrelatedKeys) {
352352
ASSERT_EQ(Value(1000, &tmp2).ToString(), v);
353353
}
354354

355-
}
355+
} // namespace leveldb
356356

357357
int main(int argc, char** argv) {
358358
return leveldb::test::RunAllTests();

db/db_bench.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ struct ThreadState {
288288
}
289289
};
290290

291-
}
291+
} // namespace
292292

293293
class Benchmark {
294294
private:
@@ -829,7 +829,7 @@ class Benchmark {
829829
}
830830
};
831831

832-
}
832+
} // namespace leveldb
833833

834834
int main(int argc, char** argv) {
835835
FLAGS_write_buffer_size = leveldb::Options().write_buffer_size;

db/db_impl.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -985,7 +985,7 @@ static void CleanupIteratorState(void* arg1, void* arg2) {
985985
state->mu->Unlock();
986986
delete state;
987987
}
988-
}
988+
} // namespace
989989

990990
Iterator* DBImpl::NewInternalIterator(const ReadOptions& options,
991991
SequenceNumber* latest_snapshot) {
@@ -1378,4 +1378,4 @@ Status DestroyDB(const std::string& dbname, const Options& options) {
13781378
return result;
13791379
}
13801380

1381-
}
1381+
} // namespace leveldb

db/db_impl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,6 @@ extern Options SanitizeOptions(const std::string& db,
187187
const InternalKeyComparator* icmp,
188188
const Options& src);
189189

190-
}
190+
} // namespace leveldb
191191

192192
#endif // STORAGE_LEVELDB_DB_DB_IMPL_H_

db/db_iter.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,4 +296,4 @@ Iterator* NewDBIterator(
296296
return new DBIter(dbname, env, user_key_comparator, internal_iter, sequence);
297297
}
298298

299-
}
299+
} // namespace leveldb

db/db_iter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,6 @@ extern Iterator* NewDBIterator(
2121
Iterator* internal_iter,
2222
const SequenceNumber& sequence);
2323

24-
}
24+
} // namespace leveldb
2525

2626
#endif // STORAGE_LEVELDB_DB_DB_ITER_H_

db/db_test.cc

Lines changed: 100 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,33 @@ class DBTest {
136136
return result;
137137
}
138138

139+
// Return a string that contains all key,value pairs in order,
140+
// formatted like "(k1->v1)(k2->v2)".
141+
std::string Contents() {
142+
std::vector<std::string> forward;
143+
std::string result;
144+
Iterator* iter = db_->NewIterator(ReadOptions());
145+
for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
146+
std::string s = IterStatus(iter);
147+
result.push_back('(');
148+
result.append(s);
149+
result.push_back(')');
150+
forward.push_back(s);
151+
}
152+
153+
// Check reverse iteration results are the reverse of forward results
154+
int matched = 0;
155+
for (iter->SeekToLast(); iter->Valid(); iter->Prev()) {
156+
ASSERT_LT(matched, forward.size());
157+
ASSERT_EQ(IterStatus(iter), forward[forward.size() - matched - 1]);
158+
matched++;
159+
}
160+
ASSERT_EQ(matched, forward.size());
161+
162+
delete iter;
163+
return result;
164+
}
165+
139166
std::string AllEntriesFor(const Slice& user_key) {
140167
Iterator* iter = dbfull()->TEST_NewInternalIterator();
141168
InternalKey target(user_key, kMaxSequenceNumber, kTypeValue);
@@ -1048,6 +1075,49 @@ TEST(DBTest, OverlapInLevel0) {
10481075
ASSERT_EQ("NOT_FOUND", Get("600"));
10491076
}
10501077

1078+
TEST(DBTest, L0_CompactionBug_Issue44_a) {
1079+
Reopen();
1080+
ASSERT_OK(Put("b", "v"));
1081+
Reopen();
1082+
ASSERT_OK(Delete("b"));
1083+
ASSERT_OK(Delete("a"));
1084+
Reopen();
1085+
ASSERT_OK(Delete("a"));
1086+
Reopen();
1087+
ASSERT_OK(Put("a", "v"));
1088+
Reopen();
1089+
Reopen();
1090+
ASSERT_EQ("(a->v)", Contents());
1091+
env_->SleepForMicroseconds(1000000); // Wait for compaction to finish
1092+
ASSERT_EQ("(a->v)", Contents());
1093+
}
1094+
1095+
TEST(DBTest, L0_CompactionBug_Issue44_b) {
1096+
Reopen();
1097+
Put("","");
1098+
Reopen();
1099+
Delete("e");
1100+
Put("","");
1101+
Reopen();
1102+
Put("c", "cv");
1103+
Reopen();
1104+
Put("","");
1105+
Reopen();
1106+
Put("","");
1107+
env_->SleepForMicroseconds(1000000); // Wait for compaction to finish
1108+
Reopen();
1109+
Put("d","dv");
1110+
Reopen();
1111+
Put("","");
1112+
Reopen();
1113+
Delete("d");
1114+
Delete("b");
1115+
Reopen();
1116+
ASSERT_EQ("(->)(c->cv)", Contents());
1117+
env_->SleepForMicroseconds(1000000); // Wait for compaction to finish
1118+
ASSERT_EQ("(->)(c->cv)", Contents());
1119+
}
1120+
10511121
TEST(DBTest, ComparatorCheck) {
10521122
class NewComparator : public Comparator {
10531123
public:
@@ -1071,6 +1141,34 @@ TEST(DBTest, ComparatorCheck) {
10711141
<< s.ToString();
10721142
}
10731143

1144+
TEST(DBTest, CustomComparator) {
1145+
class NumberComparator : public Comparator {
1146+
public:
1147+
virtual const char* Name() const { return "test.NumberComparator"; }
1148+
virtual int Compare(const Slice& a, const Slice& b) const {
1149+
return (strtol(a.ToString().c_str(), NULL, 0) -
1150+
strtol(b.ToString().c_str(), NULL, 0));
1151+
}
1152+
virtual void FindShortestSeparator(std::string* s, const Slice& l) const {}
1153+
virtual void FindShortSuccessor(std::string* key) const {}
1154+
};
1155+
NumberComparator cmp;
1156+
Options new_options;
1157+
new_options.create_if_missing = true;
1158+
new_options.comparator = &cmp;
1159+
DestroyAndReopen(&new_options);
1160+
ASSERT_OK(Put("10", "ten"));
1161+
ASSERT_OK(Put("0x14", "twenty"));
1162+
for (int i = 0; i < 2; i++) {
1163+
ASSERT_EQ("ten", Get("10"));
1164+
ASSERT_EQ("ten", Get("0xa"));
1165+
ASSERT_EQ("twenty", Get("20"));
1166+
ASSERT_EQ("twenty", Get("0x14"));
1167+
Compact("0", "9999");
1168+
fprintf(stderr, "ss\n%s\n", DumpSSTableList().c_str());
1169+
}
1170+
}
1171+
10741172
TEST(DBTest, ManualCompaction) {
10751173
ASSERT_EQ(config::kMaxMemCompactLevel, 2)
10761174
<< "Need to update this test to match kMaxMemCompactLevel";
@@ -1207,7 +1305,7 @@ static void MTThreadBody(void* arg) {
12071305
fprintf(stderr, "... stopping thread %d after %d ops\n", t->id, int(counter));
12081306
}
12091307

1210-
}
1308+
} // namespace
12111309

12121310
TEST(DBTest, MultiThreaded) {
12131311
// Initialize state
@@ -1525,7 +1623,7 @@ void BM_LogAndApply(int iters, int num_base_files) {
15251623
buf, iters, us, ((float)us) / iters);
15261624
}
15271625

1528-
}
1626+
} // namespace leveldb
15291627

15301628
int main(int argc, char** argv) {
15311629
if (argc > 1 && std::string(argv[1]) == "--benchmark") {

db/dbformat.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,4 +115,4 @@ LookupKey::LookupKey(const Slice& user_key, SequenceNumber s) {
115115
end_ = dst;
116116
}
117117

118-
}
118+
} // namespace leveldb

0 commit comments

Comments
 (0)