ARCHIVE/xahaud
3
0
Fork 0
mirror of https://github.com/Xahau/xahaud.git synced 2025-12-06 17:27:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: move RWDB online_delete validation to Config::loadFromString

Browse Source 
Move validation from SHAMapStoreImp constructor to configuration loading phase
as indicated by GitHub review comment. This improves architecture by catching
configuration errors earlier in the initialization process.

- Remove duplicate validation from SHAMapStoreImp.cpp
- Keep validation only in Config::loadFromString()
- Move tests from SHAMapStore_test to Config_test
- Clean up test string formatting to use regular literals
This commit is contained in:
Nicholas Dudfield
2025-08-19 17:26:55 +07:00
parent 76b36fb308
commit 23c7cd25a7
4 changed files with 111 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
src/ripple/app/misc/SHAMapStoreImp.cpp
View File

@@ -118,19 +118,6 @@ SHAMapStoreImp::SHAMapStoreImp(
get_if_exists(section, "online_delete", deleteInterval_);
// 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_ && !config.standalone())
{
Throw<std::runtime_error>(
"RWDB (in-memory backend) requires online_delete to be configured. "
"Without online_delete, memory usage will grow unbounded until "
"OOM. "
"Add [node_db] with online_delete setting to enable automatic "
"cleanup.");
}
if (deleteInterval_)
{
if (app_.config().reporting())
@@ -748,4 +735,5 @@ make_SHAMapStore(
{
return std::make_unique<SHAMapStoreImp>(app, scheduler, journal);
}
} // namespace ripple
} // namespace ripple

25
src/ripple/core/impl/Config.cpp
View File

@@ -45,7 +45,6 @@
namespace ripple {
namespace detail {
[[nodiscard]] std::uint64_t
getMemorySize()
{
@@ -54,7 +53,6 @@ getMemorySize()
return 0;
}
} // namespace detail
} // namespace ripple
#endif
@@ -64,7 +62,6 @@ getMemorySize()
namespace ripple {
namespace detail {
[[nodiscard]] std::uint64_t
getMemorySize()
{
@@ -73,7 +70,6 @@ getMemorySize()
return 0;
}
} // namespace detail
} // namespace ripple
@@ -85,7 +81,6 @@ getMemorySize()
namespace ripple {
namespace detail {
[[nodiscard]] std::uint64_t
getMemorySize()
{
@@ -98,13 +93,11 @@ getMemorySize()
return 0;
}
} // namespace detail
} // namespace ripple
#endif
namespace ripple {
// clang-format off
// The configurable node sizes are "tiny", "small", "medium", "large", "huge"
inline constexpr std::array<std::pair<SizedItem, std::array<int, 5>>, 13>
@@ -1007,6 +1000,23 @@ Config::loadFromString(std::string const& fileContents)
"the maximum number of allowed peers (peers_max)");
}
}
if (!RUN_STANDALONE)
{
auto db_section = section(ConfigSection::nodeDatabase());
if (auto type = get(db_section, "type", ""); type == "rwdb")
{
if (auto delete_interval = get(db_section, "online_delete", 0);
delete_interval == 0)
{
Throw<std::runtime_error>(
"RWDB (in-memory backend) requires online_delete to "
"prevent OOM "
"Exception: standalone mode (used by tests) doesn't need "
"online_delete");
}
}
}
}
boost::filesystem::path
@@ -1071,5 +1081,4 @@ setup_FeeVote(Section const& section)
}
return setup;
}
} // namespace ripple

84
src/test/app/SHAMapStore_test.cpp
View File

@@ -521,96 +521,12 @@ public:
lastRotated = ledgerSeq - 1;
}
static std::unique_ptr<Config>
rwdbNoDeleteDefaultConfig()
{
// 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");
// Default is standalone = true from envconfig.cpp
return cfg;
}
static std::unique_ptr<Config>
rwdbNoDeleteNotStandalone()
{
// 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->setupControl(
true, true, false); // bQuiet, bSilent, bStandalone=false
return cfg;
}
static std::unique_ptr<Config>
rwdbWithDeleteNotStandalone()
{
// 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->setupControl(
true, true, false); // bQuiet, bSilent, bStandalone=false
return cfg;
}
void
testRWDBOnlineDeleteEnforcement()
{
testcase("RWDB online_delete enforcement");
// Test 1: RWDB without online_delete in standalone mode (should
// succeed)
{
jtx::Env env{*this, rwdbNoDeleteDefaultConfig()};
pass();
}
// Test 2: RWDB without online_delete NOT in standalone mode (should
// throw)
try
{
jtx::Env env{*this, rwdbNoDeleteNotStandalone()};
fail("Expected exception for RWDB without online_delete");
}
catch (std::runtime_error const& e)
{
BEAST_EXPECT(
std::string(e.what()).find(
"RWDB (in-memory backend) requires online_delete") !=
std::string::npos);
pass();
}
// Test 3: RWDB with online_delete NOT in standalone mode (should
// succeed) But should throw with another error message unrelated
try
{
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();
}
}
void
run() override
{
testClear();
testAutomatic();
testCanDelete();
testRWDBOnlineDeleteEnforcement();
}
};

92
src/test/core/Config_test.cpp
View File

@@ -1206,6 +1206,97 @@ r.ripple.com:51235
}
}
void
testRWDBOnlineDelete()
{
testcase("RWDB online_delete validation");
// Test 1: RWDB without online_delete in standalone mode (should
// succeed)
{
Config c;
std::string toLoad =
"[node_db]\n"
"type=rwdb\n"
"path=main\n";
c.setupControl(true, true, true); // standalone = true
try
{
c.loadFromString(toLoad);
pass(); // Should succeed
}
catch (std::runtime_error const& e)
{
fail("Should not throw in standalone mode");
}
}
// Test 2: RWDB without online_delete NOT in standalone mode (should
// throw)
{
Config c;
std::string toLoad =
"[node_db]\n"
"type=rwdb\n"
"path=main\n";
c.setupControl(true, true, false); // standalone = false
try
{
c.loadFromString(toLoad);
fail("Expected exception for RWDB without online_delete");
}
catch (std::runtime_error const& e)
{
BEAST_EXPECT(
std::string(e.what()).find(
"RWDB (in-memory backend) requires online_delete") !=
std::string::npos);
pass();
}
}
// Test 3: RWDB with online_delete NOT in standalone mode (should
// succeed)
{
Config c;
std::string toLoad =
"[node_db]\n"
"type=rwdb\n"
"path=main\n"
"online_delete=256\n";
c.setupControl(true, true, false); // standalone = false
try
{
c.loadFromString(toLoad);
pass(); // Should succeed
}
catch (std::runtime_error const& e)
{
fail("Should not throw when online_delete is configured");
}
}
// Test 4: Non-RWDB without online_delete NOT in standalone mode (should
// succeed)
{
Config c;
std::string toLoad =
"[node_db]\n"
"type=NuDB\n"
"path=main\n";
c.setupControl(true, true, false); // standalone = false
try
{
c.loadFromString(toLoad);
pass(); // Should succeed
}
catch (std::runtime_error const& e)
{
fail("Should not throw for non-RWDB backends");
}
}
}
void
testOverlay()
{
@@ -1295,6 +1386,7 @@ r.ripple.com:51235
testComments();
testGetters();
testAmendment();
testRWDBOnlineDelete();
testOverlay();
testNetworkID();
}