From fbbb8a6144c7cb93d3a7b950993b36187ce75a5d Mon Sep 17 00:00:00 2001 From: agiardullo Date: Fri, 18 Mar 2016 14:11:06 -0700 Subject: [PATCH] Add test for Snapshot 0 Summary: I ran into this assert when stress testing transactions. It's pretty easy to repro. Changing VersionSet::last_sequence_ to start at 1 seems pretty straightforward. We would just need to change the 4 callers of SetLastSequence(), including recovery code. I'd make this change myself, but I do not have enough time to test changes to recovery code-paths this week. But checking in this test case (disabled) for future fixing. Test Plan: n/a Reviewers: yhchiang, kradhakrishnan, andrewkr, anthony, IslamAbdelRahman Reviewed By: IslamAbdelRahman Subscribers: andrewkr, dhruba, leveldb Differential Revision: https://reviews.facebook.net/D55311 --- db/db_test2.cc | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/db/db_test2.cc b/db/db_test2.cc index 3d9820b65..921dbdd60 100644 --- a/db/db_test2.cc +++ b/db/db_test2.cc @@ -77,6 +77,29 @@ TEST_F(DBTest2, CacheIndexAndFilterWithDBRestart) { std::string value; value = Get(1, "a"); } + +TEST_F(DBTest2, DISABLED_FirstSnapshotTest) { + Options options; + options.write_buffer_size = 100000; // Small write buffer + options = CurrentOptions(options); + CreateAndReopenWithCF({"pikachu"}, options); + + // This snapshot will have sequence number 0. When compaction encounters + // this snapshot, CompactionIterator::findEarliestVisibleSnapshot() will + // assert as it expects non-zero snapshots. + // + // One fix would be to simply remove this assert. However, a better fix + // might + // be to always have db sequence numbers start from 1 so that no code is + // ever + // confused by 0. + const Snapshot* s1 = db_->GetSnapshot(); + + Put(1, "k1", std::string(100000, 'x')); // Fill memtable + Put(1, "k2", std::string(100000, 'y')); // Trigger flush + + db_->ReleaseSnapshot(s1); +} } // namespace rocksdb int main(int argc, char** argv) {