issue #9654: make --rocksdb.max-write-buffer-number work

jsteemann · jsteemann · commit f28d1798f86b · 2019-08-19T15:23:12.000+02:00
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,13 @@
 v3.5.1 (XXXX-XX-XX)
 -------------------
 
+* Fixed issue #9654: honor value of `--rocksdb.max-write-buffer-number` if it
+  is set to at least 9 (which is the recommended value). Ignore it if it is
+  set to a lower value than 9, and warn the end user about it.
+
+  Previous versions of ArangoDB always silently ignored the value of this setting 
+  and effectively hard-coded the value to 9.
+
 * Fixed cut'n'pasting code from the documentation into arangosh.
 
 * Fixed issue #9652: fix ArangoSearch wrong name collision and raising
diff --git a/arangod/RocksDBEngine/RocksDBEngine.cpp b/arangod/RocksDBEngine/RocksDBEngine.cpp
@@ -561,9 +561,17 @@ void RocksDBEngine::start() {
     _options.db_write_buffer_size = opts->_totalWriteBufferSize;
   }
 
-  // this is cfFamilies.size() + 2 ... but _option needs to be set before
-  //  building cfFamilies
-  _options.max_write_buffer_number = 7 + 2;
+  if (!application_features::ApplicationServer::server->options()->processingResult().touched("rocksdb.max-write-buffer-number")) {
+    // user hasn't explicitly set the number of write buffers, so we use a default value based
+    // on the number of column families
+    // this is cfFamilies.size() + 2 ... but _option needs to be set before
+    //  building cfFamilies
+    // Update max_write_buffer_number above if you change number of families used
+    _options.max_write_buffer_number = 7 + 2;
+  } else if (_options.max_write_buffer_number < 7 + 2) {
+    // use set the value explicitly, and it is lower than recommended
+    LOG_TOPIC("d5c49", WARN, Logger::ENGINES) << "ignoring value for option `--rocksdb.max-write-buffer-number` because it is lower than recommended";
+  }
 
   // cf options for definitons (dbs, collections, views, ...)
   rocksdb::ColumnFamilyOptions definitionsCF(_options);
@@ -601,8 +609,9 @@ void RocksDBEngine::start() {
   cfFamilies.emplace_back("VPackIndex", vpackFixedPrefCF);  // 4
   cfFamilies.emplace_back("GeoIndex", fixedPrefCF);         // 5
   cfFamilies.emplace_back("FulltextIndex", fixedPrefCF);    // 6
-  // DO NOT FORGET TO DESTROY THE CFs ON CLOSE
-  //  Update max_write_buffer_number above if you change number of families used
+    
+  TRI_ASSERT(static_cast<int>(_options.max_write_buffer_number) >= static_cast<int>(cfFamilies.size()));
+  // Update max_write_buffer_number above if you change number of families used
 
   std::vector<rocksdb::ColumnFamilyHandle*> cfHandles;
   size_t const numberOfColumnFamilies = RocksDBColumnFamily::minNumberOfColumnFamilies;
diff --git a/lib/ApplicationFeatures/RocksDBOptionFeature.cpp b/lib/ApplicationFeatures/RocksDBOptionFeature.cpp
@@ -46,7 +46,7 @@ RocksDBOptionFeature::RocksDBOptionFeature(application_features::ApplicationServ
       _transactionLockTimeout(rocksDBTrxDefaults.transaction_lock_timeout),
       _totalWriteBufferSize(rocksDBDefaults.db_write_buffer_size),
       _writeBufferSize(rocksDBDefaults.write_buffer_size),
-      _maxWriteBufferNumber(rocksDBDefaults.max_write_buffer_number),
+      _maxWriteBufferNumber(7 + 2), // number of column families plus 2
       _maxTotalWalSize(80 << 20),
       _delayedWriteRate(rocksDBDefaults.delayed_write_rate),
       _minWriteBufferNumberToMerge(rocksDBDefaults.min_write_buffer_number_to_merge),
diff --git a/lib/ApplicationFeatures/RocksDBOptionFeature.h b/lib/ApplicationFeatures/RocksDBOptionFeature.h
@@ -49,6 +49,7 @@ class RocksDBOptionFeature final : public application_features::ApplicationFeatu
   std::string _walDirectory;
   uint64_t _totalWriteBufferSize;
   uint64_t _writeBufferSize;
+  // Update max_write_buffer_number above if you change number of families used
   uint64_t _maxWriteBufferNumber;
   uint64_t _maxTotalWalSize;
   uint64_t _delayedWriteRate;
