[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/AccountStateSF.cpp

@@ -31,8 +31,7 @@ AccountStateSF::gotNode(bool fromFilter, SHAMapHash const& nodeHash,
 }
 
 boost::optional<Blob>
-AccountStateSF::getNode(SHAMapHash const& nodeHash,
-    std::uint32_t ledgerSeq) const
+AccountStateSF::getNode(SHAMapHash const& nodeHash, std::uint32_t) const
 {
     return fp_.getFetchPack(nodeHash.as_uint256());
 }

src/ripple/app/ledger/ConsensusTransSetSF.cpp

@@ -40,7 +40,7 @@ ConsensusTransSetSF::ConsensusTransSetSF (Application& app, NodeCache& nodeCache
 
 void
 ConsensusTransSetSF::gotNode(bool fromFilter, SHAMapHash const& nodeHash,
-    std::uint32_t ledgerSeq, Blob&& nodeData, SHAMapTreeNode::TNType type) const
+    std::uint32_t, Blob&& nodeData, SHAMapTreeNode::TNType type) const
 {
     if (fromFilter)
         return;
@@ -76,8 +76,7 @@ ConsensusTransSetSF::gotNode(bool fromFilter, SHAMapHash const& nodeHash,
 }
 
 boost::optional<Blob>
-ConsensusTransSetSF::getNode (SHAMapHash const& nodeHash,
-    std::uint32_t ledgerSeq) const
+ConsensusTransSetSF::getNode (SHAMapHash const& nodeHash, std::uint32_t) const
 {
     Blob nodeData;
     if (m_nodeCache.retrieve (nodeHash, nodeData))

src/ripple/app/ledger/LedgerMaster.h

@@ -181,7 +181,7 @@ class LedgerMaster
         LedgerIndex ledgerIndex);
 
     boost::optional <NetClock::time_point> getCloseTimeByHash (
-        LedgerHash const& ledgerHash, std::uint32_t index);
+        LedgerHash const& ledgerHash, LedgerIndex ledgerIndex);
 
     void addHeldTransaction (std::shared_ptr<Transaction> const& trans);
     void fixMismatch (ReadView const& ledger);
@@ -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

@@ -111,7 +111,7 @@ InboundLedger::init(ScopedLockType& collectionLock)
             if (mFailed)
                 return;
         }
-        else if (shardStore)
+        else if (shardStore && mSeq >= NodeStore::genesisSeq)
         {
             if (auto l = shardStore->fetchLedger(mHash, mSeq))
             {
@@ -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);
@@ -480,7 +476,7 @@ void InboundLedger::done ()
         {
         case Reason::SHARD:
             app_.getShardStore()->setStored(mLedger);
-            // fall through
+            // TODO c++17: [[fallthrough]]
         case Reason::HISTORY:
             app_.getInboundLedgers().onLedgerFetched();
             break;

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

@@ -448,11 +448,9 @@ SHAMapStoreImp::run()
                     ;
             }
 
-            std::shared_ptr <NodeStore::Backend> newBackend =
-                    makeBackendRotating();
+            auto newBackend = makeBackendRotating();
             JLOG(journal_.debug()) << validatedSeq << " new backend "
                     << newBackend->getName();
-            std::shared_ptr <NodeStore::Backend> oldBackend;
 
             clearCaches (validatedSeq);
             switch (health())
@@ -470,13 +468,15 @@ SHAMapStoreImp::run()
             std::string nextArchiveDir =
                 dbRotating_->getWritableBackend()->getName();
             lastRotated = validatedSeq;
+            std::unique_ptr<NodeStore::Backend> oldBackend;
             {
                 std::lock_guard <std::mutex> lock (dbRotating_->peekMutex());
 
                 state_db_.setState (SavedState {newBackend->getName(),
                         nextArchiveDir, lastRotated});
                 clearCaches (validatedSeq);
-                oldBackend = dbRotating_->rotateBackends (newBackend);
+                oldBackend = std::move(
+                    dbRotating_->rotateBackends(std::move(newBackend)));
             }
             JLOG(journal_.debug()) << "finished rotation " << validatedSeq;
 
@@ -547,7 +547,7 @@ SHAMapStoreImp::dbPaths()
     }
 }
 
-std::shared_ptr <NodeStore::Backend>
+std::unique_ptr <NodeStore::Backend>
 SHAMapStoreImp::makeBackendRotating (std::string path)
 {
     boost::filesystem::path newPath;
@@ -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/app/misc/SHAMapStoreImp.h

@@ -179,8 +179,9 @@ class SHAMapStoreImp : public SHAMapStore
     bool copyNode (std::uint64_t& nodeCount, SHAMapAbstractNode const &node);
     void run();
     void dbPaths();
-    std::shared_ptr <NodeStore::Backend> makeBackendRotating (
-            std::string path = std::string());
+
+    std::unique_ptr<NodeStore::Backend>
+    makeBackendRotating (std::string path = std::string());
 
     template <class CacheInstance>
     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

@@ -1,7 +1,7 @@
 //------------------------------------------------------------------------------
 /*
     This file is part of rippled: https://github.com/ripple/rippled
-    Copyright (c) 2012, 2013 Ripple Labs Inc.
+    Copyright (c) 2012, 2017 Ripple Labs Inc.
 
     Permission to use, copy, modify, and/or distribute this software for any
     purpose  with  or without fee is hereby granted, provided that the above
@@ -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();
 };