Fix node auto-configuration code:

The `node_size` configuration option is used to automatically
configure various parameters (cache sizes, timeouts, etc) for
the server.

A previous commit included changes that caused incorrect values
to be returned which can result in sub-optimal performance that
can manifest as difficulty syncing to the network, or increased
disk I/O and/or memory usage. The problem was introduced with
commit 66fad62e66.

This commit, if merged, fixes the code to ensure that the correct
values are returned and introduces a compile-time check to prevent
this issue from reoccurring.

src/ripple/app/ledger/impl/LedgerMaster.cpp

@@ -152,7 +152,7 @@ LedgerMaster::LedgerMaster (Application& app, Stopwatch& stopwatch,
     , fetch_depth_ (app_.getSHAMapStore ().clampFetchDepth (
         app_.config().FETCH_DEPTH))
     , ledger_history_ (app_.config().LEDGER_HISTORY)
-    , ledger_fetch_size_ (app_.config().getSize (siLedgerFetch))
+    , ledger_fetch_size_ (app_.config().getValueFor(SizedItem::ledgerFetch))
     , fetch_packs_ ("FetchPack", 65536, std::chrono::seconds {45}, stopwatch,
         app_.journal("TaggedCache"))
 {

src/ripple/app/main/Application.cpp

@@ -866,7 +866,7 @@ public:
                 TxDBInit);
             mTxnDB->getSession() <<
                 boost::str(boost::format("PRAGMA cache_size=-%d;") %
-                kilobytes(config_->getSize(siTxnDBCache)));
+                kilobytes(config_->getValueFor(SizedItem::txnDBCache)));
             mTxnDB->setupCheckpointing(m_jobQueue.get(), logs());
 
             if (!setup.standAlone ||
@@ -908,7 +908,7 @@ public:
                 LgrDBInit);
             mLedgerDB->getSession() <<
                 boost::str(boost::format("PRAGMA cache_size=-%d;") %
-                kilobytes(config_->getSize(siLgrDBCache)));
+                kilobytes(config_->getValueFor(SizedItem::lgrDBCache)));
             mLedgerDB->setupCheckpointing(m_jobQueue.get(), logs());
 
             // wallet database
@@ -964,24 +964,24 @@ public:
         // tune caches
         using namespace std::chrono;
         m_nodeStore->tune(
-            config_->getSize(siNodeCacheSize),
+            config_->getValueFor(SizedItem::nodeCacheSize),
-            seconds{config_->getSize(siNodeCacheAge)});
+            seconds{config_->getValueFor(SizedItem::nodeCacheAge)});
 
         m_ledgerMaster->tune(
-            config_->getSize(siLedgerSize),
+            config_->getValueFor(SizedItem::ledgerSize),
-            seconds{config_->getSize(siLedgerAge)});
+            seconds{config_->getValueFor(SizedItem::ledgerAge)});
 
         family().treecache().setTargetSize(
-            config_->getSize (siTreeCacheSize));
+            config_->getValueFor(SizedItem::treeCacheSize));
         family().treecache().setTargetAge(
-            seconds{config_->getSize(siTreeCacheAge)});
+            seconds{config_->getValueFor(SizedItem::treeCacheAge)});
 
         if (sFamily_)
         {
             sFamily_->treecache().setTargetSize(
-                config_->getSize(siTreeCacheSize));
+                config_->getValueFor(SizedItem::treeCacheSize));
             sFamily_->treecache().setTargetAge(
-                seconds{config_->getSize(siTreeCacheAge)});
+                seconds{config_->getValueFor(SizedItem::treeCacheAge)});
         }
 
         return true;
@@ -1135,7 +1135,7 @@ public:
         {
             using namespace std::chrono;
             sweepTimer_.expires_from_now(
-                seconds{config_->getSize(siSweepInterval)});
+                seconds{config_->getValueFor(SizedItem::sweepInterval)});
             sweepTimer_.async_wait (std::move (*optionalCountedHandler));
         }
     }

src/ripple/app/misc/SHAMapStoreImp.cpp

@@ -190,7 +190,7 @@ SHAMapStoreImp::SHAMapStoreImp(
         if (!section.exists("cache_mb"))
         {
             section.set("cache_mb", std::to_string(
-                config.getSize(siHashNodeDBCache)));
+                config.getValueFor(SizedItem::hashNodeDBCache)));
         }
 
         if (!section.exists("filter_bits") && (config.NODE_SIZE >= 2))

src/ripple/core/Config.h

@@ -29,6 +29,7 @@
 #include <boost/filesystem.hpp> // VFALCO FIX: This include should not be here
 #include <boost/lexical_cast.hpp>
 #include <boost/optional.hpp>
+#include <algorithm>
 #include <cstdint>
 #include <map>
 #include <string>
@@ -42,47 +43,21 @@ class Rules;
 
 //------------------------------------------------------------------------------
 
-enum SizedItemName
+enum class SizedItem : std::size_t
 {
-    siSweepInterval = 0,
+    sweepInterval = 0,
-    siNodeCacheSize,
+    treeCacheSize,
-    siNodeCacheAge,
+    treeCacheAge,
-    siTreeCacheSize,
+    ledgerSize,
-    siTreeCacheAge,
+    ledgerAge,
-    siSLECacheSize,
+    ledgerFetch,
-    siSLECacheAge,
+    nodeCacheSize,
-    siLedgerSize,
+    nodeCacheAge,
-    siLedgerAge,
+    hashNodeDBCache,
-    siLedgerFetch,
+    txnDBCache,
-    siHashNodeDBCache,
+    lgrDBCache
-    siTxnDBCache,
-    siLgrDBCache
 };
 
-static constexpr
-std::array<std::array<int, 5>, 13> sizedItems
-{{
-    //  tiny      small   medium  large   huge
-    {{  10,       30,     60,     90,     120     }}, // siSweepInterval
-    {{  2,        3,      5,      5,      8       }}, // siLedgerFetch
-
-    {{  16384,    32768,  131072, 262144, 524288  }}, // siNodeCacheSize
-    {{  60,       90,     120,    900,    1800    }}, // siNodeCacheAge
-
-    {{  128000,   256000, 512000, 768000, 2048000 }}, // siTreeCacheSize
-    {{  30,       60,     90,     120,    900     }}, // siTreeCacheAge
-
-    {{  4096,     8192,   16384,  65536,  131072  }}, // siSLECacheSize
-    {{  30,       60,     90,     120,    300     }}, // siSLECacheAge
-
-    {{  32,       128,    256,    384,    768     }}, // siLedgerSize
-    {{  30,       90,     180,    240,    900     }}, // siLedgerAge
-
-    {{  4,        12,     24,     64,     128     }}, // siHashNodeDBCache
-    {{  4,        12,     24,     64,     128     }}, // siTxnDBCache
-    {{  4,        8,      16,     32,     128     }}  // siLgrDBCache
-}};
-
 //  This entire derived class is deprecated.
 //  For new config information use the style implied
 //  in the base class. For existing config information
@@ -185,7 +160,8 @@ public:
     // Node storage configuration
     std::uint32_t                      LEDGER_HISTORY = 256;
     std::uint32_t                      FETCH_DEPTH = 1000000000;
-    int                         NODE_SIZE = 0;
+
+    std::size_t                        NODE_SIZE = 0;
 
     bool                        SSL_VERIFY = true;
     std::string                 SSL_VERIFY_FILE;
@@ -202,21 +178,6 @@ public:
 public:
     Config() : j_ {beast::Journal::getNullSink()} {}
 
-    static
-    int
-    getSize(SizedItemName item, std::uint32_t nodeSize)
-    {
-        assert(item < sizedItems.size() && nodeSize < sizedItems[item].size());
-        return sizedItems[item][nodeSize];
-    }
-
-    int
-    getSize(SizedItemName item) const
-    {
-        assert(item < sizedItems.size());
-        return getSize(item, NODE_SIZE);
-    }
-
     /* Be very careful to make sure these bool params
         are in the right order. */
     void setup (std::string const& strConf, bool bQuiet,
@@ -236,6 +197,27 @@ public:
     bool standalone() const { return RUN_STANDALONE; }
 
     bool canSign() const { return signingEnabled_; }
+
+    /** Retrieve the default value for the item at the specified node size
+
+        @param item The item for which the default value is needed
+        @param node Optional value, used to adjust the result to match the
+                    size of a node (0: tiny, ..., 4: huge). If unseated,
+                    uses the configured size (NODE_SIZE).
+
+        @throw This method can throw std::out_of_range if you ask for values
+               that it does not recognize or request a non-default node-size.
+
+        @return The value for the requested item.
+
+        @note The defaults are selected so as to be reasonable, but the node
+              size is an imprecise metric that combines multiple aspects of
+              the underlying system; this means that we can't provide optimal
+              defaults in the code for every case.
+    */
+    int
+    getValueFor(SizedItem item,
+        boost::optional<std::size_t> node = boost::none) const;
 };
 
 } // ripple

src/ripple/core/impl/Config.cpp

@@ -32,12 +32,61 @@
 #include <boost/format.hpp>
 #include <boost/regex.hpp>
 #include <boost/system/error_code.hpp>
+#include <algorithm>
 #include <fstream>
 #include <iostream>
 #include <iterator>
 
 namespace ripple {
 
+inline constexpr
+std::array<std::pair<SizedItem, std::array<int, 5>>, 11>
+sizedItems
+{{
+    // FIXME: We should document each of these items, explaining exactly what
+    //        they control and whether there exists an explicit config option
+    //        that can be used to override the default.
+    { SizedItem::sweepInterval,
+        {{      10,      30,       60,       90,      120  }} },
+    { SizedItem::treeCacheSize,
+        {{  128000,  256000,   512000,   768000,  2048000  }} },
+    { SizedItem::treeCacheAge,
+        {{      30,      60,       90,      120,      900  }} },
+    { SizedItem::ledgerSize,
+        {{      32,     128,      256,      384,      768  }} },
+    { SizedItem::ledgerAge,
+        {{      30,      90,      180,      240,      900  }} },
+    { SizedItem::ledgerFetch,
+        {{       2,       3,        4,        5,        8  }} },
+    { SizedItem::nodeCacheSize,
+        {{   16384,   32768,   131072,   262144,   524288  }} },
+    { SizedItem::nodeCacheAge,
+        {{      60,      90,      120,      900,     1800  }} },
+    { SizedItem::hashNodeDBCache,
+        {{       4,      12,       24,       64,      128  }} },
+    { SizedItem::txnDBCache,
+        {{       4,      12,       24,       64,      128  }} },
+    { SizedItem::lgrDBCache,
+        {{       4,       8,       16,       32,      128  }} },
+}};
+
+// Ensure that the order of entries in the table corresponds to the
+// order of entries in the enum:
+static_assert([]() constexpr -> bool
+{
+    std::underlying_type_t<SizedItem> idx = 0;
+
+    for (auto const& i : sizedItems)
+    {
+        if (static_cast<std::underlying_type_t<SizedItem>>(i.first) != idx)
+            return false;
+
+        ++idx;
+    }
+
+    return true;
+}(), "Mismatch between sized item enum & array indices");
+
 //
 // TODO: Check permissions on config file before using it.
 //
@@ -329,14 +378,8 @@ void Config::loadFromString (std::string const& fileContents)
         else if (boost::iequals(strTemp, "huge"))
             NODE_SIZE = 4;
         else
-        {
+            NODE_SIZE = std::min<std::size_t>(4,
-            NODE_SIZE = beast::lexicalCastThrow <int> (strTemp);
+                beast::lexicalCastThrow<std::size_t>(strTemp));
-
-            if (NODE_SIZE < 0)
-                NODE_SIZE = 0;
-            else if (NODE_SIZE > 4)
-                NODE_SIZE = 4;
-        }
     }
 
     if (getSingleSection (secConfig, SECTION_SIGNING_SUPPORT, strTemp, j_))
@@ -594,4 +637,13 @@ boost::filesystem::path Config::getDebugLogFile () const
     return log_file;
 }
 
+int
+Config::getValueFor(SizedItem item, boost::optional<std::size_t> node) const
+{
+    auto const index = static_cast<std::underlying_type_t<SizedItem>>(item);
+    assert(index < sizedItems.size());
+    assert(!node || *node <= 4);
+    return sizedItems.at(index).second.at(node.value_or(NODE_SIZE));
+}
+
 } // ripple

src/ripple/nodestore/impl/Shard.cpp

@@ -422,24 +422,25 @@ Shard::setCache(std::lock_guard<std::mutex> const&)
     if (!pCache_)
     {
         auto const name {"shard " + std::to_string(index_)};
-        auto const sz {complete_ ?
+        auto const sz = app_.config().getValueFor(SizedItem::nodeCacheSize,
-            Config::getSize(siNodeCacheSize, 0) :
+            complete_ ? boost::optional<std::size_t>(0) : boost::none);
-            app_.config().getSize(siNodeCacheSize)};
+        auto const age = std::chrono::seconds{
-        auto const age {std::chrono::seconds{complete_ ?
+            app_.config().getValueFor(SizedItem::nodeCacheAge,
-            Config::getSize(siNodeCacheAge, 0) :
+                complete_ ? boost::optional<std::size_t>(0) : boost::none)};
-            app_.config().getSize(siNodeCacheAge)}};
 
         pCache_ = std::make_shared<PCache>(name, sz, age, stopwatch(), j_);
         nCache_ = std::make_shared<NCache>(name, stopwatch(), sz, age);
     }
     else
     {
-        auto const sz {Config::getSize(siNodeCacheSize, 0)};
+        auto const sz = app_.config().getValueFor(
+            SizedItem::nodeCacheSize, 0);
         pCache_->setTargetSize(sz);
         nCache_->setTargetSize(sz);
 
-        auto const age {std::chrono::seconds{
+        auto const age = std::chrono::seconds{
-            Config::getSize(siNodeCacheAge, 0)}};
+            app_.config().getValueFor(
+                SizedItem::nodeCacheAge, 0)};
         pCache_->setTargetAge(age);
         nCache_->setTargetAge(age);
     }
@@ -496,7 +497,7 @@ Shard::initSQLite(std::lock_guard<std::mutex> const&)
                 LgrDBInit);
             lgrSQLiteDB_->getSession() <<
                 boost::str(boost::format("PRAGMA cache_size=-%d;") %
-                kilobytes(Config::getSize(siLgrDBCache, 0)));
+                kilobytes(config.getValueFor(SizedItem::lgrDBCache, boost::none)));
 
             txSQLiteDB_ = std::make_unique <DatabaseCon>(
                 setup,
@@ -505,7 +506,7 @@ Shard::initSQLite(std::lock_guard<std::mutex> const&)
                 TxDBInit);
             txSQLiteDB_->getSession() <<
                 boost::str(boost::format("PRAGMA cache_size=-%d;") %
-                kilobytes(Config::getSize(siTxnDBCache, 0)));
+                kilobytes(config.getValueFor(SizedItem::txnDBCache, boost::none)));
         }
         else
         {
@@ -517,7 +518,7 @@ Shard::initSQLite(std::lock_guard<std::mutex> const&)
                 LgrDBInit);
             lgrSQLiteDB_->getSession() <<
                 boost::str(boost::format("PRAGMA cache_size=-%d;") %
-                kilobytes(config.getSize(siLgrDBCache)));
+                kilobytes(config.getValueFor(SizedItem::lgrDBCache)));
             lgrSQLiteDB_->setupCheckpointing(&app_.getJobQueue(), app_.logs());
 
             txSQLiteDB_ = std::make_unique <DatabaseCon>(
@@ -527,7 +528,7 @@ Shard::initSQLite(std::lock_guard<std::mutex> const&)
                 TxDBInit);
             txSQLiteDB_->getSession() <<
                 boost::str(boost::format("PRAGMA cache_size=-%d;") %
-                kilobytes(config.getSize(siTxnDBCache)));
+                kilobytes(config.getValueFor(SizedItem::txnDBCache)));
             txSQLiteDB_->setupCheckpointing(&app_.getJobQueue(), app_.logs());
         }
     }