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

Use NuDB burst size #3662

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
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
4 changes: 2 additions & 2 deletions Builds/CMake/deps/Nudb.cmake
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ if (is_root_project) # NuDB not needed in the case of xrpl_core inclusion build
FetchContent_Declare(
nudb_src
GIT_REPOSITORY https://github.com/CPPAlliance/NuDB.git
GIT_TAG 2.0.3
GIT_TAG 2.0.5
)
FetchContent_GetProperties(nudb_src)
if(NOT nudb_src_POPULATED)
@@ -23,7 +23,7 @@ if (is_root_project) # NuDB not needed in the case of xrpl_core inclusion build
ExternalProject_Add (nudb_src
PREFIX ${nih_cache_path}
GIT_REPOSITORY https://github.com/CPPAlliance/NuDB.git
GIT_TAG 2.0.3
GIT_TAG 2.0.5
CONFIGURE_COMMAND ""
BUILD_COMMAND ""
TEST_COMMAND ""
2 changes: 2 additions & 0 deletions src/ripple/app/main/Application.cpp
Original file line number Diff line number Diff line change
@@ -940,6 +940,8 @@ class ApplicationImp : public Application, public RootStoppable, public BasicApp
std::unique_ptr<NodeStore::Database> source =
NodeStore::Manager::instance().make_Database(
"NodeStore.import",
megabytes(config_->getValueFor(
SizedItem::burstSize, boost::none)),
dummyScheduler,
0,
dummyRoot,
7 changes: 6 additions & 1 deletion src/ripple/app/misc/SHAMapStoreImp.cpp
Original file line number Diff line number Diff line change
@@ -259,6 +259,8 @@ SHAMapStoreImp::makeNodeStore(std::string const& name, std::int32_t readThreads)
{
db = NodeStore::Manager::instance().make_Database(
name,
megabytes(
app_.config().getValueFor(SizedItem::burstSize, boost::none)),
scheduler_,
readThreads,
app_.getJobQueue(),
@@ -562,7 +564,10 @@ SHAMapStoreImp::makeBackendRotating(std::string path)
section.set("path", newPath.string());

auto backend{NodeStore::Manager::instance().make_Backend(
section, scheduler_, app_.logs().journal(nodeStoreName_))};
section,
megabytes(app_.config().getValueFor(SizedItem::burstSize, boost::none)),
scheduler_,
app_.logs().journal(nodeStoreName_))};
backend->open();
return backend;
}
3 changes: 2 additions & 1 deletion src/ripple/core/Config.h
Original file line number Diff line number Diff line change
@@ -57,7 +57,8 @@ enum class SizedItem : std::size_t {
hashNodeDBCache,
txnDBCache,
lgrDBCache,
openFinalLimit
openFinalLimit,
burstSize
};

// This entire derived class is deprecated.
40 changes: 21 additions & 19 deletions src/ripple/core/impl/Config.cpp
Original file line number Diff line number Diff line change
@@ -40,25 +40,27 @@

namespace ripple {

inline constexpr std::array<std::pair<SizedItem, std::array<int, 5>>, 12>
sizedItems{
{// FIXME: We should document each of these items, explaining exactly
// what
// they control and whether there exists an explicit config
// option that can be used to override the default.
{SizedItem::sweepInterval, {{10, 30, 60, 90, 120}}},
{SizedItem::treeCacheSize,
{{128000, 256000, 512000, 768000, 2048000}}},
{SizedItem::treeCacheAge, {{30, 60, 90, 120, 900}}},
{SizedItem::ledgerSize, {{32, 128, 256, 384, 768}}},
{SizedItem::ledgerAge, {{30, 90, 180, 240, 900}}},
{SizedItem::ledgerFetch, {{2, 3, 4, 5, 8}}},
{SizedItem::nodeCacheSize, {{16384, 32768, 131072, 262144, 524288}}},
{SizedItem::nodeCacheAge, {{60, 90, 120, 900, 1800}}},
{SizedItem::hashNodeDBCache, {{4, 12, 24, 64, 128}}},
{SizedItem::txnDBCache, {{4, 12, 24, 64, 128}}},
{SizedItem::lgrDBCache, {{4, 8, 16, 32, 128}}},
{SizedItem::openFinalLimit, {{8, 16, 32, 64, 128}}}}};
// The configurable node sizes are "tiny", "small", "medium", "large", "huge"
inline constexpr std::array<std::pair<SizedItem, std::array<int, 5>>, 13>
sizedItems{{
// FIXME: We should document each of these items, explaining exactly
// what
// they control and whether there exists an explicit config
// option that can be used to override the default.
{SizedItem::sweepInterval, {{10, 30, 60, 90, 120}}},
{SizedItem::treeCacheSize, {{128000, 256000, 512000, 768000, 2048000}}},
{SizedItem::treeCacheAge, {{30, 60, 90, 120, 900}}},
{SizedItem::ledgerSize, {{32, 128, 256, 384, 768}}},
{SizedItem::ledgerAge, {{30, 90, 180, 240, 900}}},
{SizedItem::ledgerFetch, {{2, 3, 4, 5, 8}}},
{SizedItem::nodeCacheSize, {{16384, 32768, 131072, 262144, 524288}}},
{SizedItem::nodeCacheAge, {{60, 90, 120, 900, 1800}}},
{SizedItem::hashNodeDBCache, {{4, 12, 24, 64, 128}}},
{SizedItem::txnDBCache, {{4, 12, 24, 64, 128}}},
{SizedItem::lgrDBCache, {{4, 8, 16, 32, 128}}},
{SizedItem::openFinalLimit, {{8, 16, 32, 64, 128}}},
{SizedItem::burstSize, {{4, 8, 16, 32, 48}}},
Copy link
Contributor

@nbougalis nbougalis Nov 15, 2020

Choose a reason for hiding this comment

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

Two points:

  1. We should think through these numbers and make sure that they are realistic for the given "node sizes". Why is "4MB" appropriate and why is x12 an appropriate factor when going from "tiny" to "huge"?

  2. Please consider reformatting this thusly:

// clang-format off
inline constexpr std::array<std::pair<SizedItem, std::array<int, 5>>, 12>
sizedItems
{{
    // FIXME: We should document each of these items separately, explaining
    //        exactly what they control and whether there exists an explicit
    //        config option that can be used to override the default.
    { SizedItem::sweepInterval,   {{     10,     30,     60,     90,     120 }} },
    { SizedItem::treeCacheSize,   {{ 128000, 256000, 512000, 768000, 2048000 }} },
    { SizedItem::treeCacheAge,    {{     30,     60,     90,    120,     900 }} },
    { SizedItem::ledgerSize,      {{     32,    128,    256,    384,     768 }} },
    { SizedItem::ledgerAge,       {{     30,     90,    180,    240,     900 }} },
    { SizedItem::ledgerFetch,     {{      2,      3,      4,      5,       8 }} },
    { SizedItem::nodeCacheSize,   {{  16384,  32768, 131072, 262144,  524288 }} },
    { SizedItem::nodeCacheAge,    {{     60,     90,    120,    900,    1800 }} },
    { SizedItem::hashNodeDBCache, {{      4,     12,     24,     64,     128 }} },
    { SizedItem::txnDBCache,      {{      4,     12,     24,     64,     128 }} },
    { SizedItem::lgrDBCache,      {{      4,      8,     16,     32,     128 }} },
    { SizedItem::openFinalLimit,  {{      8,     16,     32,     64,     128 }} }
    { SizedItem::burstSize,       {{      4,      8,     16,     32,      48 }} },
}};
// clang-format on

I love clang-format and having a consistent style but, like Morpheus says:

What you must learn is that these rules are no different from the rules of a computer system. Some of them can be bent, others can be broken.

And this is a case where rules can be bent and/or broken.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

4MB is the default NuDB size and a good minimum sized beneficial threshold. David observed increasing benefits up to 32MB, where diminishing returns begin. There is little gain with 48MB but that correlates with "huge".

}};

// Ensure that the order of entries in the table corresponds to the
// order of entries in the enum:
4 changes: 4 additions & 0 deletions src/ripple/nodestore/Factory.h
Original file line number Diff line number Diff line change
@@ -42,20 +42,23 @@ class Factory

@param keyBytes The fixed number of bytes per key.
@param parameters A set of key/value configuration pairs.
@param burstSize Backend burst size in bytes.
@param scheduler The scheduler to use for running tasks.
@return A pointer to the Backend object.
*/
virtual std::unique_ptr<Backend>
createInstance(
size_t keyBytes,
Section const& parameters,
std::size_t burstSize,
Scheduler& scheduler,
beast::Journal journal) = 0;

/** Create an instance of this factory's backend.

@param keyBytes The fixed number of bytes per key.
@param parameters A set of key/value configuration pairs.
@param burstSize Backend burst size in bytes.
@param scheduler The scheduler to use for running tasks.
@param context The context used by database.
@return A pointer to the Backend object.
@@ -64,6 +67,7 @@ class Factory
createInstance(
size_t keyBytes,
Section const& parameters,
std::size_t burstSize,
Scheduler& scheduler,
nudb::context& context,
beast::Journal journal)
4 changes: 4 additions & 0 deletions src/ripple/nodestore/Manager.h
Original file line number Diff line number Diff line change
@@ -60,6 +60,7 @@ class Manager
virtual std::unique_ptr<Backend>
make_Backend(
Section const& parameters,
std::size_t burstSize,
Scheduler& scheduler,
beast::Journal journal) = 0;

@@ -81,6 +82,7 @@ class Manager
thrown.

@param name A diagnostic label for the database.
@param burstSize Backend burst size in bytes.
@param scheduler The scheduler to use for performing asynchronous tasks.
@param readThreads The number of async read threads to create
@param backendParameters The parameter string for the persistent
@@ -93,6 +95,7 @@ class Manager
virtual std::unique_ptr<Database>
make_Database(
std::string const& name,
std::size_t burstSize,
Scheduler& scheduler,
int readThreads,
Stoppable& parent,
@@ -106,6 +109,7 @@ class Manager
std::unique_ptr<Backend>
make_Backend(
Section const& config,
std::size_t burstSize,
Scheduler& scheduler,
beast::Journal journal);

2 changes: 2 additions & 0 deletions src/ripple/nodestore/backend/MemoryFactory.cpp
Original file line number Diff line number Diff line change
@@ -55,6 +55,7 @@ class MemoryFactory : public Factory
createInstance(
size_t keyBytes,
Section const& keyValues,
std::size_t burstSize,
Scheduler& scheduler,
beast::Journal journal) override;

@@ -227,6 +228,7 @@ std::unique_ptr<Backend>
MemoryFactory::createInstance(
size_t keyBytes,
Section const& keyValues,
std::size_t,
Scheduler& scheduler,
beast::Journal journal)
{
12 changes: 10 additions & 2 deletions src/ripple/nodestore/backend/NuDBFactory.cpp
Original file line number Diff line number Diff line change
@@ -42,6 +42,7 @@ class NuDBBackend : public Backend

beast::Journal const j_;
size_t const keyBytes_;
std::size_t const burstSize_;
std::string const name_;
nudb::store db_;
std::atomic<bool> deletePath_;
@@ -50,10 +51,12 @@ class NuDBBackend : public Backend
NuDBBackend(
size_t keyBytes,
Section const& keyValues,
std::size_t burstSize,
Scheduler& scheduler,
beast::Journal journal)
: j_(journal)
, keyBytes_(keyBytes)
, burstSize_(burstSize)
, name_(get<std::string>(keyValues, "path"))
, deletePath_(false)
, scheduler_(scheduler)
@@ -66,11 +69,13 @@ class NuDBBackend : public Backend
NuDBBackend(
size_t keyBytes,
Section const& keyValues,
std::size_t burstSize,
Scheduler& scheduler,
nudb::context& context,
beast::Journal journal)
: j_(journal)
, keyBytes_(keyBytes)
, burstSize_(burstSize)
, name_(get<std::string>(keyValues, "path"))
, db_(context)
, deletePath_(false)
@@ -130,6 +135,7 @@ class NuDBBackend : public Backend
Throw<nudb::system_error>(ec);
if (db_.appnum() != currentType)
Throw<std::runtime_error>("nodestore: unknown appnum");
db_.set_burst(burstSize_);
}

bool
@@ -333,23 +339,25 @@ class NuDBFactory : public Factory
createInstance(
size_t keyBytes,
Section const& keyValues,
std::size_t burstSize,
Scheduler& scheduler,
beast::Journal journal) override
{
return std::make_unique<NuDBBackend>(
keyBytes, keyValues, scheduler, journal);
keyBytes, keyValues, burstSize, scheduler, journal);
}

std::unique_ptr<Backend>
createInstance(
size_t keyBytes,
Section const& keyValues,
std::size_t burstSize,
Scheduler& scheduler,
nudb::context& context,
beast::Journal journal) override
{
return std::make_unique<NuDBBackend>(
keyBytes, keyValues, scheduler, context, journal);
keyBytes, keyValues, burstSize, scheduler, context, journal);
}
};

7 changes: 6 additions & 1 deletion src/ripple/nodestore/backend/NullFactory.cpp
Original file line number Diff line number Diff line change
@@ -136,7 +136,12 @@ class NullFactory : public Factory
}

std::unique_ptr<Backend>
createInstance(size_t, Section const&, Scheduler&, beast::Journal) override
createInstance(
size_t,
Section const&,
std::size_t,
Scheduler&,
beast::Journal) override
{
return std::make_unique<NullBackend>();
}
1 change: 1 addition & 0 deletions src/ripple/nodestore/backend/RocksDBFactory.cpp
Original file line number Diff line number Diff line change
@@ -450,6 +450,7 @@ class RocksDBFactory : public Factory
createInstance(
size_t keyBytes,
Section const& keyValues,
std::size_t,
Scheduler& scheduler,
beast::Journal journal) override
{
10 changes: 7 additions & 3 deletions src/ripple/nodestore/impl/ManagerImp.cpp
Original file line number Diff line number Diff line change
@@ -43,6 +43,7 @@ ManagerImp::missing_backend()
std::unique_ptr<Backend>
ManagerImp::make_Backend(
Section const& parameters,
std::size_t burstSize,
Scheduler& scheduler,
beast::Journal journal)
{
@@ -55,19 +56,20 @@ ManagerImp::make_Backend(
missing_backend();

return factory->createInstance(
NodeObject::keyBytes, parameters, scheduler, journal);
NodeObject::keyBytes, parameters, burstSize, scheduler, journal);
}

std::unique_ptr<Database>
ManagerImp::make_Database(
std::string const& name,
std::size_t burstSize,
Scheduler& scheduler,
int readThreads,
Stoppable& parent,
Section const& config,
beast::Journal journal)
{
auto backend{make_Backend(config, scheduler, journal)};
auto backend{make_Backend(config, burstSize, scheduler, journal)};
backend->open();
return std::make_unique<DatabaseNodeImp>(
name,
@@ -124,10 +126,12 @@ Manager::instance()
std::unique_ptr<Backend>
make_Backend(
Section const& config,
std::size_t burstSize,
Scheduler& scheduler,
beast::Journal journal)
{
return Manager::instance().make_Backend(config, scheduler, journal);
return Manager::instance().make_Backend(
config, burstSize, scheduler, journal);
}

} // namespace NodeStore
2 changes: 2 additions & 0 deletions src/ripple/nodestore/impl/ManagerImp.h
Original file line number Diff line number Diff line change
@@ -54,12 +54,14 @@ class ManagerImp : public Manager
std::unique_ptr<Backend>
make_Backend(
Section const& parameters,
std::size_t burstSize,
Scheduler& scheduler,
beast::Journal journal) override;

std::unique_ptr<Database>
make_Database(
std::string const& name,
std::size_t burstSize,
Scheduler& scheduler,
int readThreads,
Stoppable& parent,
7 changes: 6 additions & 1 deletion src/ripple/nodestore/impl/Shard.cpp
Original file line number Diff line number Diff line change
@@ -115,7 +115,12 @@ Shard::init(Scheduler& scheduler, nudb::context& context)
return false;
}
backend_ = factory->createInstance(
NodeObject::keyBytes, section, scheduler, context, j_);
NodeObject::keyBytes,
section,
megabytes(app_.config().getValueFor(SizedItem::burstSize, boost::none)),
scheduler,
context,
j_);

return open(lock);
}
8 changes: 4 additions & 4 deletions src/test/nodestore/Backend_test.cpp
Original file line number Diff line number Diff line change
@@ -58,8 +58,8 @@ class Backend_test : public TestBase

{
// Open the backend
std::unique_ptr<Backend> backend =
Manager::instance().make_Backend(params, scheduler, journal);
std::unique_ptr<Backend> backend = Manager::instance().make_Backend(
params, megabytes(4), scheduler, journal);
backend->open();

// Write the batch
@@ -83,8 +83,8 @@ class Backend_test : public TestBase

{
// Re-open the backend
std::unique_ptr<Backend> backend =
Manager::instance().make_Backend(params, scheduler, journal);
std::unique_ptr<Backend> backend = Manager::instance().make_Backend(
params, megabytes(4), scheduler, journal);
backend->open();

// Read it back in
Loading