issue #9654: make `--rocksdb.max-write-buffer-number` work (#9751)

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

* fix the logic

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 internal issue #4378: fix bug in MoveShard::abort which causes a
   duplicate entry in the follower list.
 

arangod/RocksDBEngine/RocksDBEngine.cpp

@@ -561,9 +561,18 @@ 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) {
+    // user set the value explicitly, and it is lower than recommended
+    _options.max_write_buffer_number = 7 + 2;
+    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 +610,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;

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),

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;