fix crashes in stats and compaction filter for db_ttl_impl

Summary: fix crashes in stats and compaction filter for db_ttl_impl

Test Plan:
Ran build with lots of debugging
https://reviews.facebook.net/differential/diff/194175/

Reviewers: yhchiang, igor, rven

Reviewed By: igor

Subscribers: rven, dhruba

Differential Revision: https://reviews.facebook.net/D38001

db/db_impl.cc

@@ -265,8 +265,18 @@ DBImpl::DBImpl(const DBOptions& options, const std::string& dbname)
   LogFlush(db_options_.info_log);
 }
 
-void DBImpl::CancelAllBackgroundWork() {
+// Will only lock the mutex_ and wait for completion if wait is true
+void DBImpl::CancelAllBackgroundWork(bool wait) {
   shutting_down_.store(true, std::memory_order_release);
+  if (!wait) {
+    return;
+  }
+  // Wait for background work to finish
+  mutex_.Lock();
+  while (bg_compaction_scheduled_ || bg_flush_scheduled_ || notifying_events_) {
+    bg_cv_.Wait();
+  }
+  mutex_.Unlock();
 }
 
 DBImpl::~DBImpl() {
@@ -285,7 +295,11 @@ DBImpl::~DBImpl() {
     }
     versions_->GetColumnFamilySet()->FreeDeadColumnFamilies();
   }
-  CancelAllBackgroundWork();
+  // CancelAllBackgroundWork called with false means we just set the
+  // shutdown marker, while holding the mutex_ here. After which we
+  // do a variant of the waiting after we release the lock and unschedule work
+  // (to consider: moving all the waiting into CancelAllBackgroundWork(true))
+  CancelAllBackgroundWork(false);
   mutex_.Unlock();
   int compactions_unscheduled = env_->UnSchedule(this, Env::Priority::LOW);
   int flushes_unscheduled = env_->UnSchedule(this, Env::Priority::HIGH);

db/db_impl.h

@@ -275,7 +275,7 @@ class DBImpl : public DB {
 
   const SnapshotList& snapshots() const { return snapshots_; }
 
-  void CancelAllBackgroundWork();
+  void CancelAllBackgroundWork(bool wait);
 
  protected:
   Env* const env_;

include/rocksdb/statistics.h

@@ -240,7 +240,7 @@ enum Histograms : uint32_t {
   NUM_FILES_IN_SINGLE_COMPACTION,
   DB_SEEK,
   WRITE_STALL,
-  HISTOGRAM_ENUM_MAX,
+  HISTOGRAM_ENUM_MAX,  // TODO(ldemailly): enforce HistogramsNameMap match
 };
 
 const std::vector<std::pair<Histograms, std::string>> HistogramsNameMap = {
@@ -263,6 +263,7 @@ const std::vector<std::pair<Histograms, std::string>> HistogramsNameMap = {
   { SOFT_RATE_LIMIT_DELAY_COUNT, "rocksdb.soft.rate.limit.delay.count"},
   { NUM_FILES_IN_SINGLE_COMPACTION, "rocksdb.numfiles.in.singlecompaction" },
   { DB_SEEK, "rocksdb.db.seek.micros" },
+  { WRITE_STALL, "rocksdb.db.write.stall" },
 };
 
 struct HistogramData {

include/rocksdb/utilities/convenience.h

@@ -56,7 +56,8 @@ Status GetBlockBasedTableOptionsFromString(
 Status GetOptionsFromString(const Options& base_options,
                             const std::string& opts_str, Options* new_options);
 
-void CancelAllBackgroundWork(DB* db);
+/// Request stopping background work, if wait is true wait until it's done
+void CancelAllBackgroundWork(DB* db, bool wait = false);
 #endif  // ROCKSDB_LITE
 
 }  // namespace rocksdb

utilities/convenience/convenience.cc

@@ -15,8 +15,8 @@
 
 namespace rocksdb {
 
-void CancelAllBackgroundWork(DB* db) {
+void CancelAllBackgroundWork(DB* db, bool wait) {
-  (dynamic_cast<DBImpl*>(db))->CancelAllBackgroundWork();
+  (dynamic_cast<DBImpl*>(db))->CancelAllBackgroundWork(wait);
 }
 }  // namespace rocksdb
 

utilities/ttl/db_ttl_impl.cc

@@ -5,6 +5,7 @@
 
 #include "utilities/ttl/db_ttl_impl.h"
 
+#include "rocksdb/utilities/convenience.h"
 #include "rocksdb/utilities/db_ttl.h"
 #include "db/filename.h"
 #include "db/write_batch_internal.h"
@@ -34,7 +35,11 @@ void DBWithTTLImpl::SanitizeOptions(int32_t ttl, ColumnFamilyOptions* options,
 // Open the db inside DBWithTTLImpl because options needs pointer to its ttl
 DBWithTTLImpl::DBWithTTLImpl(DB* db) : DBWithTTL(db) {}
 
-DBWithTTLImpl::~DBWithTTLImpl() { delete GetOptions().compaction_filter; }
+DBWithTTLImpl::~DBWithTTLImpl() {
+  // Need to stop background compaction before getting rid of the filter
+  CancelAllBackgroundWork(db_, /* wait = */ true);
+  delete GetOptions().compaction_filter;
+}
 
 Status UtilityDB::OpenTtlDB(const Options& options, const std::string& dbname,
                             StackableDB** dbptr, int32_t ttl, bool read_only) {