From 3795449c9dfb983168c52a2f3b228ea5e6ec7761 Mon Sep 17 00:00:00 2001
From: Andres Noetzli <andres.noetzli@gmail.com>
Date: Wed, 26 Aug 2015 10:10:26 -0700
Subject: [PATCH] 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
---
 db/db_test.cc     | 7 +++++--
 db/table_cache.cc | 2 +-
 db/table_cache.h  | 2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/db/db_test.cc b/db/db_test.cc
index ddd59e884..4c06fa9be 100644
--- a/db/db_test.cc
+++ b/db/db_test.cc
@@ -2327,13 +2327,16 @@ TEST_F(DBTest, GetProperty) {
   sleeping_task_low.WakeUp();
   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;
   Reopen(options);
   // After reopening, no table reader is loaded, so no memory for table readers
   ASSERT_TRUE(
       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_GT(int_num, 0U);
 
diff --git a/db/table_cache.cc b/db/table_cache.cc
index 09de6001c..35250c66e 100644
--- a/db/table_cache.cc
+++ b/db/table_cache.cc
@@ -123,7 +123,7 @@ Status TableCache::FindTable(const EnvOptions& env_options,
                            const_cast<bool*>(&no_io));
 
   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");
     }
     unique_ptr<TableReader> table_reader;
diff --git a/db/table_cache.h b/db/table_cache.h
index a33d9e791..60851e502 100644
--- a/db/table_cache.h
+++ b/db/table_cache.h
@@ -91,7 +91,7 @@ class TableCache {
                             bool no_io = false);
 
   // 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(
       const EnvOptions& toptions,
       const InternalKeyComparator& internal_comparator,