Fix DBTest.GetProperty

Summary:
DBTest.GetProperty was failing occasionally (see task #8131266). The reason was
that the test closed the database before the compaction was done. When the test
reopened the database, RocksDB would schedule a compaction which in turn
created table readers and lead the test to fail the assertion that
rocksdb.estimate-table-readers-mem is 0. In most cases, GetIntProperty() of
rocksdb.estimate-table-readers-mem happened before the compaction created the
table readers, hiding the problem. This patch changes the
WaitForFlushMemTable() to WaitForCompact(). WaitForFlushMemTable() is not
necessary because it is already being called a couple of lines before without
any insertions in-between.

Test Plan:
Insert `usleep(10000);` just after `Reopen(options);` on line 2333 to make the issue more likely, then run:
make db_test && while ./db_test --gtest_filter=DBTest.GetProperty; do true; done

Reviewers: rven, yhchiang, anthony, igor, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D45603
This commit is contained in:
Andres Noetzli 2015-08-26 10:10:26 -07:00
parent a7834a1292
commit 3795449c9d
3 changed files with 7 additions and 4 deletions

View file

@ -2327,13 +2327,16 @@ TEST_F(DBTest, GetProperty) {
sleeping_task_low.WakeUp(); sleeping_task_low.WakeUp();
sleeping_task_low.WaitUntilDone(); sleeping_task_low.WaitUntilDone();
dbfull()->TEST_WaitForFlushMemTable(); // Wait for compaction to be done. This is important because otherwise RocksDB
// might schedule a compaction when reopening the database, failing assertion
// (A) as a result.
dbfull()->TEST_WaitForCompact();
options.max_open_files = 10; options.max_open_files = 10;
Reopen(options); Reopen(options);
// After reopening, no table reader is loaded, so no memory for table readers // After reopening, no table reader is loaded, so no memory for table readers
ASSERT_TRUE( ASSERT_TRUE(
dbfull()->GetIntProperty("rocksdb.estimate-table-readers-mem", &int_num)); dbfull()->GetIntProperty("rocksdb.estimate-table-readers-mem", &int_num));
ASSERT_EQ(int_num, 0U); ASSERT_EQ(int_num, 0U); // (A)
ASSERT_TRUE(dbfull()->GetIntProperty("rocksdb.estimate-num-keys", &int_num)); ASSERT_TRUE(dbfull()->GetIntProperty("rocksdb.estimate-num-keys", &int_num));
ASSERT_GT(int_num, 0U); ASSERT_GT(int_num, 0U);

View file

@ -123,7 +123,7 @@ Status TableCache::FindTable(const EnvOptions& env_options,
const_cast<bool*>(&no_io)); const_cast<bool*>(&no_io));
if (*handle == nullptr) { if (*handle == nullptr) {
if (no_io) { // Dont do IO and return a not-found status if (no_io) { // Don't do IO and return a not-found status
return Status::Incomplete("Table not found in table_cache, no_io is set"); return Status::Incomplete("Table not found in table_cache, no_io is set");
} }
unique_ptr<TableReader> table_reader; unique_ptr<TableReader> table_reader;

View file

@ -91,7 +91,7 @@ class TableCache {
bool no_io = false); bool no_io = false);
// Return total memory usage of the table reader of the file. // Return total memory usage of the table reader of the file.
// 0 of table reader of the file is not loaded. // 0 if table reader of the file is not loaded.
size_t GetMemoryUsageByTableReader( size_t GetMemoryUsageByTableReader(
const EnvOptions& toptions, const EnvOptions& toptions,
const InternalKeyComparator& internal_comparator, const InternalKeyComparator& internal_comparator,