[FOLD] Address feedback

Builds/VisualStudio2015/RippleD.vcxproj

@@ -2253,8 +2253,6 @@
     </ClCompile>
     <ClInclude Include="..\..\src\ripple\nodestore\Database.h">
     </ClInclude>
-    <ClInclude Include="..\..\src\ripple\nodestore\DatabaseNode.h">
-    </ClInclude>
     <ClInclude Include="..\..\src\ripple\nodestore\DatabaseRotating.h">
     </ClInclude>
     <ClInclude Include="..\..\src\ripple\nodestore\DatabaseShard.h">

Builds/VisualStudio2015/RippleD.vcxproj.filters

@@ -2877,9 +2877,6 @@
     <ClInclude Include="..\..\src\ripple\nodestore\Database.h">
       <Filter>ripple\nodestore</Filter>
     </ClInclude>
-    <ClInclude Include="..\..\src\ripple\nodestore\DatabaseNode.h">
-      <Filter>ripple\nodestore</Filter>
-    </ClInclude>
     <ClInclude Include="..\..\src\ripple\nodestore\DatabaseRotating.h">
       <Filter>ripple\nodestore</Filter>
     </ClInclude>

CMakeLists.txt

@@ -491,8 +491,8 @@ if(${B2_EXE} STREQUAL "B2_EXE-NOTFOUND")
     "Boost b2 executable not found. docs target will not be buildable")
 elseif(NOT BOOST_ROOT)
     if(Boost_INCLUDE_DIRS)
-    else()
         set(BOOST_ROOT ${Boost_INCLUDE_DIRS})
+    else()
         get_filename_component(BOOST_ROOT ${B2_EXE} DIRECTORY)
     endif()
 endif()

src/ripple/app/ledger/LedgerMaster.h

@@ -243,11 +243,6 @@ class LedgerMaster
 
     std::size_t getFetchPackCacheSize () const;
 
-    void fetchForHistory(
-        std::uint32_t missing,
-        bool& progress,
-        InboundLedger::Reason reason);
-
 private:
     using ScopedLockType = std::lock_guard <std::recursive_mutex>;
     using ScopedUnlockType = GenericScopedUnlock <std::recursive_mutex>;
@@ -269,10 +264,13 @@ class LedgerMaster
 
     std::size_t getNeededValidations();
     void advanceThread();
+    void fetchForHistory(
+        std::uint32_t missing,
+        bool& progress,
+        InboundLedger::Reason reason);
     // Try to publish ledgers, acquire missing ledgers.  Always called with
     // m_mutex locked.  The passed ScopedLockType is a reminder to callers.
     void doAdvance(ScopedLockType&);
-    bool shouldFetchPack(std::uint32_t seq) const;
     bool shouldAcquire(
         std::uint32_t const currentLedger,
         std::uint32_t const ledgerHistory,

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

@@ -280,52 +280,48 @@ InboundLedger::tryDB(Family& f)
 {
     if (! mHaveHeader)
     {
+        auto makeLedger = [&, this](Blob const& data)
+            {
+                JLOG(m_journal.trace()) <<
+                    "Ledger header found in fetch pack";
+                mLedger = std::make_shared<Ledger>(
+                    deserializeHeader(makeSlice(data), true),
+                        app_.config(), f);
+                if (mLedger->info().hash != mHash ||
+                    (mSeq != 0 && mSeq != mLedger->info().seq))
+                {
+                    // We know for a fact the ledger can never be acquired
+                    JLOG(m_journal.warn()) <<
+                        "hash " << mHash <<
+                        " seq " << std::to_string(mSeq) <<
+                        " cannot be a ledger";
+                    mLedger.reset();
+                    mFailed = true;
+                }
+            };
+
         // Try to fetch the ledger header from the DB
         auto node = f.db().fetch(mHash, mSeq);
         if (! node)
         {
             auto data = app_.getLedgerMaster().getFetchPack(mHash);
             if (! data)
                 return;
-
             JLOG (m_journal.trace()) <<
                 "Ledger header found in fetch pack";
-            mLedger = std::make_shared<Ledger>(
-                deserializeHeader(makeSlice(*data), true),
-                    app_.config(), f);
-            if (mLedger->info().hash != mHash ||
-                (mSeq != 0 && mSeq != mLedger->info().seq))
-            {
-                // We know for a fact the ledger can never be acquired
-                JLOG(m_journal.warn()) <<
-                    "hash " << mHash <<
-                    " seq " << std::to_string(mSeq) <<
-                    " cannot be a ledger";
-                mFailed = true;
-                return;
-            }
-            f.db().store(hotLEDGER, std::move(*data),
-                mHash, mLedger->info().seq);
+            makeLedger(*data);
+            if (mLedger)
+                f.db().store(hotLEDGER, std::move(*data),
+                    mHash, mLedger->info().seq);
         }
         else
         {
             JLOG (m_journal.trace()) <<
                 "Ledger header found in node store";
-            mLedger = std::make_shared<Ledger>(
-                deserializeHeader(makeSlice(node->getData()), true),
-                    app_.config(), f);
-            if (mLedger->info().hash != mHash ||
-                (mSeq != 0 && mSeq != mLedger->info().seq))
-            {
-                // We know for a fact the ledger can never be acquired
-                JLOG(m_journal.warn()) <<
-                    "hash " << mHash <<
-                    " seq " << std::to_string(mSeq) <<
-                    " cannot be a ledger";
-                mFailed = true;
-                return;
-            }
+            makeLedger(node->getData());
         }
+        if (mFailed)
+            return;
         if (mSeq == 0)
             mSeq = mLedger->info().seq;
         mLedger->stateMap().setLedgerSeq(mSeq);

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

@@ -946,12 +946,6 @@ LedgerMaster::getLedgerHashForHistory(
     return ret;
 }
 
-bool
-LedgerMaster::shouldFetchPack (std::uint32_t seq) const
-{
-    return (fetch_seq_ != seq);
-}
-
 std::vector<std::shared_ptr<Ledger const>>
 LedgerMaster::findNewLedgersToPublish ()
 {
@@ -1539,7 +1533,9 @@ LedgerMaster::fetchForHistory(
             {
                 ledger = app_.getInboundLedgers().acquire(
                     *hash, missing, reason);
-                if (!ledger && (missing > 32600) && missing != fetch_seq_)
+                if (!ledger &&
+                    missing > NodeStore::genesisSeq &&
+                    missing != fetch_seq_)
                 {
                     JLOG(m_journal.trace())
                         << "fetchForHistory want fetch pack " << missing;
@@ -1597,14 +1593,16 @@ LedgerMaster::fetchForHistory(
         {
             std::uint32_t fetchSz;
             if (reason == InboundLedger::Reason::SHARD)
-            {
-                auto firstSeq {NodeStore::DatabaseShard::firstSeq(
-                    NodeStore::DatabaseShard::seqToShardIndex(missing))};
-                fetchSz = (missing >= firstSeq ? std::min(
-                    ledger_fetch_size_, (missing - firstSeq) + 1) : 0);
-            }
+                // Do not fetch ledger sequences lower
+                // than the shard's first ledger sequence
+                fetchSz = NodeStore::DatabaseShard::firstSeq(
+                    NodeStore::DatabaseShard::seqToShardIndex(missing));
             else
-                fetchSz = ledger_fetch_size_;
+                // Do not fetch ledger sequences lower
+                // than the genesis ledger sequence
+                fetchSz = NodeStore::genesisSeq;
+            fetchSz = missing >= fetchSz ?
+                std::min(ledger_fetch_size_, (missing - fetchSz) + 1) : 0;
             try
             {
                 for (std::uint32_t i = 0; i < fetchSz; ++i)
@@ -1660,7 +1658,7 @@ void LedgerMaster::doAdvance (ScopedLockType& sl)
                 {
                     ScopedLockType sl(mCompleteLock);
                     missing = prevMissing(mCompleteLedgers,
-                        mPubLedger->info().seq, std::uint32_t(32600));
+                        mPubLedger->info().seq, NodeStore::genesisSeq);
                 }
                 if (missing)
                 {

src/ripple/app/misc/SHAMapStoreImp.cpp

@@ -566,8 +566,10 @@ SHAMapStoreImp::makeBackendRotating (std::string path)
     }
     parameters.set("path", newPath.string());
 
-    return NodeStore::Manager::instance().make_Backend (parameters, scheduler_,
-            nodeStoreJournal_);
+    auto backend {NodeStore::Manager::instance().make_Backend(
+        parameters, scheduler_, nodeStoreJournal_)};
+    backend->open();
+    return backend;
 }
 
 bool

src/ripple/nodestore/Backend.h

@@ -50,6 +50,11 @@ class Backend
     */
     virtual std::string getName() = 0;
 
+    /** Open the backend.
+        This allows the caller to catch exceptions.
+    */
+    virtual void open() = 0;
+
     /** Close the backend.
         This allows the caller to catch exceptions.
     */

src/ripple/nodestore/Database.h

@@ -80,17 +80,6 @@ class Database : public Stoppable
     std::string
     getName() const = 0;
 
-    /** Visit every object in the database
-        This is usually called during import.
-
-        @note This routine will not be called concurrently with itself
-                or other methods.
-        @see import
-    */
-    virtual
-    void
-    for_each(std::function <void(std::shared_ptr<NodeObject>)> f) = 0;
-
     /** Import objects from another database. */
     virtual
     void
@@ -108,7 +97,6 @@ class Database : public Stoppable
         The caller's Blob parameter is overwritten.
 
         @param type The type of object.
-        @param ledgerIndex The ledger in which the object appears.
         @param data The payload of the object. The caller's
                     variable is overwritten.
         @param hash The 256-bit hash of the payload data.
@@ -168,8 +156,9 @@ class Database : public Stoppable
 
     /** Get the maximum number of async reads the node store prefers.
 
-        @param seq The sequence of the ledger the object belongs to.
+        @param seq A ledger sequence specifying a shard to query.
         @return The number of async reads preferred.
+        @note The sequence is only used with the shard store.
     */
     virtual
     int
@@ -273,6 +262,17 @@ class Database : public Stoppable
     std::shared_ptr<NodeObject>
     fetchFrom(uint256 const& hash, std::uint32_t seq) = 0;
 
+    /** Visit every object in the database
+        This is usually called during import.
+
+        @note This routine will not be called concurrently with itself
+                or other methods.
+        @see import
+    */
+    virtual
+    void
+    for_each(std::function <void(std::shared_ptr<NodeObject>)> f) = 0;
+
     void
     threadEntry();
 };

src/ripple/nodestore/DatabaseShard.h

@@ -102,7 +102,7 @@ class DatabaseShard : public Database
     */
     virtual
     bool
-    hasLedger(std::uint32_t seq) = 0;
+    contains(std::uint32_t seq) = 0;
 
     /** Query which complete shards are stored
 
@@ -138,6 +138,7 @@ class DatabaseShard : public Database
     std::uint32_t
     seqToShardIndex(std::uint32_t seq)
     {
+        assert(seq >= genesisSeq);
         return (seq - 1) / lps_;
     }
 

src/ripple/nodestore/Types.h

@@ -49,6 +49,9 @@ enum Status
 /** A batch of NodeObjects to write at once. */
 using Batch = std::vector <std::shared_ptr<NodeObject>>;
 
+// System constant/invariant
+static constexpr std::uint32_t genesisSeq {32570u};
+
 }
 }
 

src/ripple/nodestore/backend/MemoryFactory.cpp

@@ -80,7 +80,7 @@ class MemoryBackend : public Backend
 
     std::string name_;
     beast::Journal journal_;
-    MemoryDB* db_;
+    MemoryDB* db_ {nullptr};
 
 public:
     MemoryBackend (size_t keyBytes, Section const& keyValues,
@@ -90,7 +90,6 @@ class MemoryBackend : public Backend
     {
         if (name_.empty())
             Throw<std::runtime_error> ("Missing path in Memory backend");
-        db_ = &memoryFactory.open(name_);
     }
 
     ~MemoryBackend ()
@@ -104,6 +103,12 @@ class MemoryBackend : public Backend
         return name_;
     }
 
+    void
+    open() override
+    {
+        db_ = &memoryFactory.open(name_);
+    }
+
     void
     close() override
     {
@@ -115,6 +120,7 @@ class MemoryBackend : public Backend
     Status
     fetch (void const* key, std::shared_ptr<NodeObject>* pObject) override
     {
+        assert(db_);
         uint256 const hash (uint256::fromVoid (key));
 
         std::lock_guard<std::mutex> _(db_->mutex);
@@ -145,6 +151,7 @@ class MemoryBackend : public Backend
     void
     store (std::shared_ptr<NodeObject> const& object) override
     {
+        assert(db_);
         std::lock_guard<std::mutex> _(db_->mutex);
         db_->table.emplace (object->getHash(), object);
     }
@@ -159,6 +166,7 @@ class MemoryBackend : public Backend
     void
     for_each (std::function <void(std::shared_ptr<NodeObject>)> f) override
     {
+        assert(db_);
         for (auto const& e : db_->table)
             f (e.second);
     }