allow rwdb to co-exist with pinned node_db

src/test/app/SHAMapStore_test.cpp

@@ -76,6 +76,10 @@ class SHAMapStore_test : public beast::unit_test::suite
         return cfg;
     }
 
+    // DatabasePinned with the in-memory RWDB backend as the rotating hot
+    // store and NuDB as the pinned store, over a persistent SQLite
+    // relational database and a file-backed state.db. This is the
+    // production shape for "rwdb + pinned_type".
     static auto
     pinnedPersistent(
         std::unique_ptr<Config> cfg,
@@ -96,8 +100,9 @@ class SHAMapStore_test : public beast::unit_test::suite
     }
 
     // Online delete with DatabasePinned over in-memory backends. Unlike
-    // pinnedPersistent, the hot store keeps nodes (type=rwdb is the null
-    // node store), so ledgers closed in the test can be fetched. There is
+    // pinnedPersistent, the hot store is the process-shared "memory"
+    // backend and the pinned store is in-memory too, so ledgers closed in
+    // the test can be fetched without touching disk. There is
     // no database_path, so every SHAMapStoreImp opens its own private
     // state.db: a second instance built by a test does not see (or trip
     // over) the rotation state of the Env's own.
@@ -936,15 +941,22 @@ public:
         Config upperCfg;
         Config nudbCfg;
         Config emptyCfg;
+        Config pinnedCfg;
         rwdbCfg.section(ConfigSection::nodeDatabase()).set("type", "rwdb");
         upperCfg.section(ConfigSection::nodeDatabase()).set("type", "RWDB");
         nudbCfg.section(ConfigSection::nodeDatabase()).set("type", "NuDB");
+        pinnedCfg.section(ConfigSection::nodeDatabase()).set("type", "rwdb");
+        pinnedCfg.section(ConfigSection::nodeDatabase())
+            .set("pinned_type", "NuDB");
 
         // Evaluated per Config object, not latched process-wide.
         BEAST_EXPECT(rwdbCfg.nullBackend());
         BEAST_EXPECT(upperCfg.nullBackend());
         BEAST_EXPECT(!nudbCfg.nullBackend());
         BEAST_EXPECT(!emptyCfg.nullBackend());
+        // pinned_type turns type=rwdb into the rotating hot store under
+        // DatabasePinned, not the null node store.
+        BEAST_EXPECT(!pinnedCfg.nullBackend());
         BEAST_EXPECT(rwdbCfg.nullBackend());
     }
 

src/test/core/Config_test.cpp

@@ -1707,9 +1707,10 @@ r.ripple.com:51235
             }
         }
 
-        // Test 5: type=rwdb with pinned_type (should throw, even in
-        // standalone). The rwdb node store is tree-only, so pinned data
-        // would be accepted and then dropped.
+        // Test 5: type=rwdb with pinned_type (should succeed in both
+        // modes). pinned_type opts the rwdb store out of null mode: it
+        // becomes the in-memory rotating hot store under DatabasePinned,
+        // with no [node_db] path required.
         for (bool const standalone : {false, true})
         {
             Config c;
@@ -1723,13 +1724,14 @@ r.ripple.com:51235
             try
             {
                 c.loadFromString(toLoad);
-                fail("Expected exception for pinned_type with type=rwdb");
+                BEAST_EXPECT(!c.nullBackend());
             }
             catch (std::runtime_error const& e)
             {
-                BEAST_EXPECT(
-                    std::string(e.what()).find("type=rwdb") !=
-                    std::string::npos);
+                fail(
+                    std::string("Should not throw for pinned_type with "
+                                "type=rwdb: ") +
+                    e.what());
             }
         }
 

src/test/nodestore/DatabasePinned_test.cpp

@@ -34,9 +34,9 @@ namespace test {
 class DatabasePinned_test : public beast::unit_test::suite
 {
     // Build an Env whose nodestore is wired up as DatabasePinnedImp.
-    // The rotating store must persist (type=memory). type=rwdb is
-    // NullFactory and never builds DatabasePinnedImp. pinned_type=rwdb
-    // is allowed in standalone mode.
+    // The rotating store is the process-shared "memory" backend so the
+    // hot side keeps nodes across the test. pinned_type=rwdb is allowed
+    // in standalone mode.
     static std::unique_ptr<Config>
     pinnedEnvconfig(std::string const& nodePath)
     {

src/test/rpc/Catalogue_test.cpp

@@ -110,10 +110,10 @@ struct CatalogueEnvOpts
 // envconfig with pinned_type set so catalogue_load passes the config guard.
 //
 // envconfig() selects [node_db] type=rwdb, which is the tree-only null node
-// store: SHAMapStore never builds DatabasePinned on it, so catalogue_load
-// would refuse to run. Use the in-memory "memory" store for the hot side
-// instead. It is shared by path within the process, so each config gets its
-// own path (SHAMapStore creates the directory; nothing is written to it).
+// store unless pinned_type is also set. Use the in-memory "memory" store for
+// the hot side so the Env's node store is the same shape as a persisting
+// one. It is shared by path within the process, so each config gets its own
+// path (SHAMapStore creates the directory; nothing is written to it).
 // The pinned side uses rwdb, an in-memory persistent backend.
 inline std::unique_ptr<Config>
 catalogueEnvconfig(CatalogueEnvOpts const& opts = {})
@@ -1416,13 +1416,14 @@ public:
         using namespace test::jtx;
         using beast::severities::kDisabled;
 
-        // pinned_type is set, but [node_db] type=rwdb is the null node
-        // store, on which SHAMapStore never builds the pinned store.
+        // pinned_type is set, but online_delete is not, so SHAMapStore
+        // builds a plain node store rather than DatabasePinned. (Config
+        // validation rejects this combination from a config file; jtx
+        // sets sections directly and bypasses it.)
         {
             auto cfg = test::jtx::envconfig();
             auto& nodeDb = cfg->section(ConfigSection::nodeDatabase());
             nodeDb.set("pinned_type", "rwdb");
-            nodeDb.set("online_delete", "256");
             Env env{*this, std::move(cfg), features, nullptr, kDisabled};
 
             Json::Value params{Json::objectValue};

src/xrpld/app/misc/SHAMapStoreImp.cpp

@@ -207,7 +207,11 @@ SHAMapStoreImp::SHAMapStoreImp(
                 std::to_string(config.LEDGER_HISTORY) + ")");
         }
 
-        if (!isNullBackend_)
+        // Only on-disk rotating backends keep files under [node_db] path.
+        // The null node store has none, and neither does type=rwdb used
+        // as the rotating hot store under DatabasePinned (pinned_type
+        // set), which was never required to configure a path.
+        if (!isNullBackend_ && !boost::iequals(get(section, "type"), "rwdb"))
             dbPaths();
     }
 }
@@ -425,6 +429,16 @@ SHAMapStoreImp::loadPinnedRanges()
     // is not the one these ranges were loaded into.
     constexpr std::uint32_t maxTailTrim = 65536;
 
+    // [relational_db] backend=rwdb keeps the SQL tables in memory, so the
+    // seq -> hash index behind hasLedgerData() starts empty on every run
+    // even when the pinned node store still holds every node. An interval
+    // the index cannot resolve is unreachable rather than evidence of a
+    // mismatched pinned store: drop it and have the operator re-run
+    // catalogue_load (node writes de-duplicate; only SQL is rebuilt).
+    auto const& rdb = app_.config().section(SECTION_RELATIONAL_DB);
+    bool const sqlPersistent =
+        rdb.empty() || !boost::iequals(get(rdb, "backend"), "rwdb");
+
     RangeSet<std::uint32_t> verified;
     for (auto const& interval : persistedRanges)
     {
@@ -439,7 +453,18 @@ SHAMapStoreImp::loadPinnedRanges()
         };
 
         if (!hasLedgerData(lower))
-            Throw<std::runtime_error>(notPresent());
+        {
+            if (sqlPersistent)
+                Throw<std::runtime_error>(notPresent());
+
+            JLOG(journal_.warn())
+                << "Dropping pinned interval " << lower << "-"
+                << interval.upper()
+                << ": [relational_db] backend=rwdb does not persist the "
+                   "ledger index across restarts, so it cannot be served. "
+                   "Re-run catalogue_load to restore it.";
+            continue;
+        }
 
         std::uint32_t trimmed = 0;
         while (upper > lower && !hasLedgerData(upper))
@@ -464,7 +489,7 @@ SHAMapStoreImp::loadPinnedRanges()
     }
 
     if (verified != persistedRanges)
-        state_db_.setPinnedRanges(to_string(verified));
+        setPinnedRanges(verified);
 
     JLOG(journal_.info()) << "Loaded pinned ranges from database: "
                           << to_string(verified);

src/xrpld/core/Config.h

@@ -387,10 +387,14 @@ public:
         return isMem;
     }
 
-    /** True when [node_db] type=rwdb.
+    /** True when [node_db] type=rwdb and no pinned_type is configured.
 
         The app node store is the existing NullFactory (type=none).
         Nodes stay alive through Ledger and SHAMap pointers.
+
+        With pinned_type set, type=rwdb keeps its older meaning: the
+        in-memory RWDB backend is the rotating hot store under
+        DatabasePinnedImp and the durable pinned store holds history.
         The result is read from this configuration, so two
         applications in one process do not share it.
     */
@@ -399,7 +403,8 @@ public:
     {
         auto const& node = section(ConfigSection::nodeDatabase());
         return !node.empty() &&
-            boost::beast::iequals(get(node, "type"), "rwdb");
+            boost::beast::iequals(get(node, "type"), "rwdb") &&
+            !node.exists("pinned_type");
     }
 
     bool

src/xrpld/core/detail/Config.cpp

@@ -1087,15 +1087,10 @@ Config::loadFromString(std::string const& fileContents)
                 "type must be specified when using pinned_type");
         }
 
-        // type=rwdb is the tree-only null node store. SHAMapStore never
-        // builds DatabasePinned for it, so pinned (and catalogue) data
-        // would be accepted and then silently dropped.
-        if (nullBackend())
-        {
-            Throw<std::runtime_error>(
-                "pinned_type is not supported with [node_db] type=rwdb "
-                "(the rwdb node store does not persist nodes)");
-        }
+        // type=rwdb with pinned_type is not the null node store (see
+        // Config::nullBackend): SHAMapStore builds DatabasePinned with the
+        // in-memory RWDB backend as the rotating hot store, so pinned data
+        // lands in the durable pinned backend as with any other type.
 
         // Ensure pinned_path is specified for filesystem-backed types.
         // In-memory backends (rwdb) don't need a path.