You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by GitBox <gi...@apache.org> on 2022/08/09 03:46:00 UTC
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1102: feat(backup): 2. update and refactor meta backup engine class
acelyc111 commented on code in PR #1102:
URL: https://github.com/apache/incubator-pegasus/pull/1102#discussion_r940854781
##########
src/rdsn/src/common/backup.thrift:
##########
@@ -20,6 +20,30 @@ include "../../../../idl/dsn.layer2.thrift"
namespace cpp dsn.replication
+enum backup_status
+{
+ INVALID,
+ CHECKPOINTING,
+ CHECKPOINTED,
+ UPLOADING,
+ SUCCEED,
+ FAILED,
+ CANCELED
Review Comment:
typo
##########
src/rdsn/include/dsn/dist/replication/replication_enums.h:
##########
@@ -157,4 +157,14 @@ ENUM_REG(replication::manual_compaction_status::QUEUING)
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::INVALID)
+ENUM_REG(replication::backup_status::INVALID)
+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)
Review Comment:
typo: CANCELLED
##########
src/rdsn/src/meta/meta_backup_engine.cpp:
##########
@@ -16,74 +16,93 @@
// under the License.
#include <dsn/dist/fmt_logging.h>
+#include <dsn/utility/fail_point.h>
#include <dsn/utility/filesystem.h>
-#include "common/backup_common.h"
-#include "common/replication_common.h"
-#include "server_state.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) : _meta_svc(meta_svc) {}
Review Comment:
I'm a bit of confused can it be replaced by the other version `meta_backup_engine(meta_service *meta_svc, bool is_periodic)`?
##########
src/rdsn/src/common/backup.thrift:
##########
@@ -20,6 +20,30 @@ include "../../../../idl/dsn.layer2.thrift"
namespace cpp dsn.replication
+enum backup_status
+{
+ INVALID,
+ CHECKPOINTING,
+ CHECKPOINTED,
Review Comment:
maybe CHECKING_POINT and CHECKED_POINT ?
##########
src/rdsn/src/meta/meta_backup_engine.h:
##########
@@ -18,74 +18,107 @@
#pragma once
#include <dsn/cpp/json_helper.h>
-#include <dsn/dist/block_service.h>
#include <dsn/tool-api/zlocks.h>
+#include "common/backup_common.h"
+#include "meta_service.h"
+#include "server_state.h"
+#include "meta_backup_service.h"
+
namespace dsn {
namespace replication {
-enum backup_status
-{
- UNALIVE = 1,
- ALIVE = 2,
- COMPLETED = 3,
- FAILED = 4
-};
-
+// backup_info file written into block service
struct app_backup_info
{
int64_t backup_id;
int64_t start_time_ms;
int64_t end_time_ms;
-
int32_t app_id;
std::string app_name;
-
app_backup_info() : backup_id(0), start_time_ms(0), end_time_ms(0) {}
-
DEFINE_JSON_SERIALIZATION(backup_id, start_time_ms, end_time_ms, app_id, app_name)
};
-class app_state;
-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);
+ 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 rpc_address &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; // {
+ bool _is_periodic_backup;
Review Comment:
also init as `false`?
##########
src/rdsn/src/common/backup.thrift:
##########
@@ -20,6 +20,30 @@ include "../../../../idl/dsn.layer2.thrift"
namespace cpp dsn.replication
+enum backup_status
+{
+ INVALID,
Review Comment:
how about UNINITIALIZED?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org