Skip to content

Commit

Permalink
fix comments
Browse files Browse the repository at this point in the history
  • Loading branch information
cindyyan317 committed Dec 16, 2024
1 parent 7e48eb0 commit 17f8d6b
Show file tree
Hide file tree
Showing 25 changed files with 112 additions and 100 deletions.
2 changes: 1 addition & 1 deletion src/app/CliArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ CliArgs::parse(int argc, char const* argv[])
("version,v", "print version and exit")
("conf,c", po::value<std::string>()->default_value(defaultConfigPath), "configuration file")
("ng-web-server,w", "Use ng-web-server")
("migrate", po::value<std::string>(),"start migration helper")
("migrate", po::value<std::string>(), "start migration helper")
;
// clang-format on
po::positional_options_description positional;
Expand Down
6 changes: 3 additions & 3 deletions src/data/BackendInterface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -684,10 +684,10 @@ class BackendInterface {
finishWrites(std::uint32_t ledgerSequence);

/**
*@brief Mark the migration status of a migrator as Migrated in the database
* @brief Mark the migration status of a migrator as Migrated in the database
*
*@param migratorName The name of the migrator
*@param status The status to set
* @param migratorName The name of the migrator
* @param status The status to set
*/
virtual void
writeMigratorStatus(std::string const& migratorName, std::string const& status) = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/data/cassandra/impl/Statement.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class Statement : public ManagedObject<CassStatement> {
auto const rc = bindBytes(reinterpret_cast<unsigned char const*>(value.data()), value.size());
throwErrorIfNeeded(rc, "Bind string (as bytes)");
} else if constexpr (std::is_convertible_v<DecayedType, Text>) {
auto const rc = cass_statement_bind_string(*this, idx, value.text.c_str());
auto const rc = cass_statement_bind_string_n(*this, idx, value.text.c_str(), value.text.size());
throwErrorIfNeeded(rc, "Bind string (as TEXT)");
} else if constexpr (std::is_same_v<DecayedType, UintTupleType> ||
std::is_same_v<DecayedType, UintByteTupleType>) {
Expand Down
7 changes: 3 additions & 4 deletions src/migration/MigrationApplication.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

#include "migration/MigratiorStatus.hpp"
#include "migration/impl/MigrationManagerFactory.hpp"
#include "migration/impl/MigrationManagerInterface.hpp"
#include "util/OverloadSet.hpp"
#include "util/config/Config.hpp"
#include "util/log/Logger.hpp"
Expand Down Expand Up @@ -64,13 +63,13 @@ int
MigratorApplication::printStatus()

Check warning on line 63 in src/migration/MigrationApplication.cpp

View check run for this annotation

Codecov / codecov/patch

src/migration/MigrationApplication.cpp#L63

Added line #L63 was not covered by tests
{
std::cout << "Current Migration Status:" << std::endl;
auto const allMigratorsStatus = migrationManager_->allMigratorsStatus();
auto const allMigratorsStatusPairs = migrationManager_->allMigratorsStatusPairs();

if (allMigratorsStatus.empty()) {
if (allMigratorsStatusPairs.empty()) {
std::cout << "No migrator found" << std::endl;
}

for (auto const& [migrator, status] : allMigratorsStatus) {
for (auto const& [migrator, status] : allMigratorsStatusPairs) {
std::cout << "Migrator: " << migrator << " - " << migrationManager_->getMigratorDescriptionByName(migrator)
<< " - " << status.toString() << std::endl;
}
Expand Down
4 changes: 2 additions & 2 deletions src/migration/MigrationApplication.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

#pragma once

#include "migration/impl/MigrationManagerInterface.hpp"
#include "migration/MigrationManagerInterface.hpp"
#include "util/config/Config.hpp"

#include <memory>
Expand Down Expand Up @@ -74,7 +74,7 @@ struct MigrateSubCmd {
*/
class MigratorApplication {
std::string option_;
std::shared_ptr<migration::impl::MigrationManagerInterface> migrationManager_;
std::shared_ptr<migration::MigrationManagerInterface> migrationManager_;
MigrateSubCmd cmd_;

public:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
#include <tuple>
#include <vector>

namespace migration::impl {
namespace migration {

/**
* @brief The interface for the migration manager. This interface is tend to be implemented for specific database. The
Expand All @@ -45,7 +45,7 @@ struct MigrationManagerInterface {
* @return A vector of tuple, the first element is the migrator's name, the second element is the status of the
*/
virtual std::vector<std::tuple<std::string, MigratorStatus>>
allMigratorsStatus() const = 0;
allMigratorsStatusPairs() const = 0;

/**
* @brief Get all registered migrators' names
Expand Down Expand Up @@ -74,4 +74,4 @@ struct MigrationManagerInterface {
getMigratorDescriptionByName(std::string const& name) const = 0;
};

} // namespace migration::impl
} // namespace migration
5 changes: 2 additions & 3 deletions src/migration/MigratiorStatus.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class MigratorStatus {
/**
* @brief The status of a migrator
*/
enum Status { Migrated, NotMigrated, NotKnown, Count };
enum Status { Migrated, NotMigrated, NotKnown, COUNT };

/**
* @brief Construct a new Migrator Status object with the given status
Expand Down Expand Up @@ -79,8 +79,7 @@ class MigratorStatus {
fromString(std::string const& statusStr);

private:
// Private static array for mapping
static constexpr std::array<char const*, static_cast<size_t>(Count)> statusStrMap = {
static constexpr std::array<char const*, static_cast<size_t>(COUNT)> statusStrMap = {
"Migrated",
"NotMigrated",
"NotKnown"
Expand Down
2 changes: 1 addition & 1 deletion src/migration/MigratorStatus.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ MigratorStatus::fromString(std::string const& statusStr)
return MigratorStatus(static_cast<Status>(i));
}
}
return MigratorStatus(NotMigrated);
return MigratorStatus(Status::NotMigrated);
}

} // namespace migration
4 changes: 2 additions & 2 deletions src/migration/cassandra/CassandraMigrationManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ namespace migration::cassandra {
template <typename BackendType>
using CassandraSupportedMigrators = migration::impl::MigratorsRegister<BackendType>;

/// Register with MigrationBackend which proceeds the migration
// Register with MigrationBackend which proceeds the migration
using MigrationProcesser = CassandraSupportedMigrators<CassandraMigrationBackend>;

/// The Cassandra migration manager
// The Cassandra migration manager
using CassandraMigrationManager = migration::impl::MigrationManagerBase<MigrationProcesser>;
} // namespace migration::cassandra
4 changes: 2 additions & 2 deletions src/migration/cassandra/impl/CassandraMigrationSchema.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ class CassandraMigrationSchema {
static auto prepared = handler.prepare(fmt::format(
R"(
INSERT INTO {}
(migrator_name, status)
VALUES (?, ?)
(migrator_name, status)
VALUES (?, ?)
)",
data::cassandra::qualifiedTableName<SettingsProviderType>(settingsProvider_.get(), "migrator_status")
));
Expand Down
11 changes: 9 additions & 2 deletions src/migration/cassandra/impl/FullTableScanner.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,20 @@ class FullTableScanner {
* @param reader The table adapter
*/
template <typename ExecutionContextType = util::async::CoroExecutionContext>
FullTableScanner(std::uint32_t ctxThreadsNum, std::uint32_t workersNum, TableAdapter&& reader)
FullTableScanner(
std::uint32_t ctxThreadsNum,
std::uint32_t workersNum,
std::uint32_t cursorsPerWorker,
TableAdapter&& reader
)
: ctx_(ExecutionContextType(ctxThreadsNum))
, cursorsNum_(workersNum * CursorNumMultiplier)
, cursorsNum_(workersNum * cursorsPerWorker)
, queue_{cursorsNum_}
, reader_{std::move(reader)}
{
ASSERT(workersNum > 0, "workersNum for full table scanner must be greater than 0");
ASSERT(cursorsPerWorker > 0, "cursorsPerWorker for full table scanner must be greater than 0");

auto const cursors = TokenRangesProvider{cursorsNum_}.getRanges();
std::ranges::for_each(cursors, [this](auto const& cursor) { queue_.push(cursor); });
load(workersNum);
Expand Down
4 changes: 2 additions & 2 deletions src/migration/impl/MigrationManagerBase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

#pragma once

#include "migration/MigrationManagerInterface.hpp"
#include "migration/MigratiorStatus.hpp"
#include "migration/impl/MigrationManagerInterface.hpp"
#include "util/config/Config.hpp"

#include <memory>
Expand Down Expand Up @@ -76,7 +76,7 @@ class MigrationManagerBase : public MigrationManagerInterface {
* migrator
*/
std::vector<std::tuple<std::string, MigratorStatus>>
allMigratorsStatus() const override
allMigratorsStatusPairs() const override
{
return migrators_.getMigratorsStatus();
}
Expand Down
4 changes: 2 additions & 2 deletions src/migration/impl/MigrationManagerFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
#include "migration/impl/MigrationManagerFactory.hpp"

#include "data/cassandra/SettingsProvider.hpp"
#include "migration/MigrationManagerInterface.hpp"
#include "migration/cassandra/CassandraMigrationBackend.hpp"
#include "migration/cassandra/CassandraMigrationManager.hpp"
#include "migration/impl/MigrationManagerInterface.hpp"
#include "util/log/Logger.hpp"

#include <boost/algorithm/string/predicate.hpp>
Expand All @@ -33,7 +33,7 @@

namespace migration::impl {

std::expected<std::shared_ptr<impl::MigrationManagerInterface>, std::string>
std::expected<std::shared_ptr<MigrationManagerInterface>, std::string>
makeMigrationManager(util::Config const& config)
{
static util::Logger const log{"Migration"};
Expand Down
4 changes: 2 additions & 2 deletions src/migration/impl/MigrationManagerFactory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

#pragma once

#include "migration/impl/MigrationManagerInterface.hpp"
#include "migration/MigrationManagerInterface.hpp"
#include "util/config/Config.hpp"

#include <expected>
Expand All @@ -35,7 +35,7 @@ namespace migration::impl {
* other migration specific configurations
* @return A shared pointer to the MigrationManagerInterface if the creation was successful, otherwise an error message
*/
std::expected<std::shared_ptr<impl::MigrationManagerInterface>, std::string>
std::expected<std::shared_ptr<MigrationManagerInterface>, std::string>
makeMigrationManager(util::Config const& config);

} // namespace migration::impl
4 changes: 2 additions & 2 deletions src/migration/impl/MigratorsRegister.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class MigratorsRegister {
if (name == Migrator::name) {
LOG(log_.info()) << "Running migration: " << name;
Migrator::runMigration(backend_, config);
backend_->writeMigratorStatus(name, MigratorStatus(MigratorStatus::Status::Migrated).toString());
backend_->writeMigratorStatus(name, MigratorStatus(MigratorStatus::Migrated).toString());
LOG(log_.info()) << "Finished migration: " << name;
}
}
Expand Down Expand Up @@ -122,7 +122,7 @@ class MigratorsRegister {

std::vector<std::tuple<std::string, MigratorStatus>> status;

std::transform(fullList.begin(), fullList.end(), std::back_inserter(status), [&](auto const& migratorName) {
std::ranges::transform(fullList, std::back_inserter(status), [&](auto const& migratorName) {
auto const migratorNameStr = std::string(migratorName);
return std::make_tuple(migratorNameStr, getMigratorStatus(migratorNameStr));
});
Expand Down
13 changes: 5 additions & 8 deletions src/util/Concepts.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,11 @@ constexpr bool
hasNoDuplicateNames()
{
constexpr std::array<std::string_view, sizeof...(Types)> names = {Types::name...};
for (std::size_t i = 0; i < names.size(); ++i) {
for (std::size_t j = i + 1; j < names.size(); ++j) {
if (names[i] == names[j]) {
return false;
}
}
}
return true;
return !std::ranges::any_of(names, [&](std::string_view const& name1) {
return std::ranges::any_of(names, [&](std::string_view const& name2) {
return &name1 != &name2 && name1 == name2; // Ensure different elements are compared
});
});
}

} // namespace util
2 changes: 1 addition & 1 deletion tests/common/util/MockMigrationBackendFixture.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ struct MockMigrationBackendTestBase : virtual public NoLoggerFixture {
};

protected:
BackendProxy backend;
BackendProxy backend_;
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,15 @@
#include "data/DBHelpers.hpp"
#include "data/cassandra/Handle.hpp"
#include "data/cassandra/SettingsProvider.hpp"
#include "migration/MigrationManagerInterface.hpp"
#include "migration/MigratiorStatus.hpp"
#include "migration/cassandra/CassandraMigrationTestBackend.hpp"
#include "migration/cassandra/DBRawData.hpp"
#include "migration/cassandra/ExampleDropTableMigrator.hpp"
#include "migration/cassandra/ExampleLedgerMigrator.hpp"
#include "migration/cassandra/ExampleObjectsMigrator.hpp"
#include "migration/cassandra/ExampleTransactionsMigrator.hpp"
#include "migration/impl/MigrationManagerBase.hpp"
#include "migration/impl/MigrationManagerInterface.hpp"
#include "migration/impl/MigratorsRegister.hpp"
#include "util/CassandraDBHelper.hpp"
#include "util/LoggerFixtures.hpp"
Expand Down Expand Up @@ -67,7 +68,7 @@ using CassandraSupportedTestMigrators = migration::impl::MigratorsRegister<
using CassandraMigrationTestManager = migration::impl::MigrationManagerBase<CassandraSupportedTestMigrators>;

namespace {
std::pair<std::shared_ptr<migration::impl::MigrationManagerInterface>, std::shared_ptr<CassandraMigrationTestBackend>>
std::pair<std::shared_ptr<migration::MigrationManagerInterface>, std::shared_ptr<CassandraMigrationTestBackend>>
make_MigrationTestManagerAndBackend(util::Config const& config)
{
auto const cfg = config.section("database.cassandra");
Expand Down Expand Up @@ -105,7 +106,7 @@ class MigrationCassandraSimpleTest : public WithPrometheus, public NoLoggerFixtu
TestGlobals::instance().backendKeyspace
))};

std::shared_ptr<migration::impl::MigrationManagerInterface> testMigrationManager;
std::shared_ptr<migration::MigrationManagerInterface> testMigrationManager;
std::shared_ptr<CassandraMigrationTestBackend> testMigrationBackend;

MigrationCassandraSimpleTest()
Expand Down Expand Up @@ -146,7 +147,7 @@ TEST_F(MigrationCassandraManagerCleanDBTest, GetAllMigratorNames)

TEST_F(MigrationCassandraManagerCleanDBTest, AllMigratorStatusBeforeAnyMigration)
{
auto const status = testMigrationManager->allMigratorsStatus();
auto const status = testMigrationManager->allMigratorsStatusPairs();
EXPECT_EQ(status.size(), 4);
EXPECT_EQ(std::get<1>(status[0]), MigratorStatus::NotMigrated);
EXPECT_EQ(std::get<1>(status[1]), MigratorStatus::NotMigrated);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,24 +316,24 @@ class CassandraMigrationTestBackend : public migration::cassandra::CassandraMigr

statements.emplace_back(fmt::format(
R"(
CREATE TABLE IF NOT EXISTS {}
(
CREATE TABLE IF NOT EXISTS {}
(
hash blob,
tx_type text,
PRIMARY KEY (hash)
)
tx_type text,
PRIMARY KEY (hash)
)
)",
data::cassandra::qualifiedTableName(settingsProvider_, "tx_index_example")
));

statements.emplace_back(fmt::format(
R"(
CREATE TABLE IF NOT EXISTS {}
(
CREATE TABLE IF NOT EXISTS {}
(
sequence bigint,
account_hash blob,
PRIMARY KEY (sequence)
)
account_hash blob,
PRIMARY KEY (sequence)
)
)",
data::cassandra::qualifiedTableName(settingsProvider_, "ledger_example")
));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,8 @@

#include <atomic>
#include <cstdint>
#include <iostream>
#include <memory>
#include <optional>
#include <ostream>
#include <unordered_set>

std::atomic_int64_t ExampleObjectsMigrator::count;
Expand All @@ -41,14 +39,15 @@ std::atomic_int64_t ExampleObjectsMigrator::accountCount;
void
ExampleObjectsMigrator::runMigration(std::shared_ptr<Backend> const& backend, util::Config const& config)
{
// Do something
auto const ctxFullScanThreads = config.valueOr<std::uint32_t>("full_scan_threads", 2);
auto const jobsFullScan = config.valueOr<std::uint32_t>("jobs_full_scan", 4);
auto const cursorPerJobsFullScan = config.valueOr<std::uint32_t>("cursors_per_job", 100);

std::unordered_set<ripple::uint256> idx;
migration::cassandra::impl::ObjectsScanner scaner(
ctxFullScanThreads,
jobsFullScan,
cursorPerJobsFullScan,
migration::cassandra::impl::ObjectsAdapter(
backend,
[&](std::uint32_t, std::optional<ripple::SLE> sle) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,14 @@ ExampleTransactionsMigrator::runMigration(std::shared_ptr<Backend> const& backen
{
auto const ctxFullScanThreads = config.valueOr<std::uint32_t>("full_scan_threads", 2);
auto const jobsFullScan = config.valueOr<std::uint32_t>("jobs_full_scan", 4);
auto const cursorPerJobsFullScan = config.valueOr<std::uint32_t>("cursors_per_job", 100);

std::unordered_set<std::string> hashSet;
std::mutex mtx; // protect hashSet
migration::cassandra::impl::TransactionsScanner scaner(
ctxFullScanThreads,
jobsFullScan,
cursorPerJobsFullScan,
migration::cassandra::impl::TransactionsAdapter(
backend,
[&](ripple::STTx const& tx, ripple::TxMeta const&) {
Expand Down
Loading

0 comments on commit 17f8d6b

Please sign in to comment.