From ea73b47ebee43209ba460ae45df6816f6c1877b8 Mon Sep 17 00:00:00 2001 From: ninsmiracle <110282526+ninsmiracle@users.noreply.github.com> Date: Wed, 10 Apr 2024 15:36:36 +0800 Subject: [PATCH 01/10] feat(backup): 1. remove pervious implementation (#1956) This pr removes some previous backup implementation, because the enhancement code is strongly different with the current one. Besides, this pr also disable related unit tests and function tests. --- src/meta/meta_backup_service.cpp | 8 +++- src/meta/test/meta_http_service_test.cpp | 8 ++-- src/replica/backup/replica_backup_manager.cpp | 3 +- src/replica/replica.h | 1 + src/replica/replica_stub.cpp | 1 + src/replica/test/mock_utils.h | 16 ++++++- src/replica/test/replica_test.cpp | 43 ++----------------- 7 files changed, 32 insertions(+), 48 deletions(-) diff --git a/src/meta/meta_backup_service.cpp b/src/meta/meta_backup_service.cpp index fa6ee4bc8b..5586278a86 100644 --- a/src/meta/meta_backup_service.cpp +++ b/src/meta/meta_backup_service.cpp @@ -37,9 +37,13 @@ #include "meta_backup_service.h" #include "meta_service.h" #include "runtime/api_layer1.h" +#include "runtime/rpc/rpc_host_port.h" +#include "runtime/rpc/rpc_holder.h" +#include "runtime/rpc/rpc_message.h" +#include "runtime/rpc/serialization.h" #include "security/access_controller.h" -#include "task/async_calls.h" -#include "task/task_code.h" +#include "runtime/task/async_calls.h" +#include "runtime/task/task_code.h" #include "server_state.h" #include "utils/autoref_ptr.h" #include "utils/blob.h" diff --git a/src/meta/test/meta_http_service_test.cpp b/src/meta/test/meta_http_service_test.cpp index c70fc7fda8..0747ee063f 100644 --- a/src/meta/test/meta_http_service_test.cpp +++ b/src/meta/test/meta_http_service_test.cpp @@ -39,10 +39,10 @@ #include "meta/meta_state_service.h" #include "meta_service_test_app.h" #include "meta_test_base.h" -#include "rpc/rpc_holder.h" -#include "rpc/rpc_message.h" -#include "task/task.h" -#include "task/task_code.h" +#include "runtime/rpc/rpc_holder.h" +#include "runtime/rpc/rpc_message.h" +#include "runtime/task/task.h" +#include "runtime/task/task_code.h" #include "utils/autoref_ptr.h" #include "utils/blob.h" #include "utils/chrono_literals.h" diff --git a/src/replica/backup/replica_backup_manager.cpp b/src/replica/backup/replica_backup_manager.cpp index 70d8d12d45..31ac3426ee 100644 --- a/src/replica/backup/replica_backup_manager.cpp +++ b/src/replica/backup/replica_backup_manager.cpp @@ -27,6 +27,7 @@ #include #include "backup_types.h" +#include "cold_backup_context.h" #include "common/gpid.h" #include "common/replication.codes.h" #include "dsn.layer2_types.h" @@ -35,13 +36,13 @@ #include "replica/replica_context.h" #include "replica/replication_app_base.h" #include "runtime/api_layer1.h" +#include "runtime/task/async_calls.h" #include "utils/autoref_ptr.h" #include "utils/filesystem.h" #include "utils/flags.h" #include "utils/fmt_logging.h" #include "utils/strings.h" #include "utils/thread_access_checker.h" -#include "utils/metrics.h" METRIC_DEFINE_gauge_int64(replica, backup_running_count, diff --git a/src/replica/replica.h b/src/replica/replica.h index 4cd725bccb..6340cb9073 100644 --- a/src/replica/replica.h +++ b/src/replica/replica.h @@ -108,6 +108,7 @@ class replication_app_base; class replication_options; struct dir_node; + namespace test { class test_checker; } diff --git a/src/replica/replica_stub.cpp b/src/replica/replica_stub.cpp index 4dc2ad719a..00cf4dcf17 100644 --- a/src/replica/replica_stub.cpp +++ b/src/replica/replica_stub.cpp @@ -44,6 +44,7 @@ #include #include +#include "backup/replica_backup_server.h" #include "absl/strings/string_view.h" #include "bulk_load/replica_bulk_loader.h" #include "common/backup_common.h" diff --git a/src/replica/test/mock_utils.h b/src/replica/test/mock_utils.h index 04874f6dd1..e89e6c7f98 100644 --- a/src/replica/test/mock_utils.h +++ b/src/replica/test/mock_utils.h @@ -33,7 +33,8 @@ #include "replica/replica.h" #include "replica/replica_stub.h" -#include "rpc/rpc_host_port.h" +#include "replica/backup/cold_backup_context.h" +#include "runtime/rpc/rpc_host_port.h" DSN_DECLARE_int32(log_private_file_size_mb); @@ -206,6 +207,19 @@ class mock_replica : public replica _primary_states.secondary_bulk_load_states.size() == 0); } + // mock cold backup related function. + void generate_backup_checkpoint(cold_backup_context_ptr backup_context) override + { + if (backup_context->status() != ColdBackupCheckpointing) { + LOG_INFO("{}: ignore generating backup checkpoint because backup_status = {}", + backup_context->name, + cold_backup_status_to_string(backup_context->status())); + backup_context->ignore_checkpoint(); + return; + } + backup_context->complete_checkpoint(); + } + void update_last_applied_decree(decree decree) { dynamic_cast(_app.get())->set_last_applied_decree(decree); diff --git a/src/replica/test/replica_test.cpp b/src/replica/test/replica_test.cpp index 6038df8c9b..4e4836f795 100644 --- a/src/replica/test/replica_test.cpp +++ b/src/replica/test/replica_test.cpp @@ -154,25 +154,6 @@ class replica_test : public replica_test_base bool is_checkpointing() { return _mock_replica->_is_manual_emergency_checkpointing; } - void test_trigger_manual_emergency_checkpoint(const decree min_checkpoint_decree, - const error_code expected_err, - std::function callback = {}) - { - dsn::utils::notify_event op_completed; - _mock_replica->async_trigger_manual_emergency_checkpoint( - min_checkpoint_decree, 0, [&](error_code actual_err) { - ASSERT_EQ(expected_err, actual_err); - - if (callback) { - callback(); - } - - op_completed.notify(); - }); - - op_completed.wait(); - } - bool has_gpid(gpid &pid) const { for (const auto &node : stub->_fs_manager.get_dir_nodes()) { @@ -367,28 +348,10 @@ TEST_P(replica_test, update_allow_ingest_behind_test) } } -TEST_P(replica_test, test_trigger_manual_emergency_checkpoint) +TEST_F(replica_test, test_trigger_manual_emergency_checkpoint) { - // There is only one replica for the unit test. - PRESERVE_FLAG(mutation_2pc_min_replica_count); - FLAGS_mutation_2pc_min_replica_count = 1; - - // Initially the mutation log is empty. - ASSERT_EQ(0, _mock_replica->last_applied_decree()); - ASSERT_EQ(0, _mock_replica->last_durable_decree()); - - // Commit at least an empty write to make the replica become non-empty. - _mock_replica->update_expect_last_durable_decree(1); - test_trigger_manual_emergency_checkpoint(1, ERR_OK); - _mock_replica->tracker()->wait_outstanding_tasks(); - - // Committing multiple empty writes (retry multiple times) might make the last - // applied decree greater than 1. - ASSERT_LE(1, _mock_replica->last_applied_decree()); - ASSERT_EQ(1, _mock_replica->last_durable_decree()); - - test_trigger_manual_emergency_checkpoint( - 100, ERR_OK, [this]() { ASSERT_TRUE(is_checkpointing()); }); + ASSERT_EQ(_mock_replica->trigger_manual_emergency_checkpoint(100), ERR_OK); + ASSERT_TRUE(is_checkpointing()); _mock_replica->update_last_durable_decree(100); // There's no need to trigger checkpoint since min_checkpoint_decree <= last_durable_decree. From f591b2a439cfd1cbc1139ce93ac5ecb0b8e9b01d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=83=AD=E5=AE=81=E6=B7=B1?= Date: Mon, 21 Oct 2024 15:56:34 +0800 Subject: [PATCH 02/10] make code can be compiled --- src/meta/meta_backup_service.cpp | 8 +--- src/meta/test/meta_http_service_test.cpp | 8 ++-- src/replica/backup/replica_backup_manager.cpp | 2 - src/replica/replica_stub.cpp | 1 - src/replica/test/mock_utils.h | 16 +------ src/replica/test/replica_test.cpp | 43 +++++++++++++++++-- 6 files changed, 47 insertions(+), 31 deletions(-) diff --git a/src/meta/meta_backup_service.cpp b/src/meta/meta_backup_service.cpp index 5586278a86..fa6ee4bc8b 100644 --- a/src/meta/meta_backup_service.cpp +++ b/src/meta/meta_backup_service.cpp @@ -37,13 +37,9 @@ #include "meta_backup_service.h" #include "meta_service.h" #include "runtime/api_layer1.h" -#include "runtime/rpc/rpc_host_port.h" -#include "runtime/rpc/rpc_holder.h" -#include "runtime/rpc/rpc_message.h" -#include "runtime/rpc/serialization.h" #include "security/access_controller.h" -#include "runtime/task/async_calls.h" -#include "runtime/task/task_code.h" +#include "task/async_calls.h" +#include "task/task_code.h" #include "server_state.h" #include "utils/autoref_ptr.h" #include "utils/blob.h" diff --git a/src/meta/test/meta_http_service_test.cpp b/src/meta/test/meta_http_service_test.cpp index 0747ee063f..c70fc7fda8 100644 --- a/src/meta/test/meta_http_service_test.cpp +++ b/src/meta/test/meta_http_service_test.cpp @@ -39,10 +39,10 @@ #include "meta/meta_state_service.h" #include "meta_service_test_app.h" #include "meta_test_base.h" -#include "runtime/rpc/rpc_holder.h" -#include "runtime/rpc/rpc_message.h" -#include "runtime/task/task.h" -#include "runtime/task/task_code.h" +#include "rpc/rpc_holder.h" +#include "rpc/rpc_message.h" +#include "task/task.h" +#include "task/task_code.h" #include "utils/autoref_ptr.h" #include "utils/blob.h" #include "utils/chrono_literals.h" diff --git a/src/replica/backup/replica_backup_manager.cpp b/src/replica/backup/replica_backup_manager.cpp index 31ac3426ee..655a197034 100644 --- a/src/replica/backup/replica_backup_manager.cpp +++ b/src/replica/backup/replica_backup_manager.cpp @@ -27,7 +27,6 @@ #include #include "backup_types.h" -#include "cold_backup_context.h" #include "common/gpid.h" #include "common/replication.codes.h" #include "dsn.layer2_types.h" @@ -36,7 +35,6 @@ #include "replica/replica_context.h" #include "replica/replication_app_base.h" #include "runtime/api_layer1.h" -#include "runtime/task/async_calls.h" #include "utils/autoref_ptr.h" #include "utils/filesystem.h" #include "utils/flags.h" diff --git a/src/replica/replica_stub.cpp b/src/replica/replica_stub.cpp index 00cf4dcf17..4dc2ad719a 100644 --- a/src/replica/replica_stub.cpp +++ b/src/replica/replica_stub.cpp @@ -44,7 +44,6 @@ #include #include -#include "backup/replica_backup_server.h" #include "absl/strings/string_view.h" #include "bulk_load/replica_bulk_loader.h" #include "common/backup_common.h" diff --git a/src/replica/test/mock_utils.h b/src/replica/test/mock_utils.h index e89e6c7f98..04874f6dd1 100644 --- a/src/replica/test/mock_utils.h +++ b/src/replica/test/mock_utils.h @@ -33,8 +33,7 @@ #include "replica/replica.h" #include "replica/replica_stub.h" -#include "replica/backup/cold_backup_context.h" -#include "runtime/rpc/rpc_host_port.h" +#include "rpc/rpc_host_port.h" DSN_DECLARE_int32(log_private_file_size_mb); @@ -207,19 +206,6 @@ class mock_replica : public replica _primary_states.secondary_bulk_load_states.size() == 0); } - // mock cold backup related function. - void generate_backup_checkpoint(cold_backup_context_ptr backup_context) override - { - if (backup_context->status() != ColdBackupCheckpointing) { - LOG_INFO("{}: ignore generating backup checkpoint because backup_status = {}", - backup_context->name, - cold_backup_status_to_string(backup_context->status())); - backup_context->ignore_checkpoint(); - return; - } - backup_context->complete_checkpoint(); - } - void update_last_applied_decree(decree decree) { dynamic_cast(_app.get())->set_last_applied_decree(decree); diff --git a/src/replica/test/replica_test.cpp b/src/replica/test/replica_test.cpp index 4e4836f795..6038df8c9b 100644 --- a/src/replica/test/replica_test.cpp +++ b/src/replica/test/replica_test.cpp @@ -154,6 +154,25 @@ class replica_test : public replica_test_base bool is_checkpointing() { return _mock_replica->_is_manual_emergency_checkpointing; } + void test_trigger_manual_emergency_checkpoint(const decree min_checkpoint_decree, + const error_code expected_err, + std::function callback = {}) + { + dsn::utils::notify_event op_completed; + _mock_replica->async_trigger_manual_emergency_checkpoint( + min_checkpoint_decree, 0, [&](error_code actual_err) { + ASSERT_EQ(expected_err, actual_err); + + if (callback) { + callback(); + } + + op_completed.notify(); + }); + + op_completed.wait(); + } + bool has_gpid(gpid &pid) const { for (const auto &node : stub->_fs_manager.get_dir_nodes()) { @@ -348,10 +367,28 @@ TEST_P(replica_test, update_allow_ingest_behind_test) } } -TEST_F(replica_test, test_trigger_manual_emergency_checkpoint) +TEST_P(replica_test, test_trigger_manual_emergency_checkpoint) { - ASSERT_EQ(_mock_replica->trigger_manual_emergency_checkpoint(100), ERR_OK); - ASSERT_TRUE(is_checkpointing()); + // There is only one replica for the unit test. + PRESERVE_FLAG(mutation_2pc_min_replica_count); + FLAGS_mutation_2pc_min_replica_count = 1; + + // Initially the mutation log is empty. + ASSERT_EQ(0, _mock_replica->last_applied_decree()); + ASSERT_EQ(0, _mock_replica->last_durable_decree()); + + // Commit at least an empty write to make the replica become non-empty. + _mock_replica->update_expect_last_durable_decree(1); + test_trigger_manual_emergency_checkpoint(1, ERR_OK); + _mock_replica->tracker()->wait_outstanding_tasks(); + + // Committing multiple empty writes (retry multiple times) might make the last + // applied decree greater than 1. + ASSERT_LE(1, _mock_replica->last_applied_decree()); + ASSERT_EQ(1, _mock_replica->last_durable_decree()); + + test_trigger_manual_emergency_checkpoint( + 100, ERR_OK, [this]() { ASSERT_TRUE(is_checkpointing()); }); _mock_replica->update_last_durable_decree(100); // There's no need to trigger checkpoint since min_checkpoint_decree <= last_durable_decree. From 187338966ad63e60fb618ad359bbf6e10ecca141 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=83=AD=E5=AE=81=E6=B7=B1?= Date: Mon, 21 Oct 2024 16:21:46 +0800 Subject: [PATCH 03/10] format code --- src/replica/replica.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/replica/replica.h b/src/replica/replica.h index 6340cb9073..4cd725bccb 100644 --- a/src/replica/replica.h +++ b/src/replica/replica.h @@ -108,7 +108,6 @@ class replication_app_base; class replication_options; struct dir_node; - namespace test { class test_checker; } From 99eb184a221c0cc1971ed084c40b8b8d4db5afa6 Mon Sep 17 00:00:00 2001 From: ninsmiracle Date: Tue, 22 Oct 2024 16:35:00 +0800 Subject: [PATCH 04/10] renamed src/meta/backup_engine to src/meta/meta_backup_engine --- src/meta/{backup_engine.cpp => meta_backup_engine.cpp} | 0 src/meta/{backup_engine.h => meta_backup_engine.h} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename src/meta/{backup_engine.cpp => meta_backup_engine.cpp} (100%) rename src/meta/{backup_engine.h => meta_backup_engine.h} (100%) diff --git a/src/meta/backup_engine.cpp b/src/meta/meta_backup_engine.cpp similarity index 100% rename from src/meta/backup_engine.cpp rename to src/meta/meta_backup_engine.cpp diff --git a/src/meta/backup_engine.h b/src/meta/meta_backup_engine.h similarity index 100% rename from src/meta/backup_engine.h rename to src/meta/meta_backup_engine.h From 5e2feab606d4c762652018867c052723a5e3940d Mon Sep 17 00:00:00 2001 From: ninsmiracle Date: Tue, 22 Oct 2024 17:03:37 +0800 Subject: [PATCH 05/10] update backup_time struct --- idl/backup.thrift | 24 ++++++++++++++++++------ src/common/json_helper.h | 14 ++++++++++++++ src/common/replication_enums.h | 12 ++++++++++++ 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/idl/backup.thrift b/idl/backup.thrift index a73fa12e12..c4165606ab 100644 --- a/idl/backup.thrift +++ b/idl/backup.thrift @@ -184,16 +184,28 @@ struct start_backup_app_response 3:optional i64 backup_id; } +enum backup_status +{ + UNINITIALIZED, + CHECKPOINTING, + CHECKPOINTED, + UPLOADING, + SUCCEED, + FAILED, + CANCELED +} + struct backup_item { 1:i64 backup_id; - 2:string app_name; - 3:string backup_provider_type; + 2:i32 app_id; + 3:string app_name; + 4:string backup_provider_type; // user specified backup_path. - 4:string backup_path; - 5:i64 start_time_ms; - 6:i64 end_time_ms; - 7:bool is_backup_failed; + 5:string backup_path; + 6:i64 start_time_ms; + 7:i64 end_time_ms; + 8:backup_status status; } struct query_backup_status_request diff --git a/src/common/json_helper.h b/src/common/json_helper.h index d56bcecba6..c13f02965d 100644 --- a/src/common/json_helper.h +++ b/src/common/json_helper.h @@ -402,6 +402,8 @@ ENUM_TYPE_SERIALIZATION(dsn::replication::partition_status::type, ENUM_TYPE_SERIALIZATION(dsn::app_status::type, dsn::app_status::AS_INVALID) ENUM_TYPE_SERIALIZATION(dsn::replication::bulk_load_status::type, dsn::replication::bulk_load_status::BLS_INVALID) +ENUM_TYPE_SERIALIZATION(dsn::replication::backup_status::type, + dsn::replication::backup_status::UNINITIALIZED) // json serialization for gpid, we treat it as string: "app_id.partition_id" inline void json_encode(JsonWriter &out, const dsn::gpid &pid) @@ -456,6 +458,8 @@ inline void json_encode(JsonWriter &out, const dsn::replication::file_meta &f_me inline bool json_decode(const JsonObject &in, dsn::replication::file_meta &f_meta); inline void json_encode(JsonWriter &out, const dsn::replication::bulk_load_metadata &metadata); inline bool json_decode(const JsonObject &in, dsn::replication::bulk_load_metadata &metadata); +inline void json_encode(JsonWriter &out, const dsn::replication::backup_item &backup_item); +inline bool json_decode(const JsonObject &in, dsn::replication::backup_item &backup_item); template inline void json_encode_iterable(JsonWriter &out, const T &t) @@ -745,5 +749,15 @@ NON_MEMBER_JSON_SERIALIZATION(dsn::app_info, NON_MEMBER_JSON_SERIALIZATION(dsn::replication::file_meta, name, size, md5) NON_MEMBER_JSON_SERIALIZATION(dsn::replication::bulk_load_metadata, files, file_total_size) + +NON_MEMBER_JSON_SERIALIZATION(dsn::replication::backup_item, + backup_id, + app_id, + app_name, + backup_provider_type, + backup_path, + start_time_ms, + end_time_ms, + status) } // namespace json } // namespace dsn diff --git a/src/common/replication_enums.h b/src/common/replication_enums.h index bd16d81415..e2ab4f6c9b 100644 --- a/src/common/replication_enums.h +++ b/src/common/replication_enums.h @@ -165,6 +165,18 @@ ENUM_REG(replication::manual_compaction_status::RUNNING) ENUM_REG(replication::manual_compaction_status::FINISHED) ENUM_END2(replication::manual_compaction_status::type, manual_compaction_status) +ENUM_BEGIN2(replication::backup_status::type, + backup_status, + replication::backup_status::UNINITIALIZED) +ENUM_REG(replication::backup_status::UNINITIALIZED) +ENUM_REG(replication::backup_status::CHECKPOINTING) +ENUM_REG(replication::backup_status::CHECKPOINTED) +ENUM_REG(replication::backup_status::UPLOADING) +ENUM_REG(replication::backup_status::SUCCEED) +ENUM_REG(replication::backup_status::FAILED) +ENUM_REG(replication::backup_status::CANCELED) +ENUM_END2(replication::backup_status::type, backup_status) + USER_DEFINED_ENUM_FORMATTER(app_status::type) namespace replication { USER_DEFINED_ENUM_FORMATTER(bulk_load_status::type) From 74e52ec5e58949fd9a7bf86953dc4bac208d2455 Mon Sep 17 00:00:00 2001 From: ninsmiracle Date: Tue, 22 Oct 2024 17:44:17 +0800 Subject: [PATCH 06/10] refactory meta_backup_engine --- src/meta/meta_backup_engine.cpp | 175 +++++++++++++++----------------- src/meta/meta_backup_engine.h | 102 ++++++++++++------- 2 files changed, 145 insertions(+), 132 deletions(-) diff --git a/src/meta/meta_backup_engine.cpp b/src/meta/meta_backup_engine.cpp index 7197f4e48b..7fc0a6baa4 100644 --- a/src/meta/meta_backup_engine.cpp +++ b/src/meta/meta_backup_engine.cpp @@ -26,7 +26,7 @@ #include "backup_types.h" #include "block_service/block_service.h" #include "block_service/block_service_manager.h" -#include "common/backup_common.h" + #include "common/gpid.h" #include "common/json_helper.h" #include "common/replication.codes.h" @@ -39,7 +39,7 @@ #include "rpc/rpc_holder.h" #include "rpc/rpc_host_port.h" #include "runtime/api_layer1.h" -#include "server_state.h" + #include "task/async_calls.h" #include "task/task.h" #include "task/task_code.h" @@ -51,69 +51,95 @@ #include "utils/filesystem.h" #include "utils/fmt_logging.h" #include "utils/zlocks.h" +#include "utils/fail_point.h" + +#include "meta_backup_engine.h" + namespace dsn { namespace replication { -backup_engine::backup_engine(backup_service *service) - : _backup_service(service), _block_service(nullptr), _backup_path(""), _is_backup_failed(false) +meta_backup_engine::meta_backup_engine(meta_service *meta_svc, bool is_periodic) + : _meta_svc(meta_svc), _is_periodic_backup(is_periodic) { } -backup_engine::~backup_engine() { _tracker.cancel_outstanding_tasks(); } +meta_backup_engine::~meta_backup_engine() { _tracker.cancel_outstanding_tasks(); } -error_code backup_engine::init_backup(int32_t app_id) +// ThreadPool: THREAD_POOL_DEFAULT +void meta_backup_engine::init_backup(int32_t app_id, + int32_t partition_count, + const std::string &app_name, + const std::string &provider, + const std::string &backup_root_path) { - std::string app_name; - int partition_count; - { - zauto_read_lock l; - _backup_service->get_state()->lock_read(l); - std::shared_ptr app = _backup_service->get_state()->get_app(app_id); - if (app == nullptr || app->status != app_status::AS_AVAILABLE) { - LOG_ERROR("app {} is not available, couldn't do backup now.", app_id); - return ERR_INVALID_STATE; - } - app_name = app->app_name; - partition_count = app->partition_count; - } + zauto_write_lock l(_lock); - zauto_lock lock(_lock); _backup_status.clear(); for (int i = 0; i < partition_count; ++i) { - _backup_status.emplace(i, backup_status::UNALIVE); + _backup_status.emplace_back(backup_status::UNINITIALIZED); } _cur_backup.app_id = app_id; _cur_backup.app_name = app_name; _cur_backup.backup_id = static_cast(dsn_now_ms()); _cur_backup.start_time_ms = _cur_backup.backup_id; - return ERR_OK; + _cur_backup.backup_provider_type = provider; + _cur_backup.backup_path = backup_root_path; + _cur_backup.status = backup_status::UNINITIALIZED; + _is_backup_failed = false; + _is_backup_canceled = false; } -error_code backup_engine::set_block_service(const std::string &provider) +// ThreadPool: THREAD_POOL_DEFAULT +void meta_backup_engine::start() { - _provider_type = provider; - _block_service = _backup_service->get_meta_service() - ->get_block_service_manager() - .get_or_create_block_filesystem(provider); - if (_block_service == nullptr) { - return ERR_INVALID_PARAMETERS; + ddebug_f("App[{}] start {} backup[{}] on {}, root_path = {}", + _cur_backup.app_name, + _is_periodic_backup ? "periodic" : "onetime", + _cur_backup.backup_id, + _cur_backup.backup_provider_type, + _cur_backup.backup_path); + error_code err = write_app_info(); + if (err != ERR_OK) { + derror_f("backup_id({}): backup meta data for app {} failed, error {}", + _cur_backup.backup_id, + _cur_backup.app_id, + err); + update_backup_item_on_remote_storage(backup_status::FAILED, dsn_now_ms()); + return; + } + update_backup_item_on_remote_storage(backup_status::CHECKPOINTING); + FAIL_POINT_INJECT_F("meta_backup_engine_start", [](dsn::string_view) {}); + for (auto i = 0; i < _backup_status.size(); ++i) { + zauto_write_lock l(_lock); + _backup_status[i] = backup_status::CHECKPOINTING; + tasking::enqueue(LPC_DEFAULT_CALLBACK, &_tracker, [this, i]() { + backup_app_partition(gpid(_cur_backup.app_id, i)); + }); } - return ERR_OK; } -error_code backup_engine::set_backup_path(const std::string &path) +// ThreadPool: THREAD_POOL_DEFAULT +error_code meta_backup_engine::write_app_info() { - if (_block_service && _block_service->is_root_path_set()) { - return ERR_INVALID_PARAMETERS; - } - LOG_INFO("backup path is set to {}.", path); - _backup_path = path; + // TODO(heyuchen): TBD + // guoningshen: write app info on block service return ERR_OK; } -error_code backup_engine::write_backup_file(const std::string &file_name, - const dsn::blob &write_buffer) + +// ThreadPool: THREAD_POOL_DEFAULT +void meta_backup_engine::update_backup_item_on_remote_storage(backup_status::type new_status, + int64_t end_time) +{ + // TODO(heyuchen): TBD +} + +// TODO(heyuchen): update following functions + + +error_code meta_backup_engine::write_backup_file(const std::string &file_name, + const dsn::blob &write_buffer) { dist::block_service::create_file_request create_file_req; create_file_req.ignore_metadata = true; @@ -143,7 +169,7 @@ error_code backup_engine::write_backup_file(const std::string &file_name, return err; } -error_code backup_engine::backup_app_meta() +error_code meta_backup_engine::backup_app_meta() { dsn::blob app_info_buffer; { @@ -168,7 +194,7 @@ error_code backup_engine::backup_app_meta() return write_backup_file(file_name, app_info_buffer); } -void backup_engine::backup_app_partition(const gpid &pid) +void meta_backup_engine::backup_app_partition(const gpid &pid) { dsn::host_port partition_primary; { @@ -178,7 +204,7 @@ void backup_engine::backup_app_partition(const gpid &pid) if (app == nullptr || app->status != app_status::AS_AVAILABLE) { LOG_ERROR("app {} is not available, couldn't do backup now.", pid.get_app_id()); - zauto_lock lock(_lock); + zauto_write_lock lock(_lock); _is_backup_failed = true; return; } @@ -222,11 +248,11 @@ void backup_engine::backup_app_partition(const gpid &pid) on_backup_reply(err, rpc.response(), pid, partition_primary); }); - zauto_lock l(_lock); - _backup_status[pid.get_partition_index()] = backup_status::ALIVE; + zauto_write_lock l(_lock); + _backup_status[pid.get_partition_index()] = backup_status::CHECKPOINTING; } -inline void backup_engine::handle_replica_backup_failed(const backup_response &response, +inline void meta_backup_engine::handle_replica_backup_failed(const backup_response &response, const gpid pid) { CHECK_EQ(response.pid, pid); @@ -236,13 +262,13 @@ inline void backup_engine::handle_replica_backup_failed(const backup_response &r _cur_backup.backup_id, pid, response.err); - zauto_lock l(_lock); + zauto_write_lock l(_lock); // if one partition fail, the whole backup plan fail. _is_backup_failed = true; _backup_status[pid.get_partition_index()] = backup_status::FAILED; } -inline void backup_engine::retry_backup(const dsn::gpid pid) +inline void meta_backup_engine::retry_backup(const dsn::gpid pid) { tasking::enqueue( LPC_DEFAULT_CALLBACK, @@ -252,13 +278,13 @@ inline void backup_engine::retry_backup(const dsn::gpid pid) std::chrono::seconds(1)); } -void backup_engine::on_backup_reply(const error_code err, +void meta_backup_engine::on_backup_reply(const error_code err, const backup_response &response, const gpid pid, const host_port &primary) { { - zauto_lock l(_lock); + zauto_read_lock l(_lock); // if backup of some partition failed, we would not handle response from other partitions. if (_is_backup_failed) { return; @@ -293,8 +319,8 @@ void backup_engine::on_backup_reply(const error_code err, CHECK_EQ(response.backup_id, _cur_backup.backup_id); LOG_INFO("backup_id({}): backup for partition {} completed.", _cur_backup.backup_id, pid); { - zauto_lock l(_lock); - _backup_status[pid.get_partition_index()] = backup_status::COMPLETED; + zauto_write_lock l(_lock); + _backup_status[pid.get_partition_index()] = backup_status::SUCCEED; } complete_current_backup(); return; @@ -311,19 +337,20 @@ void backup_engine::on_backup_reply(const error_code err, retry_backup(pid); } -void backup_engine::write_backup_info() +void meta_backup_engine::write_backup_info() { std::string backup_root = dsn::utils::filesystem::path_combine(_backup_path, _backup_service->backup_root()); std::string file_name = cold_backup::get_backup_info_file(backup_root, _cur_backup.backup_id); - blob buf = dsn::json::json_forwarder::encode(_cur_backup); + // Updated backup item struct, so need use another json method + blob buf = dsn::json::json_forwarder::encode(_cur_backup); error_code err = write_backup_file(file_name, buf); if (err == ERR_FS_INTERNAL) { LOG_ERROR( "backup_id({}): write backup info failed, error {}, do not try again for this error.", _cur_backup.backup_id, err); - zauto_lock l(_lock); + zauto_write_lock l(_lock); _is_backup_failed = true; return; } @@ -341,11 +368,11 @@ void backup_engine::write_backup_info() LOG_INFO("backup_id({}): successfully wrote backup info, backup for app {} completed.", _cur_backup.backup_id, _cur_backup.app_id); - zauto_lock l(_lock); + zauto_write_lock l(_lock); _cur_backup.end_time_ms = dsn_now_ms(); } -void backup_engine::complete_current_backup() +void meta_backup_engine::complete_current_backup() { { zauto_lock l(_lock); @@ -360,43 +387,5 @@ void backup_engine::complete_current_backup() write_backup_info(); } -error_code backup_engine::start() -{ - error_code err = backup_app_meta(); - if (err != ERR_OK) { - LOG_ERROR("backup_id({}): backup meta data for app {} failed, error {}", - _cur_backup.backup_id, - _cur_backup.app_id, - err); - return err; - } - for (int i = 0; i < _backup_status.size(); ++i) { - tasking::enqueue(LPC_DEFAULT_CALLBACK, &_tracker, [this, i]() { - backup_app_partition(gpid(_cur_backup.app_id, i)); - }); - } - return ERR_OK; -} - -bool backup_engine::is_in_progress() const -{ - zauto_lock l(_lock); - return _cur_backup.end_time_ms == 0 && !_is_backup_failed; -} - -backup_item backup_engine::get_backup_item() const -{ - zauto_lock l(_lock); - backup_item item; - item.backup_id = _cur_backup.backup_id; - item.app_name = _cur_backup.app_name; - item.backup_path = _backup_path; - item.backup_provider_type = _provider_type; - item.start_time_ms = _cur_backup.start_time_ms; - item.end_time_ms = _cur_backup.end_time_ms; - item.is_backup_failed = _is_backup_failed; - return item; -} - } // namespace replication } // namespace dsn diff --git a/src/meta/meta_backup_engine.h b/src/meta/meta_backup_engine.h index 2f68b92b11..08baa65e08 100644 --- a/src/meta/meta_backup_engine.h +++ b/src/meta/meta_backup_engine.h @@ -28,6 +28,11 @@ #include "utils/error_code.h" #include "utils/zlocks.h" +#include "common/backup_common.h" +#include "meta_service.h" +#include "server_state.h" +#include "meta_backup_service.h" + namespace dsn { class blob; class gpid; @@ -41,14 +46,6 @@ class block_filesystem; namespace replication { -enum backup_status -{ - UNALIVE = 1, - ALIVE = 2, - COMPLETED = 3, - FAILED = 4 -}; - struct app_backup_info { int64_t backup_id; @@ -63,45 +60,80 @@ struct app_backup_info DEFINE_JSON_SERIALIZATION(backup_id, start_time_ms, end_time_ms, app_id, app_name) }; -class backup_service; - -class backup_engine +/// +/// Meta backup status +/// +/// start backup +/// | +/// v Error/Cancel +/// Checkpointing ------------->| +/// | | +/// v Error/Cancel | +/// Uploading -------------->| +/// | | +/// v v +/// Succeed Failed/Canceled +/// +class meta_backup_engine { public: - backup_engine(backup_service *service); - ~backup_engine(); - - error_code init_backup(int32_t app_id); - error_code set_block_service(const std::string &provider); - error_code set_backup_path(const std::string &path); - - error_code start(); + explicit meta_backup_engine(meta_service *meta_svc, bool is_periodic); + ~meta_backup_engine(); int64_t get_current_backup_id() const { return _cur_backup.backup_id; } int32_t get_backup_app_id() const { return _cur_backup.app_id; } - bool is_in_progress() const; - backup_item get_backup_item() const; + backup_item get_backup_item() const + { + zauto_read_lock l(_lock); + backup_item item = _cur_backup; + return item; + } -private: - friend class backup_engine_test; - friend class backup_service_test; + bool is_in_progress() const + { + zauto_read_lock l(_lock); + return _cur_backup.end_time_ms == 0 && !_is_backup_failed && !_is_backup_canceled; + } - FRIEND_TEST(backup_engine_test, test_on_backup_reply); - FRIEND_TEST(backup_engine_test, test_backup_completed); - FRIEND_TEST(backup_engine_test, test_write_backup_info_failed); +private: + void init_backup(int32_t app_id, + int32_t partition_count, + const std::string &app_name, + const std::string &provider, + const std::string &backup_root_path); + void start(); - error_code write_backup_file(const std::string &file_name, const dsn::blob &write_buffer); - error_code backup_app_meta(); void backup_app_partition(const gpid &pid); void on_backup_reply(error_code err, const backup_response &response, gpid pid, const host_port &primary); + void retry_backup(const dsn::gpid pid); + void handle_replica_backup_failed(const backup_response &response, const gpid pid); + error_code write_backup_file(const std::string &file_name, const dsn::blob &write_buffer); + error_code write_app_info(); + void write_backup_info(); + + void update_backup_item_on_remote_storage(backup_status::type new_status, int64_t end_time = 0); +private: + friend class meta_backup_engine_test; + meta_service *_meta_svc; + task_tracker _tracker; + mutable zrwlock_nr _lock; // { + // TODO(guoningshen): remove this flag cause we do not implement perodic in pegasus server + bool _is_periodic_backup; + bool _is_backup_failed{false}; + bool _is_backup_canceled{false}; + backup_item _cur_backup; + std::vector _backup_status; + // } + // TODO(heyuchen): remove following functions and vars +private: + error_code backup_app_meta(); + void complete_current_backup(); - void handle_replica_backup_failed(const backup_response &response, const gpid pid); - void retry_backup(const dsn::gpid pid); const std::string get_policy_name() const { @@ -112,14 +144,6 @@ class backup_engine dist::block_service::block_filesystem *_block_service; std::string _backup_path; std::string _provider_type; - dsn::task_tracker _tracker; - - // lock the following variables. - mutable dsn::zlock _lock; - bool _is_backup_failed; - app_backup_info _cur_backup; - // partition_id -> backup_status - std::map _backup_status; }; } // namespace replication From b77704a6eb9b586bdeea331f9c1299e5cbd54381 Mon Sep 17 00:00:00 2001 From: ninsmiracle Date: Wed, 23 Oct 2024 11:54:17 +0800 Subject: [PATCH 07/10] fix: pass complie --- src/meta/meta_backup_engine.cpp | 13 +++++++------ src/meta/meta_backup_service.cpp | 2 +- src/meta/meta_service.h | 2 ++ 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/meta/meta_backup_engine.cpp b/src/meta/meta_backup_engine.cpp index 7fc0a6baa4..17f2c25467 100644 --- a/src/meta/meta_backup_engine.cpp +++ b/src/meta/meta_backup_engine.cpp @@ -27,11 +27,12 @@ #include "block_service/block_service.h" #include "block_service/block_service_manager.h" +#include "absl/strings/string_view.h" #include "common/gpid.h" #include "common/json_helper.h" #include "common/replication.codes.h" #include "dsn.layer2_types.h" -#include "meta/backup_engine.h" +#include "meta_backup_engine.h" #include "meta/meta_backup_service.h" #include "meta/meta_data.h" #include "meta/meta_service.h" @@ -93,7 +94,7 @@ void meta_backup_engine::init_backup(int32_t app_id, // ThreadPool: THREAD_POOL_DEFAULT void meta_backup_engine::start() { - ddebug_f("App[{}] start {} backup[{}] on {}, root_path = {}", + LOG_DEBUG("App[{}] start {} backup[{}] on {}, root_path = {}", _cur_backup.app_name, _is_periodic_backup ? "periodic" : "onetime", _cur_backup.backup_id, @@ -101,7 +102,7 @@ void meta_backup_engine::start() _cur_backup.backup_path); error_code err = write_app_info(); if (err != ERR_OK) { - derror_f("backup_id({}): backup meta data for app {} failed, error {}", + LOG_ERROR("backup_id({}): backup meta data for app {} failed, error {}", _cur_backup.backup_id, _cur_backup.app_id, err); @@ -109,7 +110,7 @@ void meta_backup_engine::start() return; } update_backup_item_on_remote_storage(backup_status::CHECKPOINTING); - FAIL_POINT_INJECT_F("meta_backup_engine_start", [](dsn::string_view) {}); + FAIL_POINT_INJECT_F("meta_backup_engine_start", [](absl::string_view) {}); for (auto i = 0; i < _backup_status.size(); ++i) { zauto_write_lock l(_lock); _backup_status[i] = backup_status::CHECKPOINTING; @@ -375,9 +376,9 @@ void meta_backup_engine::write_backup_info() void meta_backup_engine::complete_current_backup() { { - zauto_lock l(_lock); + zauto_read_lock l(_lock); for (const auto &status : _backup_status) { - if (status.second != backup_status::COMPLETED) { + if (status != backup_status::SUCCEED) { // backup for some partition was not finished. return; } diff --git a/src/meta/meta_backup_service.cpp b/src/meta/meta_backup_service.cpp index fa6ee4bc8b..72e7395e2a 100644 --- a/src/meta/meta_backup_service.cpp +++ b/src/meta/meta_backup_service.cpp @@ -30,7 +30,7 @@ #include "common/replication.codes.h" #include "common/replication_enums.h" #include "dsn.layer2_types.h" -#include "meta/backup_engine.h" +#include "meta_backup_engine.h" #include "meta/meta_data.h" #include "meta/meta_rpc_types.h" #include "meta/meta_state_service.h" diff --git a/src/meta/meta_service.h b/src/meta/meta_service.h index efec058640..dfbae32f10 100644 --- a/src/meta/meta_service.h +++ b/src/meta/meta_service.h @@ -89,6 +89,8 @@ namespace mss { struct meta_storage; } // namespace mss +class backup_service; + namespace test { class test_checker; } From aca01d7f9647614fee0b7e1685cb7d604356059c Mon Sep 17 00:00:00 2001 From: ninsmiracle Date: Wed, 23 Oct 2024 16:30:36 +0800 Subject: [PATCH 08/10] pass IWYU --- src/meta/meta_backup_engine.cpp | 13 ++++----- src/meta/meta_backup_engine.h | 10 ++----- src/meta/meta_backup_service.cpp | 28 +------------------ src/meta/meta_service.h | 2 -- src/meta/test/meta_http_service_test.cpp | 14 ---------- src/replica/backup/replica_backup_manager.cpp | 22 +-------------- src/replica/backup/replica_backup_manager.h | 4 --- src/replica/replica.h | 3 -- src/replica/replica_stub.cpp | 1 - src/replica/test/replica_test.cpp | 1 - 10 files changed, 10 insertions(+), 88 deletions(-) diff --git a/src/meta/meta_backup_engine.cpp b/src/meta/meta_backup_engine.cpp index 17f2c25467..65366d188d 100644 --- a/src/meta/meta_backup_engine.cpp +++ b/src/meta/meta_backup_engine.cpp @@ -16,31 +16,30 @@ // under the License. #include -#include #include #include #include #include #include +#include "absl/strings/string_view.h" #include "backup_types.h" #include "block_service/block_service.h" -#include "block_service/block_service_manager.h" -#include "absl/strings/string_view.h" +#include "common/backup_common.h" #include "common/gpid.h" #include "common/json_helper.h" #include "common/replication.codes.h" #include "dsn.layer2_types.h" -#include "meta_backup_engine.h" #include "meta/meta_backup_service.h" #include "meta/meta_data.h" #include "meta/meta_service.h" +#include "meta/server_state.h" +#include "meta_backup_engine.h" #include "rpc/dns_resolver.h" #include "rpc/rpc_holder.h" #include "rpc/rpc_host_port.h" #include "runtime/api_layer1.h" - #include "task/async_calls.h" #include "task/task.h" #include "task/task_code.h" @@ -49,12 +48,10 @@ #include "utils/blob.h" #include "utils/chrono_literals.h" #include "utils/error_code.h" +#include "utils/fail_point.h" #include "utils/filesystem.h" #include "utils/fmt_logging.h" #include "utils/zlocks.h" -#include "utils/fail_point.h" - -#include "meta_backup_engine.h" namespace dsn { diff --git a/src/meta/meta_backup_engine.h b/src/meta/meta_backup_engine.h index 08baa65e08..0f912bbd44 100644 --- a/src/meta/meta_backup_engine.h +++ b/src/meta/meta_backup_engine.h @@ -17,10 +17,9 @@ #pragma once -#include #include -#include #include +#include #include "backup_types.h" #include "common/json_helper.h" @@ -28,11 +27,6 @@ #include "utils/error_code.h" #include "utils/zlocks.h" -#include "common/backup_common.h" -#include "meta_service.h" -#include "server_state.h" -#include "meta_backup_service.h" - namespace dsn { class blob; class gpid; @@ -45,6 +39,8 @@ class block_filesystem; } // namespace dist namespace replication { +class backup_service; +class meta_service; struct app_backup_info { diff --git a/src/meta/meta_backup_service.cpp b/src/meta/meta_backup_service.cpp index 72e7395e2a..7f77088ad4 100644 --- a/src/meta/meta_backup_service.cpp +++ b/src/meta/meta_backup_service.cpp @@ -16,38 +16,12 @@ // under the License. #include -#include -#include -#include -#include -#include -#include -#include -#include "block_service/block_service.h" -#include "block_service/block_service_manager.h" -#include "common/backup_common.h" -#include "common/replication.codes.h" -#include "common/replication_enums.h" -#include "dsn.layer2_types.h" -#include "meta_backup_engine.h" -#include "meta/meta_data.h" #include "meta/meta_rpc_types.h" -#include "meta/meta_state_service.h" #include "meta_backup_service.h" #include "meta_service.h" -#include "runtime/api_layer1.h" -#include "security/access_controller.h" -#include "task/async_calls.h" -#include "task/task_code.h" -#include "server_state.h" -#include "utils/autoref_ptr.h" -#include "utils/blob.h" -#include "utils/chrono_literals.h" -#include "utils/defer.h" #include "utils/flags.h" -#include "utils/fmt_logging.h" -#include "utils/time_utils.h" +#include "utils/metrics.h" DSN_DECLARE_int32(cold_backup_checkpoint_reserve_minutes); DSN_DECLARE_int32(fd_lease_seconds); diff --git a/src/meta/meta_service.h b/src/meta/meta_service.h index dfbae32f10..efec058640 100644 --- a/src/meta/meta_service.h +++ b/src/meta/meta_service.h @@ -89,8 +89,6 @@ namespace mss { struct meta_storage; } // namespace mss -class backup_service; - namespace test { class test_checker; } diff --git a/src/meta/test/meta_http_service_test.cpp b/src/meta/test/meta_http_service_test.cpp index c70fc7fda8..8d8e4cc0bf 100644 --- a/src/meta/test/meta_http_service_test.cpp +++ b/src/meta/test/meta_http_service_test.cpp @@ -15,38 +15,24 @@ // specific language governing permissions and limitations // under the License. -#include #include #include #include #include #include #include -#include -#include "backup_types.h" #include "bulk_load_types.h" #include "common/gpid.h" #include "common/replication_other_types.h" #include "gtest/gtest.h" #include "http/http_server.h" #include "http/http_status_code.h" -#include "meta/meta_backup_service.h" #include "meta/meta_bulk_load_service.h" #include "meta/meta_data.h" #include "meta/meta_http_service.h" -#include "meta/meta_service.h" -#include "meta/meta_state_service.h" -#include "meta_service_test_app.h" #include "meta_test_base.h" -#include "rpc/rpc_holder.h" -#include "rpc/rpc_message.h" -#include "task/task.h" -#include "task/task_code.h" -#include "utils/autoref_ptr.h" #include "utils/blob.h" -#include "utils/chrono_literals.h" -#include "utils/error_code.h" #include "utils/fail_point.h" namespace dsn { diff --git a/src/replica/backup/replica_backup_manager.cpp b/src/replica/backup/replica_backup_manager.cpp index 655a197034..59a3d48714 100644 --- a/src/replica/backup/replica_backup_manager.cpp +++ b/src/replica/backup/replica_backup_manager.cpp @@ -18,29 +18,9 @@ #include "replica_backup_manager.h" #include -#include -#include -#include -#include -#include -#include -#include -#include "backup_types.h" -#include "common/gpid.h" -#include "common/replication.codes.h" -#include "dsn.layer2_types.h" -#include "metadata_types.h" -#include "replica/replica.h" -#include "replica/replica_context.h" -#include "replica/replication_app_base.h" -#include "runtime/api_layer1.h" -#include "utils/autoref_ptr.h" -#include "utils/filesystem.h" #include "utils/flags.h" -#include "utils/fmt_logging.h" -#include "utils/strings.h" -#include "utils/thread_access_checker.h" +#include "utils/metrics.h" METRIC_DEFINE_gauge_int64(replica, backup_running_count, diff --git a/src/replica/backup/replica_backup_manager.h b/src/replica/backup/replica_backup_manager.h index 6d0f135e2c..aa941b31ef 100644 --- a/src/replica/backup/replica_backup_manager.h +++ b/src/replica/backup/replica_backup_manager.h @@ -17,11 +17,7 @@ #pragma once -#include - #include "replica/replica_base.h" -#include "task/task.h" -#include "utils/metrics.h" namespace dsn { diff --git a/src/replica/replica.h b/src/replica/replica.h index 4cd725bccb..8878ccfde0 100644 --- a/src/replica/replica.h +++ b/src/replica/replica.h @@ -84,9 +84,6 @@ class access_controller; } // namespace security namespace replication { -class backup_request; -class backup_response; - class configuration_restore_request; class detect_hotkey_request; class detect_hotkey_response; diff --git a/src/replica/replica_stub.cpp b/src/replica/replica_stub.cpp index 4dc2ad719a..1f567e4c5e 100644 --- a/src/replica/replica_stub.cpp +++ b/src/replica/replica_stub.cpp @@ -44,7 +44,6 @@ #include #include -#include "absl/strings/string_view.h" #include "bulk_load/replica_bulk_loader.h" #include "common/backup_common.h" #include "common/duplication_common.h" diff --git a/src/replica/test/replica_test.cpp b/src/replica/test/replica_test.cpp index 6038df8c9b..3bec36cfa1 100644 --- a/src/replica/test/replica_test.cpp +++ b/src/replica/test/replica_test.cpp @@ -52,7 +52,6 @@ #include "rpc/network.sim.h" #include "rpc/rpc_address.h" #include "rpc/rpc_message.h" -#include "runtime/api_layer1.h" #include "task/task_code.h" #include "task/task_tracker.h" #include "test_util/test_util.h" From 11456fb55be6a4aa1daa5809fc0e109b7c202477 Mon Sep 17 00:00:00 2001 From: ninsmiracle Date: Tue, 29 Oct 2024 11:02:58 +0800 Subject: [PATCH 09/10] format code after IWYU --- src/meta/meta_backup_engine.cpp | 27 ++++++++++++--------------- src/meta/meta_backup_engine.h | 1 + 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/meta/meta_backup_engine.cpp b/src/meta/meta_backup_engine.cpp index 65366d188d..330abe23c9 100644 --- a/src/meta/meta_backup_engine.cpp +++ b/src/meta/meta_backup_engine.cpp @@ -53,7 +53,6 @@ #include "utils/fmt_logging.h" #include "utils/zlocks.h" - namespace dsn { namespace replication { @@ -92,17 +91,17 @@ void meta_backup_engine::init_backup(int32_t app_id, void meta_backup_engine::start() { LOG_DEBUG("App[{}] start {} backup[{}] on {}, root_path = {}", - _cur_backup.app_name, - _is_periodic_backup ? "periodic" : "onetime", - _cur_backup.backup_id, - _cur_backup.backup_provider_type, - _cur_backup.backup_path); + _cur_backup.app_name, + _is_periodic_backup ? "periodic" : "onetime", + _cur_backup.backup_id, + _cur_backup.backup_provider_type, + _cur_backup.backup_path); error_code err = write_app_info(); if (err != ERR_OK) { LOG_ERROR("backup_id({}): backup meta data for app {} failed, error {}", - _cur_backup.backup_id, - _cur_backup.app_id, - err); + _cur_backup.backup_id, + _cur_backup.app_id, + err); update_backup_item_on_remote_storage(backup_status::FAILED, dsn_now_ms()); return; } @@ -125,7 +124,6 @@ error_code meta_backup_engine::write_app_info() return ERR_OK; } - // ThreadPool: THREAD_POOL_DEFAULT void meta_backup_engine::update_backup_item_on_remote_storage(backup_status::type new_status, int64_t end_time) @@ -135,7 +133,6 @@ void meta_backup_engine::update_backup_item_on_remote_storage(backup_status::typ // TODO(heyuchen): update following functions - error_code meta_backup_engine::write_backup_file(const std::string &file_name, const dsn::blob &write_buffer) { @@ -251,7 +248,7 @@ void meta_backup_engine::backup_app_partition(const gpid &pid) } inline void meta_backup_engine::handle_replica_backup_failed(const backup_response &response, - const gpid pid) + const gpid pid) { CHECK_EQ(response.pid, pid); CHECK_EQ(response.backup_id, _cur_backup.backup_id); @@ -277,9 +274,9 @@ inline void meta_backup_engine::retry_backup(const dsn::gpid pid) } void meta_backup_engine::on_backup_reply(const error_code err, - const backup_response &response, - const gpid pid, - const host_port &primary) + const backup_response &response, + const gpid pid, + const host_port &primary) { { zauto_read_lock l(_lock); diff --git a/src/meta/meta_backup_engine.h b/src/meta/meta_backup_engine.h index 0f912bbd44..87ca945350 100644 --- a/src/meta/meta_backup_engine.h +++ b/src/meta/meta_backup_engine.h @@ -113,6 +113,7 @@ class meta_backup_engine void write_backup_info(); void update_backup_item_on_remote_storage(backup_status::type new_status, int64_t end_time = 0); + private: friend class meta_backup_engine_test; meta_service *_meta_svc; From f5f3a8f315c98a2957db4dc92254d175a9a2a972 Mon Sep 17 00:00:00 2001 From: ninsmiracle Date: Thu, 31 Oct 2024 16:32:36 +0800 Subject: [PATCH 10/10] rebase code and pass IWYU --- src/meta/meta_backup_engine.cpp | 2 +- src/replica/backup/replica_backup_manager.cpp | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/meta/meta_backup_engine.cpp b/src/meta/meta_backup_engine.cpp index 330abe23c9..e70cbd599d 100644 --- a/src/meta/meta_backup_engine.cpp +++ b/src/meta/meta_backup_engine.cpp @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +#include #include #include #include @@ -25,7 +26,6 @@ #include "absl/strings/string_view.h" #include "backup_types.h" #include "block_service/block_service.h" - #include "common/backup_common.h" #include "common/gpid.h" #include "common/json_helper.h" diff --git a/src/replica/backup/replica_backup_manager.cpp b/src/replica/backup/replica_backup_manager.cpp index 59a3d48714..c3f8486fe7 100644 --- a/src/replica/backup/replica_backup_manager.cpp +++ b/src/replica/backup/replica_backup_manager.cpp @@ -17,10 +17,11 @@ #include "replica_backup_manager.h" -#include +#include #include "utils/flags.h" #include "utils/metrics.h" +#include "replica/replica.h" METRIC_DEFINE_gauge_int64(replica, backup_running_count,