Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve online_delete configuration and DB tuning: #3429

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions cfg/rippled-example.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -993,6 +993,9 @@
# These settings trade speed and reduced I/O
# for a higher risk of data loss. See the
# individual settings below for more information.
# This setting may not be combined with any of the
# other tuning settings: "journal_mode",
# "synchronous", or "temp_store".
#
# journal_mode Valid values: delete, truncate, persist, memory, wal, off
# The default is "wal", which uses a write-ahead
Expand All @@ -1002,6 +1005,8 @@
# database is likely to be corrupted.
# See https://www.sqlite.org/pragma.html#pragma_journal_mode
# for more details about the available options.
# This setting may not be combined with the
# "safety_level" setting.
#
# synchronous Valid values: off, normal, full, extra
# The default is "normal", which works well with
Expand All @@ -1013,6 +1018,8 @@
# data to disk.
# See https://www.sqlite.org/pragma.html#pragma_synchronous
# for more details about the available options.
# This setting may not be combined with the
# "safety_level" setting.
#
# temp_store Valid values: default, file, memory
seelabs marked this conversation as resolved.
Show resolved Hide resolved
# The default is "file", which will use files
Expand All @@ -1022,6 +1029,8 @@
# of these temporary objects.
# See https://www.sqlite.org/pragma.html#pragma_temp_store
# for more details about the available options.
# This setting may not be combined with the
# "safety_level" setting.
#
#
#
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/main/Application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1364,7 +1364,7 @@ class ApplicationImp : public Application, public RootStoppable, public BasicApp
JLOG(m_journal.fatal())
<< "Free SQLite space for transaction db is less than "
"512MB. To fix this, rippled must be executed with the "
"vacuum <sqlitetmpdir> parameter before restarting. "
"\"--vacuum\" parameter before restarting. "
"Note that this activity can take multiple days, "
"depending on database size.";
signalStop();
Expand Down
30 changes: 12 additions & 18 deletions src/ripple/app/main/Main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -354,10 +354,7 @@ run(int argc, char** argv)
"nodetoshard", "Import node store into shards")(
"replay", "Replay a ledger close.")(
"start", "Start from a fresh Ledger.")(
"vacuum",
po::value<std::string>(),
"VACUUM the transaction db. Mandatory string argument specifies "
"temporary directory path.")(
"vacuum", "VACUUM the transaction db.")(
"valid", "Consider the initial ledger a valid network ledger.")(
"validateShards", shardsText.c_str());

Expand Down Expand Up @@ -522,26 +519,26 @@ run(int argc, char** argv)
using namespace boost::filesystem;
DatabaseCon::Setup dbSetup = setup_DatabaseCon(*config);
path dbPath = dbSetup.dataDir / TxDBName;
path tmpPath = vm["vacuum"].as<std::string>();

try
{
uintmax_t const dbSize = file_size(dbPath);
assert(dbSize != static_cast<uintmax_t>(-1));

if (space(tmpPath).available < dbSize)
{
std::cerr << "A valid directory for vacuuming must be "
"specified on a filesystem with at least "
"as much free space as the size of "
<< dbPath.string() << ", which is " << dbSize
<< " bytes. The filesystem for " << tmpPath.string()
<< " only has " << space(tmpPath).available
<< " bytes.\n";
return -1;
auto available = space(dbPath.parent_path()).available;
if (available < dbSize)
seelabs marked this conversation as resolved.
Show resolved Hide resolved
{
std::cerr
<< "The database filesystem must have at least as "
"much free space as the size of "
<< dbPath.string() << ", which is " << dbSize
<< " bytes. Only " << available
<< " bytes are available.\n";
return -1;
}
}

dbSetup.noPragma();
auto txnDB = std::make_unique<DatabaseCon>(
dbSetup, TxDBName, TxDBPragma, TxDBInit);
auto& session = txnDB->getSession();
Expand All @@ -552,9 +549,6 @@ run(int argc, char** argv)
std::cout << "VACUUM beginning. page_size: " << pageSize
<< std::endl;

session << "PRAGMA journal_mode=OFF;";
session << "PRAGMA temp_store_directory=\"" << tmpPath.string()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of the temp_store_directory is to make sure there's enough space to perform a VACUUM, which essentially requires as much as the entire database being VACUUMed. The temp_store in memory will likely cause a system to run out of RAM. Imagine transaction DB with 2TB+. (https://sqlite.org/tempfiles.html)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I'll change that.

<< "\";";
session << "VACUUM;";
assert(dbSetup.globalPragma);
for (auto const& p : *dbSetup.globalPragma)
Expand Down
43 changes: 29 additions & 14 deletions src/ripple/core/impl/DatabaseCon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,21 @@ setup_DatabaseCon(Config const& c, boost::optional<beast::Journal> j)
result->reserve(3);

// defaults
std::string safety_level = "high";
std::string safety_level;
std::string journal_mode = "wal";
std::string synchronous = "normal";
std::string temp_store = "file";
bool showRiskWarning = false;

bool showWarning = false;
if (set(safety_level, "safety_level", sqlite))
{
if (boost::iequals(safety_level, "low"))
showRiskWarning = boost::iequals(safety_level, "low");
if (showRiskWarning)
seelabs marked this conversation as resolved.
Show resolved Hide resolved
{
// low safety defaults
journal_mode = "memory";
synchronous = "off";
temp_store = "memory";
showWarning = true;
}
else if (!boost::iequals(safety_level, "high"))
{
Expand All @@ -74,11 +74,16 @@ setup_DatabaseCon(Config const& c, boost::optional<beast::Journal> j)
{
// #journal_mode Valid values : delete, truncate, persist,
// memory, wal, off
if (c.LEDGER_HISTORY < SQLITE_TUNING_CUTOFF)
set(journal_mode, "journal_mode", sqlite);
if (set(journal_mode, "journal_mode", sqlite) &&
!safety_level.empty())
{
Throw<std::runtime_error>(
"Configuration file may not define both "
"\"safety_level\" and \"journal_mode\"");
}
bool higherRisk = boost::iequals(journal_mode, "memory") ||
boost::iequals(journal_mode, "off");
showWarning = showWarning || higherRisk;
showRiskWarning = showRiskWarning || higherRisk;
if (higherRisk || boost::iequals(journal_mode, "delete") ||
boost::iequals(journal_mode, "truncate") ||
boost::iequals(journal_mode, "persist") ||
Expand All @@ -96,10 +101,15 @@ setup_DatabaseCon(Config const& c, boost::optional<beast::Journal> j)

{
//#synchronous Valid values : off, normal, full, extra
if (c.LEDGER_HISTORY < SQLITE_TUNING_CUTOFF)
set(synchronous, "synchronous", sqlite);
if (set(synchronous, "synchronous", sqlite) &&
!safety_level.empty())
{
Throw<std::runtime_error>(
"Configuration file may not define both "
"\"safety_level\" and \"synchronous\"");
}
bool higherRisk = boost::iequals(synchronous, "off");
showWarning = showWarning || higherRisk;
showRiskWarning = showRiskWarning || higherRisk;
if (higherRisk || boost::iequals(synchronous, "normal") ||
boost::iequals(synchronous, "full") ||
boost::iequals(synchronous, "extra"))
Expand All @@ -116,10 +126,15 @@ setup_DatabaseCon(Config const& c, boost::optional<beast::Journal> j)

{
// #temp_store Valid values : default, file, memory
if (c.LEDGER_HISTORY < SQLITE_TUNING_CUTOFF)
set(temp_store, "temp_store", sqlite);
if (set(temp_store, "temp_store", sqlite) &&
!safety_level.empty())
{
Throw<std::runtime_error>(
"Configuration file may not define both "
"\"safety_level\" and \"temp_store\"");
}
bool higherRisk = boost::iequals(temp_store, "memory");
showWarning = showWarning || higherRisk;
showRiskWarning = showRiskWarning || higherRisk;
if (higherRisk || boost::iequals(temp_store, "default") ||
boost::iequals(temp_store, "file"))
{
Expand All @@ -133,7 +148,7 @@ setup_DatabaseCon(Config const& c, boost::optional<beast::Journal> j)
}
}

if (showWarning && j && c.LEDGER_HISTORY > SQLITE_TUNING_CUTOFF)
if (showRiskWarning && j && c.LEDGER_HISTORY > SQLITE_TUNING_CUTOFF)
{
JLOG(j->warn())
<< "reducing the data integrity guarantees from the "
Expand Down
6 changes: 1 addition & 5 deletions src/test/jtx/Env.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,7 @@ class Env
std::unique_ptr<Logs> logs = nullptr,
beast::severities::Severity thresh = beast::severities::kError)
: test(suite_)
, bundle_(
suite_,
std::move(config),
logs ? std::move(logs) : std::make_unique<SuiteLogs>(suite_),
thresh)
, bundle_(suite_, std::move(config), std::move(logs), thresh)
, journal{bundle_.app->journal("Env")}
{
memoize(Account::master);
Expand Down
13 changes: 11 additions & 2 deletions src/test/jtx/impl/Env.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,17 @@ Env::AppBundle::AppBundle(
: AppBundle()
{
using namespace beast::severities;
// Use kFatal threshold to reduce noise from STObject.
setDebugLogSink(std::make_unique<SuiteJournalSink>("Debug", kFatal, suite));
if (logs)
{
setDebugLogSink(logs->makeSink("Debug", kFatal));
}
else
{
logs = std::make_unique<SuiteLogs>(suite);
// Use kFatal threshold to reduce noise from STObject.
setDebugLogSink(
std::make_unique<SuiteJournalSink>("Debug", kFatal, suite));
}
auto timeKeeper_ = std::make_unique<ManualTimeKeeper>();
timeKeeper = timeKeeper_.get();
// Hack so we don't have to call Config::setup
Expand Down
Loading