From 76b36fb308bc25846dac26c97d9590a3cad72a9a Mon Sep 17 00:00:00 2001 From: Nicholas Dudfield Date: Tue, 19 Aug 2025 16:03:11 +0700 Subject: [PATCH] feat: enforce online_delete requirement for RWDB to prevent OOM RWDB (in-memory backend) now requires online_delete configuration to prevent unbounded memory growth. Exception: standalone mode (used by tests) bypasses this requirement since tests run for short durations. - Add enforcement check in SHAMapStoreImp constructor - Tests use Config::setupControl() to simulate non-standalone environment - Comprehensive test coverage for all enforcement scenarios --- src/ripple/app/misc/SHAMapStoreImp.cpp | 8 +--- src/test/app/SHAMapStore_test.cpp | 53 +++++++++++++++----------- src/test/jtx/impl/envconfig.cpp | 4 -- 3 files changed, 32 insertions(+), 33 deletions(-) diff --git a/src/ripple/app/misc/SHAMapStoreImp.cpp b/src/ripple/app/misc/SHAMapStoreImp.cpp index a25c401d4..829bb757b 100644 --- a/src/ripple/app/misc/SHAMapStoreImp.cpp +++ b/src/ripple/app/misc/SHAMapStoreImp.cpp @@ -118,16 +118,10 @@ SHAMapStoreImp::SHAMapStoreImp( get_if_exists(section, "online_delete", deleteInterval_); - auto actual_standalone = config.standalone(); - auto effective_standalone = - get(section, - "_online_delete_standalone_override", - actual_standalone ? "true" : "false") != "false"; - // RWDB (in-memory backend) requires online_delete to prevent OOM // Exception: standalone mode (used by tests) doesn't need online_delete // since tests run for short durations - if (config.mem_backend() && !deleteInterval_ && !effective_standalone) + if (config.mem_backend() && !deleteInterval_ && !config.standalone()) { Throw( "RWDB (in-memory backend) requires online_delete to be configured. " diff --git a/src/test/app/SHAMapStore_test.cpp b/src/test/app/SHAMapStore_test.cpp index 55b7184be..c90cbf15e 100644 --- a/src/test/app/SHAMapStore_test.cpp +++ b/src/test/app/SHAMapStore_test.cpp @@ -522,40 +522,39 @@ public: } static std::unique_ptr - rwdbNoDelete() + rwdbNoDeleteDefaultConfig() { - // RWDB without online_delete, but keep standalone override = true - // (default) + // RWDB without online_delete, in standalone mode (default) auto cfg = test::jtx::envconfig(); cfg->section(ConfigSection::nodeDatabase()).set("type", "rwdb"); cfg->section(ConfigSection::nodeDatabase()).set("path", "main"); - // Keep the default standalone override value of "true" from - // envconfig.cpp + // Default is standalone = true from envconfig.cpp return cfg; } static std::unique_ptr - rwdbNoDeleteEnforced() + rwdbNoDeleteNotStandalone() { - // RWDB without online_delete and force enforcement + // RWDB without online_delete and NOT in standalone mode + // This should trigger the enforcement auto cfg = test::jtx::envconfig(); cfg->section(ConfigSection::nodeDatabase()).set("type", "rwdb"); cfg->section(ConfigSection::nodeDatabase()).set("path", "main"); - cfg->section(ConfigSection::nodeDatabase()) - .set("_online_delete_standalone_override", "false"); + cfg->setupControl( + true, true, false); // bQuiet, bSilent, bStandalone=false return cfg; } static std::unique_ptr - rwdbWithDeleteEnforced() + rwdbWithDeleteNotStandalone() { - // RWDB with online_delete and force enforcement + // RWDB with online_delete and NOT in standalone mode auto cfg = test::jtx::envconfig(); cfg->section(ConfigSection::nodeDatabase()).set("type", "rwdb"); cfg->section(ConfigSection::nodeDatabase()).set("path", "main"); cfg->section(ConfigSection::nodeDatabase()).set("online_delete", "256"); - cfg->section(ConfigSection::nodeDatabase()) - .set("_online_delete_standalone_override", "false"); + cfg->setupControl( + true, true, false); // bQuiet, bSilent, bStandalone=false return cfg; } @@ -564,18 +563,18 @@ public: { testcase("RWDB online_delete enforcement"); - // Test 1: RWDB without online_delete but with standalone override = - // true (should succeed) + // Test 1: RWDB without online_delete in standalone mode (should + // succeed) { - test::jtx::Env env{*this, rwdbNoDelete()}; + jtx::Env env{*this, rwdbNoDeleteDefaultConfig()}; pass(); } - // Test 2: RWDB without online_delete and standalone override = false - // (should throw) + // Test 2: RWDB without online_delete NOT in standalone mode (should + // throw) try { - test::jtx::Env env{*this, rwdbNoDeleteEnforced()}; + jtx::Env env{*this, rwdbNoDeleteNotStandalone()}; fail("Expected exception for RWDB without online_delete"); } catch (std::runtime_error const& e) @@ -587,10 +586,20 @@ public: pass(); } - // Test 3: RWDB with online_delete and standalone override = false - // (should succeed) + // Test 3: RWDB with online_delete NOT in standalone mode (should + // succeed) But should throw with another error message unrelated + try { - test::jtx::Env env{*this, rwdbWithDeleteEnforced()}; + jtx::Env env{*this, rwdbWithDeleteNotStandalone()}; + // Currently throwing due to missing database_path + fail("Test relies upon throwing exception"); + } + catch (std::runtime_error const& e) + { + BEAST_EXPECT( + std::string(e.what()).find( + "RWDB (in-memory backend) requires online_delete") == + std::string::npos); pass(); } } diff --git a/src/test/jtx/impl/envconfig.cpp b/src/test/jtx/impl/envconfig.cpp index f40be5878..ce2c2ae60 100644 --- a/src/test/jtx/impl/envconfig.cpp +++ b/src/test/jtx/impl/envconfig.cpp @@ -51,10 +51,6 @@ setupConfigForUnitTests(Config& cfg) cfg.overwrite(ConfigSection::nodeDatabase(), "type", "rwdb"); cfg.overwrite(ConfigSection::nodeDatabase(), "path", "main"); - cfg.overwrite( - ConfigSection::nodeDatabase(), - "_online_delete_standalone_override", - "true"); cfg.overwrite(SECTION_RELATIONAL_DB, "backend", "rwdb"); cfg.deprecatedClearSection(ConfigSection::importNodeDatabase()); cfg.legacy("database_path", "");