Skip to content

Commit

Permalink
[FOLD] Address feedback from Scott Schurr
Browse files Browse the repository at this point in the history
  • Loading branch information
@miguelportilla
miguelportilla committed Mar 5, 2021
1 parent d134990 commit 4ed1a55
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 174 deletions.
4 changes: 2 additions & 2 deletions src/ripple/nodestore/impl/DatabaseShardImp.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1288,8 +1288,8 @@ DatabaseShardImp::finalizeShard(
if (isStopping())
return;

auto const boundaryIndex{shardBoundaryIndex()};
{
auto const boundaryIndex{shardBoundaryIndex()};
std::lock_guard lock(mutex_);

if (shard->index() < boundaryIndex)
Expand All @@ -1313,7 +1313,7 @@ DatabaseShardImp::finalizeShard(
if (boundaryIndex == shard->index())
secondLatestShardIndex_ = boundaryIndex;
else
latestShardIndex_ = boundaryIndex;
latestShardIndex_ = shard->index();

if (shard->getDir().parent_path() != dir_)
{
Expand Down
279 changes: 107 additions & 172 deletions src/test/nodestore/DatabaseShard_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class DatabaseShard_test : public TestBase
/* ring used to generate pseudo-random sequence */
beast::xor_shift_engine rng_;
/* number of shards to generate */
int nShards_;
int numShards_;
/* vector of accounts used to send test transactions */
std::vector<test::jtx::Account> accounts_;
/* nAccounts_[i] is the number of these accounts existed before i-th
Expand All @@ -76,11 +76,11 @@ class DatabaseShard_test : public TestBase
TestData(
std::uint64_t const seedValue,
int dataSize = dataSizeMax,
int nShards = 1)
: rng_(seedValue), nShards_(nShards)
int numShards = 1)
: rng_(seedValue), numShards_(numShards)
{
std::uint32_t n = 0;
std::uint32_t nLedgers = ledgersPerShard * nShards;
std::uint32_t nLedgers = ledgersPerShard * numShards;

nAccounts_.reserve(nLedgers);
payAccounts_.reserve(nLedgers);
Expand Down Expand Up @@ -165,7 +165,7 @@ class DatabaseShard_test : public TestBase
}
}

for (std::uint32_t i = 0; i < ledgersPerShard * nShards_; ++i)
for (std::uint32_t i = 0; i < ledgersPerShard * numShards_; ++i)
{
auto const index = i + (startIndex * ledgersPerShard);

Expand Down Expand Up @@ -490,41 +490,41 @@ class DatabaseShard_test : public TestBase
std::optional<std::uint32_t>
createShard(
TestData& data,
DatabaseShard& db,
DatabaseShard& shardStore,
int maxShardIndex = 1,
int ledgerOffset = 0)
{
int shardIndex{-1};

for (std::uint32_t i = 0; i < ledgersPerShard; ++i)
{
auto const ledgerSeq{
db.prepareLedger((maxShardIndex + 1) * ledgersPerShard)};
auto const ledgerSeq{shardStore.prepareLedger(
(maxShardIndex + 1) * ledgersPerShard)};
if (!BEAST_EXPECT(ledgerSeq != boost::none))
return std::nullopt;

shardIndex = db.seqToShardIndex(*ledgerSeq);
shardIndex = shardStore.seqToShardIndex(*ledgerSeq);

int const arrInd = *ledgerSeq - (ledgersPerShard * ledgerOffset) -
ledgersPerShard - 1;
BEAST_EXPECT(
arrInd >= 0 && arrInd < maxShardIndex * ledgersPerShard);
BEAST_EXPECT(saveLedger(db, *data.ledgers_[arrInd]));
BEAST_EXPECT(saveLedger(shardStore, *data.ledgers_[arrInd]));
if (arrInd % ledgersPerShard == (ledgersPerShard - 1))
{
uint256 const finalKey_{0};
Serializer s;
s.add32(Shard::version);
s.add32(db.firstLedgerSeq(shardIndex));
s.add32(db.lastLedgerSeq(shardIndex));
s.add32(shardStore.firstLedgerSeq(shardIndex));
s.add32(shardStore.lastLedgerSeq(shardIndex));
s.addRaw(data.ledgers_[arrInd]->info().hash.data(), 256 / 8);
db.store(
shardStore.store(
hotUNKNOWN, std::move(s.modData()), finalKey_, *ledgerSeq);
}
db.setStored(data.ledgers_[arrInd]);
shardStore.setStored(data.ledgers_[arrInd]);
}

return waitShard(db, shardIndex);
return waitShard(shardStore, shardIndex);
}

void
Expand Down Expand Up @@ -1024,8 +1024,7 @@ class DatabaseShard_test : public TestBase

using namespace test::jtx;

// Test importing with multiple historical
// paths
// Test importing with multiple historical paths
{
beast::temp_dir shardDir;
std::array<beast::temp_dir, 4> historicalDirs;
Expand Down Expand Up @@ -1095,8 +1094,7 @@ class DatabaseShard_test : public TestBase
BEAST_EXPECT(historicalPathCount == ledgerCount - 2);
}

// Test importing with a single historical
// path
// Test importing with a single historical path
{
beast::temp_dir shardDir;
beast::temp_dir historicalDir;
Expand Down Expand Up @@ -1156,177 +1154,114 @@ class DatabaseShard_test : public TestBase

using namespace test::jtx;

// Test importing with multiple historical
// paths
{
beast::temp_dir shardDir;
std::array<beast::temp_dir, 4> historicalDirs;
std::array<boost::filesystem::path, 4> historicalPaths;

std::transform(
historicalDirs.begin(),
historicalDirs.end(),
historicalPaths.begin(),
[](const beast::temp_dir& dir) { return dir.path(); });

beast::temp_dir nodeDir;
auto c = testConfig(shardDir.path());

auto& historyPaths = c->section(SECTION_HISTORICAL_SHARD_PATHS);
historyPaths.append(
{historicalPaths[0].string(),
historicalPaths[1].string(),
historicalPaths[2].string(),
historicalPaths[3].string()});
// Create the primary shard directory
beast::temp_dir primaryDir;
auto config{testConfig(primaryDir.path())};

Env env{*this, std::move(c)};
DatabaseShard* db = env.app().getShardStore();
BEAST_EXPECT(db);
// Create four historical directories
std::array<beast::temp_dir, 4> historicalDirs;
{
auto& paths{config->section(SECTION_HISTORICAL_SHARD_PATHS)};
for (auto const& dir : historicalDirs)
paths.append(dir.path());
}

auto const ledgerCount = 4;
Env env{*this, std::move(config)};

TestData data(seedValue, 4, ledgerCount);
if (!BEAST_EXPECT(data.makeLedgers(env)))
return;
// Create some shards
std::uint32_t constexpr numShards{4};
TestData data(seedValue, 4, numShards);
if (!BEAST_EXPECT(data.makeLedgers(env)))
return;

BEAST_EXPECT(db->getShardInfo()->finalized().empty());
auto shardStore{env.app().getShardStore()};
BEAST_EXPECT(shardStore);

// Add ten shards to the Shard Database
for (auto i = 0; i < ledgerCount; ++i)
for (auto i = 0; i < numShards; ++i)
{
auto const shardIndex{createShard(data, *shardStore, numShards)};
if (!BEAST_EXPECT(
shardIndex && *shardIndex >= 1 && *shardIndex <= numShards))
{
auto const shardIndex{createShard(data, *db, ledgerCount)};
if (!BEAST_EXPECT(
shardIndex && *shardIndex >= 1 &&
*shardIndex <= ledgerCount))
{
return;
}

BEAST_EXPECT(boost::icl::contains(
db->getShardInfo()->finalized(), *shardIndex));
return;
}
}

auto mainPathCount = std::distance(
boost::filesystem::directory_iterator(shardDir.path()),
boost::filesystem::directory_iterator());

// Only the two most recent shards
// should be stored at the main path
BEAST_EXPECT(mainPathCount == 2);

// Confirm recent shard locations
std::set<boost::filesystem::path> mainPathShards{
shardDir.path() / boost::filesystem::path("3"),
shardDir.path() / boost::filesystem::path("4")};
std::set<boost::filesystem::path> actual(
boost::filesystem::directory_iterator(shardDir.path()),
boost::filesystem::directory_iterator());

BEAST_EXPECT(mainPathShards == actual);

const auto generateHistoricalStems = [&historicalPaths, &actual] {
for (auto const& path : historicalPaths)
{
for (auto const& shard :
boost::filesystem::directory_iterator(path))
{
actual.insert(boost::filesystem::path(shard).stem());
}
}
};

// Confirm historical shard locations
std::set<boost::filesystem::path> historicalPathShards;
std::generate_n(
std::inserter(
historicalPathShards, historicalPathShards.begin()),
2,
[n = 1]() mutable { return std::to_string(n++); });
actual.clear();
generateHistoricalStems();
{
// Confirm finalized shards are in the shard store
auto const finalized{shardStore->getShardInfo()->finalized()};
BEAST_EXPECT(boost::icl::length(finalized) == numShards);
BEAST_EXPECT(boost::icl::first(finalized) == 1);
BEAST_EXPECT(boost::icl::last(finalized) == numShards);
}

BEAST_EXPECT(historicalPathShards == actual);
using namespace boost::filesystem;
auto const dirContains = [](beast::temp_dir const& dir,
std::uint32_t shardIndex) {
boost::filesystem::path const path(std::to_string(shardIndex));
for (auto const& it : directory_iterator(dir.path()))
if (boost::filesystem::path(it).stem() == path)
return true;
return false;
};
auto const historicalDirsContains = [&](std::uint32_t shardIndex) {
for (auto const& dir : historicalDirs)
if (dirContains(dir, shardIndex))
return true;
return false;
};

auto historicalPathCount = std::accumulate(
historicalPaths.begin(),
historicalPaths.end(),
0,
[](int const sum, boost::filesystem::path const& path) {
return sum +
std::distance(
boost::filesystem::directory_iterator(path),
boost::filesystem::directory_iterator());
});
// Confirm two most recent shards are in the primary shard directory
for (auto const shardIndex : {numShards - 1, numShards})
{
BEAST_EXPECT(dirContains(primaryDir, shardIndex));
BEAST_EXPECT(!historicalDirsContains(shardIndex));
}

// All historical shards should be stored
// at historical paths
BEAST_EXPECT(historicalPathCount == ledgerCount - 2);
// Confirm remaining shards are in the historical shard directories
for (auto shardIndex = 1; shardIndex < numShards - 1; ++shardIndex)
{
BEAST_EXPECT(!dirContains(primaryDir, shardIndex));
BEAST_EXPECT(historicalDirsContains(shardIndex));
}

data = TestData(seedValue * 2, 4, ledgerCount);
if (!BEAST_EXPECT(data.makeLedgers(env, ledgerCount)))
return;
// Create some more shards to exercise recent shard rotation
data = TestData(seedValue * 2, 4, numShards);
if (!BEAST_EXPECT(data.makeLedgers(env, numShards)))
return;

// Add ten more shards to the Shard Database
// to exercise recent shard rotation
for (auto i = 0; i < ledgerCount; ++i)
for (auto i = 0; i < numShards; ++i)
{
auto const shardIndex{
createShard(data, *shardStore, numShards * 2, numShards)};
if (!BEAST_EXPECT(
shardIndex && *shardIndex >= numShards + 1 &&
*shardIndex <= numShards * 2))
{
auto const shardIndex{
createShard(data, *db, ledgerCount * 2, ledgerCount)};
if (!BEAST_EXPECT(
shardIndex && *shardIndex >= ledgerCount + 1 &&
*shardIndex <= ledgerCount * 2))
{
return;
}

BEAST_EXPECT(boost::icl::contains(
db->getShardInfo()->finalized(), *shardIndex));
return;
}
}

mainPathCount = std::distance(
boost::filesystem::directory_iterator(shardDir.path()),
boost::filesystem::directory_iterator());

// Only the two most recent shards
// should be stored at the main path
BEAST_EXPECT(mainPathCount == 2);

// Confirm recent shard locations
mainPathShards = {
shardDir.path() / boost::filesystem::path("7"),
shardDir.path() / boost::filesystem::path("8")};
actual = {
boost::filesystem::directory_iterator(shardDir.path()),
boost::filesystem::directory_iterator()};

BEAST_EXPECT(mainPathShards == actual);

// Confirm historical shard locations
historicalPathShards.clear();
std::generate_n(
std::inserter(
historicalPathShards, historicalPathShards.begin()),
6,
[n = 1]() mutable { return std::to_string(n++); });
actual.clear();
generateHistoricalStems();

BEAST_EXPECT(historicalPathShards == actual);
{
// Confirm finalized shards are in the shard store
auto const finalized{shardStore->getShardInfo()->finalized()};
BEAST_EXPECT(boost::icl::length(finalized) == numShards * 2);
BEAST_EXPECT(boost::icl::first(finalized) == 1);
BEAST_EXPECT(boost::icl::last(finalized) == numShards * 2);
}

historicalPathCount = std::accumulate(
historicalPaths.begin(),
historicalPaths.end(),
0,
[](int const sum, boost::filesystem::path const& path) {
return sum +
std::distance(
boost::filesystem::directory_iterator(path),
boost::filesystem::directory_iterator());
});
// Confirm two most recent shards are in the primary shard directory
for (auto const shardIndex : {numShards * 2 - 1, numShards * 2})
{
BEAST_EXPECT(dirContains(primaryDir, shardIndex));
BEAST_EXPECT(!historicalDirsContains(shardIndex));
}

// All historical shards should be stored
// at historical paths
BEAST_EXPECT(historicalPathCount == (ledgerCount * 2) - 2);
// Confirm remaining shards are in the historical shard directories
for (auto shardIndex = 1; shardIndex < numShards * 2 - 1; ++shardIndex)
{
BEAST_EXPECT(!dirContains(primaryDir, shardIndex));
BEAST_EXPECT(historicalDirsContains(shardIndex));
}
}

Expand Down

0 comments on commit 4ed1a55

Please sign in to comment.