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/09/20 03:48:52 UTC
[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1150: feat(backup): 9. replica handle backup clear context and invalid status
acelyc111 commented on code in PR #1150:
URL: https://github.com/apache/incubator-pegasus/pull/1150#discussion_r974855703
##########
src/rdsn/src/common/backup_restore_common.h:
##########
@@ -29,6 +29,7 @@ namespace dsn {
namespace replication {
DSN_DECLARE_string(cold_backup_root);
+DSN_DECLARE_uint32(cold_backup_checkpoint_reserve_minutes);
Review Comment:
It's no needed to declare it in header files, declare it in the cpp files where you use it is OK?
##########
src/rdsn/src/replica/backup/test/replica_backup_manager_test.cpp:
##########
@@ -120,7 +143,178 @@ class replica_backup_manager_test : public replica_test_base
std::unique_ptr<replica_backup_manager> _backup_mgr;
};
-// TODO(heyuchen): add unit test for on_backup after implement all status
+TEST_F(replica_backup_manager_test, on_backup_test)
+{
+ fail::cfg("replica_backup_start_checkpointing", "return()");
+ fail::cfg("replica_backup_start_uploading", "return()");
+ fail::cfg("replica_backup_upload_completed", "return()");
+ fail::cfg("replica_backup_clear_context", "return()");
+
+ struct test_struct
+ {
+ backup_status::type local_status;
+ backup_status::type request_status;
+ error_code expected_err;
+ backup_status::type expected_status;
+ } tests[]{
+ // request_status = UNINITIALIZED
+ {backup_status::UNINITIALIZED,
+ backup_status::UNINITIALIZED,
+ ERR_INVALID_STATE,
+ backup_status::UNINITIALIZED},
+ {backup_status::CHECKPOINTING,
+ backup_status::UNINITIALIZED,
+ ERR_INVALID_STATE,
+ backup_status::CHECKPOINTING},
+ {backup_status::CHECKPOINTED,
+ backup_status::UNINITIALIZED,
+ ERR_INVALID_STATE,
+ backup_status::CHECKPOINTED},
+ {backup_status::UPLOADING,
+ backup_status::UNINITIALIZED,
+ ERR_INVALID_STATE,
+ backup_status::UPLOADING},
+ {backup_status::SUCCEED,
+ backup_status::UNINITIALIZED,
+ ERR_INVALID_STATE,
+ backup_status::SUCCEED},
+ {backup_status::FAILED,
+ backup_status::UNINITIALIZED,
+ ERR_INVALID_STATE,
+ backup_status::FAILED},
+ {backup_status::CANCELED,
+ backup_status::UNINITIALIZED,
+ ERR_INVALID_STATE,
+ backup_status::CANCELED},
+ // request_status = CHECKPOINTING
+ {backup_status::UNINITIALIZED,
+ backup_status::CHECKPOINTING,
+ ERR_OK,
+ backup_status::CHECKPOINTING},
+ {backup_status::CHECKPOINTING,
+ backup_status::CHECKPOINTING,
+ ERR_OK,
+ backup_status::CHECKPOINTING},
+ {backup_status::CHECKPOINTED,
+ backup_status::CHECKPOINTING,
+ ERR_OK,
+ backup_status::CHECKPOINTED},
+ {backup_status::UPLOADING,
+ backup_status::CHECKPOINTING,
+ ERR_INVALID_STATE,
+ backup_status::UPLOADING},
+ {backup_status::SUCCEED,
+ backup_status::CHECKPOINTING,
+ ERR_INVALID_STATE,
+ backup_status::SUCCEED},
+ {backup_status::FAILED,
+ backup_status::CHECKPOINTING,
+ ERR_INVALID_STATE,
+ backup_status::FAILED},
+ {backup_status::CANCELED,
+ backup_status::CHECKPOINTING,
+ ERR_INVALID_STATE,
+ backup_status::CANCELED},
+ // request_status = CHECKPOINTED
+ {backup_status::UNINITIALIZED,
+ backup_status::CHECKPOINTED,
+ ERR_INVALID_STATE,
+ backup_status::UNINITIALIZED},
+ {backup_status::CHECKPOINTING,
+ backup_status::CHECKPOINTED,
+ ERR_INVALID_STATE,
+ backup_status::CHECKPOINTING},
+ {backup_status::CHECKPOINTED,
+ backup_status::CHECKPOINTED,
+ ERR_INVALID_STATE,
+ backup_status::CHECKPOINTED},
+ {backup_status::UPLOADING,
+ backup_status::CHECKPOINTED,
+ ERR_INVALID_STATE,
+ backup_status::UPLOADING},
+ {backup_status::SUCCEED,
+ backup_status::CHECKPOINTED,
+ ERR_INVALID_STATE,
+ backup_status::SUCCEED},
+ {backup_status::FAILED,
+ backup_status::CHECKPOINTED,
+ ERR_INVALID_STATE,
+ backup_status::FAILED},
+ {backup_status::CANCELED,
+ backup_status::CHECKPOINTED,
+ ERR_INVALID_STATE,
+ backup_status::CANCELED},
+ // request_status = UPLOADING
+ {backup_status::UNINITIALIZED,
+ backup_status::UPLOADING,
+ ERR_INVALID_STATE,
+ backup_status::UNINITIALIZED},
+ {backup_status::CHECKPOINTING,
+ backup_status::UPLOADING,
+ ERR_INVALID_STATE,
+ backup_status::CHECKPOINTING},
+ {backup_status::CHECKPOINTED, backup_status::UPLOADING, ERR_OK, backup_status::UPLOADING},
+ {backup_status::UPLOADING, backup_status::UPLOADING, ERR_OK, backup_status::UPLOADING},
+ {backup_status::SUCCEED, backup_status::UPLOADING, ERR_OK, backup_status::SUCCEED},
+ {backup_status::FAILED, backup_status::UPLOADING, ERR_INVALID_STATE, backup_status::FAILED},
+ {backup_status::CANCELED,
+ backup_status::UPLOADING,
+ ERR_INVALID_STATE,
+ backup_status::CANCELED},
+ // request_status = SUCCEED
+ {backup_status::UNINITIALIZED,
+ backup_status::SUCCEED,
+ ERR_INVALID_STATE,
+ backup_status::UNINITIALIZED},
+ {backup_status::CHECKPOINTING,
+ backup_status::SUCCEED,
+ ERR_INVALID_STATE,
+ backup_status::CHECKPOINTING},
+ {backup_status::CHECKPOINTED,
+ backup_status::SUCCEED,
+ ERR_INVALID_STATE,
+ backup_status::CHECKPOINTED},
+ {backup_status::UPLOADING,
+ backup_status::SUCCEED,
+ ERR_INVALID_STATE,
+ backup_status::UPLOADING},
+ {backup_status::SUCCEED, backup_status::SUCCEED, ERR_INVALID_STATE, backup_status::SUCCEED},
+ {backup_status::FAILED, backup_status::SUCCEED, ERR_INVALID_STATE, backup_status::FAILED},
+ {backup_status::CANCELED,
+ backup_status::SUCCEED,
+ ERR_INVALID_STATE,
+ backup_status::CANCELED},
+ // request_status = FAILED
+ {backup_status::UNINITIALIZED, backup_status::FAILED, ERR_OK, backup_status::UNINITIALIZED},
+ {backup_status::CHECKPOINTING, backup_status::FAILED, ERR_OK, backup_status::UNINITIALIZED},
+ {backup_status::CHECKPOINTED, backup_status::FAILED, ERR_OK, backup_status::UNINITIALIZED},
+ {backup_status::UPLOADING, backup_status::FAILED, ERR_OK, backup_status::UNINITIALIZED},
+ {backup_status::SUCCEED, backup_status::FAILED, ERR_OK, backup_status::UNINITIALIZED},
+ {backup_status::FAILED, backup_status::FAILED, ERR_OK, backup_status::UNINITIALIZED},
+ {backup_status::CANCELED, backup_status::FAILED, ERR_OK, backup_status::UNINITIALIZED},
+ // request_status = CANCELED
+ {backup_status::UNINITIALIZED,
+ backup_status::CANCELED,
+ ERR_OK,
+ backup_status::UNINITIALIZED},
+ {backup_status::CHECKPOINTING,
+ backup_status::CANCELED,
+ ERR_OK,
+ backup_status::UNINITIALIZED},
+ {backup_status::CHECKPOINTED,
+ backup_status::CANCELED,
+ ERR_OK,
+ backup_status::UNINITIALIZED},
+ {backup_status::UPLOADING, backup_status::CANCELED, ERR_OK, backup_status::UNINITIALIZED},
+ {backup_status::SUCCEED, backup_status::CANCELED, ERR_OK, backup_status::UNINITIALIZED},
+ {backup_status::FAILED, backup_status::CANCELED, ERR_OK, backup_status::UNINITIALIZED},
+ {backup_status::CANCELED, backup_status::CANCELED, ERR_OK, backup_status::UNINITIALIZED}};
+
+ for (const auto &test : tests) {
+ ASSERT_EQ(on_backup(test.local_status, test.request_status), test.expected_err);
Review Comment:
if any test case failed, it would be a bit of harder to find out which one failed? add some message after ASSERT_* may help, for example the index of `tests`.
e.g.
```
int i = 0;
...
ASSERT_EQ(...) << i++;
```
##########
src/rdsn/src/replica/backup/replica_backup_manager.cpp:
##########
@@ -414,7 +422,70 @@ void replica_backup_manager::clear_context()
{
FAIL_POINT_INJECT_F("replica_backup_clear_context",
[this](dsn::string_view) { _status = backup_status::UNINITIALIZED; });
- // TODO(heyuchen): TBD
+
+ zauto_write_lock l(_lock);
+ if (_status == backup_status::UNINITIALIZED) {
+ return;
+ }
+ ddebug_replica("start to clear backup context, old status = {}", enum_to_string(_status));
+
+ for (auto &kv : _upload_files_task) {
Review Comment:
```suggestion
for (const auto &kv : _upload_files_task) {
```
--
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