Skip to content

Commit

Permalink
Fix comments
Browse files Browse the repository at this point in the history
  • Loading branch information
cindyyan317 committed Dec 13, 2024
1 parent a29c189 commit 5e3a3ac
Show file tree
Hide file tree
Showing 55 changed files with 823 additions and 673 deletions.
10 changes: 2 additions & 8 deletions src/app/CliArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ CliArgs::parse(int argc, char const* argv[])
("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_rollback", po::value<std::string>(),"rollback the given migration")
;
// clang-format on
po::positional_options_description positional;
Expand All @@ -72,13 +71,8 @@ CliArgs::parse(int argc, char const* argv[])
if (parsed.count("migrate") != 0u) {
auto const opt = parsed["migrate"].as<std::string>();
if (opt == "status")
return Action{Action::Migrate{std::move(configPath), MigratorApplication::Cmd::status()}};
return Action{Action::Migrate{std::move(configPath), MigratorApplication::Cmd::migration(opt)}};
}

if (parsed.count("migrate_rollback") != 0u) {
auto const opt = parsed["migrate_rollback"].as<std::string>();
return Action{Action::Migrate{std::move(configPath), MigratorApplication::Cmd::rollback(opt)}};
return Action{Action::Migrate{std::move(configPath), MigrateSubCmd::status()}};

Check warning on line 74 in src/app/CliArgs.cpp

View check run for this annotation

Codecov / codecov/patch

src/app/CliArgs.cpp#L74

Added line #L74 was not covered by tests
return Action{Action::Migrate{std::move(configPath), MigrateSubCmd::migration(opt)}};
}

Check warning on line 76 in src/app/CliArgs.cpp

View check run for this annotation

Codecov / codecov/patch

src/app/CliArgs.cpp#L76

Added line #L76 was not covered by tests

return Action{Action::Run{.configPath = std::move(configPath), .useNgWebServer = parsed.count("ng-web-server") != 0}
Expand Down
2 changes: 1 addition & 1 deletion src/app/CliArgs.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class CliArgs {
/** @brief Migration action. */
struct Migrate {
std::string configPath;
MigratorApplication::Cmd subCmd;
MigrateSubCmd subCmd;
};

/**
Expand Down
19 changes: 14 additions & 5 deletions src/data/BackendInterface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
#include <string>
#include <thread>
#include <type_traits>
#include <unordered_set>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -550,13 +549,14 @@ class BackendInterface {
) const;

/**
* @brief Database-specific implementation of fetching the migrated features.
* @brief Fetches the status of migrator by name.
*
* @param migratorName The name of the migrator
* @param yield The coroutine context
* @return The name of migrated features on success; nullopt otherwise
* @return The status of the migrator if found; nullopt otherwise
*/
virtual std::optional<std::unordered_set<std::string>>
fetchMigratedFeatures(boost::asio::yield_context yield) const = 0;
virtual std::optional<std::string>
fetchMigratorStatus(std::string const& migratorName, boost::asio::yield_context yield) const = 0;

/**
* @brief Synchronously fetches the ledger range from DB.
Expand Down Expand Up @@ -683,6 +683,15 @@ class BackendInterface {
bool
finishWrites(std::uint32_t ledgerSequence);

/**
*@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
*/
virtual void
writeMigratorStatus(std::string const& migratorName, std::string const& status) = 0;

/**
* @return true if database is overwhelmed; false otherwise
*/
Expand Down
26 changes: 16 additions & 10 deletions src/data/CassandraBackend.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -838,26 +838,24 @@ class BasicCassandraBackend : public BackendInterface {
return results;
}

std::optional<std::unordered_set<std::string>>
fetchMigratedFeatures(boost::asio::yield_context yield) const override
std::optional<std::string>
fetchMigratorStatus(std::string const& migratorName, boost::asio::yield_context yield) const override
{
auto const res = executor_.read(yield, schema_->selectMigratedFeatures);
auto const res = executor_.read(yield, schema_->selectMigratorStatus, Text(migratorName));
if (not res) {
LOG(log_.error()) << "Could not fetch migrated features: " << res.error();
LOG(log_.error()) << "Could not fetch migrator status: " << res.error();
return {};
}

std::unordered_set<std::string> features;
auto const& results = res.value();
if (not results) {
LOG(log_.warn()) << "No migrated features in database";
return features;
return {};
}

for (auto [feature] : extract<std::string>(results))
features.insert(std::move(feature));
for (auto [statusString] : extract<std::string>(results))
return statusString;

return features;
return {};
}

void
Expand Down Expand Up @@ -987,6 +985,14 @@ class BasicCassandraBackend : public BackendInterface {
// probably was used in PG to start a transaction or smth.
}

void
writeMigratorStatus(std::string const& migratorName, std::string const& status) override
{
executor_.writeSync(
schema_->insertMigratorStatus, data::cassandra::Text{migratorName}, data::cassandra::Text(status)
);
}

bool
isTooBusy() const override
{
Expand Down
14 changes: 7 additions & 7 deletions src/data/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -263,14 +263,14 @@ CREATE TABLE clio.nf_token_transactions (

The `nf_token_transactions` table serves as the NFT counterpart to `account_tx`, inspired by the same motivations and fulfilling a similar role within this context. It drives the `nft_history` API.

### migrated_features
### migrator_status

```
CREATE TABLE clio.migrated_features (
status TEXT, # The migration status of the feature
feature_name TEXT, # The name of the feature
PRIMARY KEY (status, feature_name)
)
CREATE TABLE clio.migrator_status (
migrator_name TEXT, # The name of the migrator
status TEXT, # The status of the migrator
PRIMARY KEY (migrator_name)
)
```

The `migrated_features` table stores the status of the migration of features in this database. If a feature's status is `migrated`, it means this database has finished data migration for this feature. The feature name is the name of the feature that has been migrated. Now the status can only be `migrated`.
The `migrator_status` table stores the status of the migratior in this database. If a migrator's status is `migrated`, it means this database has finished data migration for this migrator.
27 changes: 19 additions & 8 deletions src/data/cassandra/Schema.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,12 +274,12 @@ class Schema {
R"(
CREATE TABLE IF NOT EXISTS {}
(
status TEXT,
feature_name TEXT,
PRIMARY KEY (status, feature_name)
migrator_name TEXT,
status TEXT,
PRIMARY KEY (migrator_name)
)
)",
qualifiedTableName(settingsProvider_.get(), "migrated_features")
qualifiedTableName(settingsProvider_.get(), "migrator_status")
));

return statements;
Expand Down Expand Up @@ -478,6 +478,17 @@ class Schema {
));
}();

PreparedStatement insertMigratorStatus = [this]() {
return handle_.get().prepare(fmt::format(
R"(
INSERT INTO {}
(migrator_name, status)
VALUES (?, ?)
)",
qualifiedTableName(settingsProvider_.get(), "migrator_status")
));
}();

//
// Select queries
//
Expand Down Expand Up @@ -781,14 +792,14 @@ class Schema {
));
}();

PreparedStatement selectMigratedFeatures = [this]() {
PreparedStatement selectMigratorStatus = [this]() {
return handle_.get().prepare(fmt::format(
R"(
SELECT feature_name
SELECT status
FROM {}
WHERE status = 'migrated'
WHERE migrator_name = ?
)",
qualifiedTableName(settingsProvider_.get(), "migrated_features")
qualifiedTableName(settingsProvider_.get(), "migrator_status")
));
}();
};
Expand Down
9 changes: 0 additions & 9 deletions src/data/cassandra/Types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,6 @@ struct Text {
explicit Text(std::string text) : text{std::move(text)}
{
}

/**
* @brief Construct a new Text object from char const* type
*
* @param text The text to wrap
*/
explicit Text(char const* text) : text{text}
{
}
};

class Handle;
Expand Down
4 changes: 2 additions & 2 deletions src/migration/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
add_library(clio_migration)

target_sources(
clio_migration PRIVATE MigrationApplication.cpp MigrationManagerFactory.cpp cassandra/ObjectsAdapter.cpp
cassandra/TransactionsAdapter.cpp
clio_migration PRIVATE MigrationApplication.cpp impl/MigrationManagerFactory.cpp MigratorStatus.cpp
cassandra/impl/ObjectsAdapter.cpp cassandra/impl/TransactionsAdapter.cpp
)

target_link_libraries(clio_migration PRIVATE clio_util clio_etl)
40 changes: 9 additions & 31 deletions src/migration/MigrationApplication.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@

#include "migration/MigrationApplication.hpp"

#include "migration/MigrationManagerFactory.hpp"
#include "migration/MigrationManagerInterface.hpp"
#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 All @@ -35,21 +36,20 @@

namespace app {

MigratorApplication::MigratorApplication(util::Config const& config, Cmd command) : cmd_(std::move(command))
MigratorApplication::MigratorApplication(util::Config const& config, MigrateSubCmd command) : cmd_(std::move(command))

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

View check run for this annotation

Codecov / codecov/patch

src/migration/MigrationApplication.cpp#L39

Added line #L39 was not covered by tests
{
PrometheusService::init(config);

migrationManager_ = migration::makeMigrationManager(config);
migrationManager_ = migration::impl::makeMigrationManager(config);
}

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

View check run for this annotation

Codecov / codecov/patch

src/migration/MigrationApplication.cpp#L44

Added line #L44 was not covered by tests

int
MigratorApplication::run()

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

View check run for this annotation

Codecov / codecov/patch

src/migration/MigrationApplication.cpp#L47

Added line #L47 was not covered by tests
{
return std::visit(
util::OverloadSet{
[this](Cmd::Status const&) { return printStatus(); },
[this](Cmd::Migration const& cmdBundle) { return migrate(cmdBundle.migratorName); },
[this](Cmd::Rollback const& cmdBundle) { return rollback(cmdBundle.migratorName); }
[this](MigrateSubCmd::Status const&) { return printStatus(); },
[this](MigrateSubCmd::Migration const& cmdBundle) { return migrate(cmdBundle.migratorName); }

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

View check run for this annotation

Codecov / codecov/patch

src/migration/MigrationApplication.cpp#L49-L52

Added lines #L49 - L52 were not covered by tests
},
cmd_.state
);

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

View check run for this annotation

Codecov / codecov/patch

src/migration/MigrationApplication.cpp#L55

Added line #L55 was not covered by tests
Expand All @@ -66,8 +66,8 @@ MigratorApplication::printStatus()
}

for (auto const& [migrator, status] : allMigratorsStatus) {
std::cout << "Migrator: " << migrator << " - "
<< (status == migration::MigratorStatus::Migrated ? "migrated" : "not migrated") << std::endl;
std::cout << "Migrator: " << migrator << " - " << migrationManager_->getMigratorDescriptionByName(migrator)
<< " - " << status.toString() << std::endl;
}
return EXIT_SUCCESS;
}

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

View check run for this annotation

Codecov / codecov/patch

src/migration/MigrationApplication.cpp#L72-L73

Added lines #L72 - L73 were not covered by tests
Expand All @@ -94,26 +94,4 @@ MigratorApplication::migrate(std::string const& migratorName)
return EXIT_SUCCESS;

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

View check run for this annotation

Codecov / codecov/patch

src/migration/MigrationApplication.cpp#L94

Added line #L94 was not covered by tests
}

int
MigratorApplication::rollback(std::string const& migratorName)
{
auto const status = migrationManager_->getMigratorStatusByName(migratorName);
if (status == migration::MigratorStatus::NotMigrated) {
std::cout << "Migrator " << migratorName << " yet not migrated" << std::endl;
printStatus();
return EXIT_SUCCESS;
}

if (status == migration::MigratorStatus::NotKnown) {
std::cout << "Migrator " << migratorName << " not found" << std::endl;
printStatus();
return EXIT_FAILURE;
}

std::cout << "Running rollback for " << migratorName << std::endl;
migrationManager_->runRollback(migratorName);
std::cout << "Rollback for " << migratorName << " finished" << std::endl;
return EXIT_SUCCESS;
}

} // namespace app
Loading

0 comments on commit 5e3a3ac

Please sign in to comment.