Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

feat(one-time backup): part-1 meta server handle rpc and init backup #772

Merged
merged 4 commits into from
Mar 8, 2021

Conversation

zhangyifan27
Copy link
Contributor

@zhangyifan27 zhangyifan27 commented Mar 3, 2021

Prior to this patch, we could only backup tables by creating a policy. But there are some issues in policy:

  • we don't know when the meta server would start a backup, even though we have set the start_time.
  • we couldn't disable a policy in some cases.
  • if the interval time is less than 24 hours, the behavior of policy_context would be uncontrollable.

In this patch, meta server could start backup tables immediately after received a backup rpc.

TODO:

  • meta server backup app metadata.
  • meta server send backup request to replica servers and handle response.
  • add query_backup_status rpc.
  • add tests.

src/meta/meta_backup_service.h Outdated Show resolved Hide resolved
src/common/backup.thrift Outdated Show resolved Hide resolved
src/meta/meta_backup_service.h Outdated Show resolved Hide resolved
src/meta/meta_backup_service.cpp Outdated Show resolved Hide resolved
src/meta/meta_backup_service.cpp Outdated Show resolved Hide resolved
@zhangyifan27 zhangyifan27 changed the title feat(one-time backup): implement one-time backup in meta_backup_service feat(one-time backup): part-1 meta server handle rpc and init backup Mar 5, 2021
src/meta/backup_engine.h Outdated Show resolved Hide resolved
src/meta/backup_engine.h Outdated Show resolved Hide resolved
src/meta/backup_engine.cpp Outdated Show resolved Hide resolved
neverchanje
neverchanje previously approved these changes Mar 8, 2021
src/common/backup.thrift Show resolved Hide resolved
src/meta/backup_engine.h Show resolved Hide resolved

backup_engine::~backup_engine() { _tracker.cancel_outstanding_tasks(); }

error_code backup_engine::get_app_stat(int32_t app_id, std::shared_ptr<app_state> &app)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"stat" and "state" are different. The former is usually the abbr of "statistics."

Suggested change
error_code backup_engine::get_app_stat(int32_t app_id, std::shared_ptr<app_state> &app)
error_code backup_engine::get_app_state(int32_t app_id, /*out*/ std::shared_ptr<app_state> &app)

bool is_backing_up();

private:
error_code get_app_stat(int32_t app_id, std::shared_ptr<app_state> &app);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
error_code get_app_stat(int32_t app_id, std::shared_ptr<app_state> &app);
error_code get_app_stat(int32_t app_id, /*out*/ std::shared_ptr<app_state> &app);

dsn::task_tracker _tracker;

// lock the following variables.
dsn::zlock _lock;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can declare _lock a mutable so that you can let is_backing_up const. Making locks mutable is a convention in C++.

    bool is_backing_up() const;
Suggested change
dsn::zlock _lock;
mutable dsn::zlock _lock;

@neverchanje
Copy link
Contributor

You can do the commented fixes in later pr. No big problems :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants