This is an automated email from the ASF dual-hosted git repository.
laiyingchun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pegasus.git
The following commit(s) were added to refs/heads/master by this push:
new efe950dba feat(backup): Add --force option for 'disable_backup_policy'
shell command (#2057)
efe950dba is described below
commit efe950dba5262cdd2f3d91c825ed08b1ab383f47
Author: Yingchun Lai <[email protected]>
AuthorDate: Thu Jul 4 15:42:04 2024 +0800
feat(backup): Add --force option for 'disable_backup_policy' shell command
(#2057)
Before this patch, once a backup policy is added and enabled, it's
impossible to disable it when a new job of the policy is starting,
even if there are some reasons block the job to complete.
This patch add a new flag '-f|--force' to disable the policy by force,
then it's possible to stop the job after restarting the servers.
---
idl/backup.thrift | 7 ++++-
src/client/replication_ddl_client.cpp | 4 ++-
src/client/replication_ddl_client.h | 2 +-
src/meta/meta_backup_service.cpp | 13 ++++++---
src/shell/commands.h | 1 +
src/shell/commands/cold_backup.cpp | 51 +++++++++++++++++------------------
src/shell/main.cpp | 2 +-
7 files changed, 46 insertions(+), 34 deletions(-)
diff --git a/idl/backup.thrift b/idl/backup.thrift
index 2fdeded17..a73fa12e1 100644
--- a/idl/backup.thrift
+++ b/idl/backup.thrift
@@ -77,7 +77,12 @@ struct configuration_modify_backup_policy_request
4:optional i64 new_backup_interval_sec;
5:optional i32 backup_history_count_to_keep;
6:optional bool is_disable;
- 7:optional string start_time; // restrict the start time of each
backup, hour:minute
+
+ // Restrict the start time of each backup, in the form of 'hh:mm', for
example '02:05'.
+ 7:optional string start_time;
+
+ // Force disable the policy, even if the policy is in during backup.
+ 8:optional bool force_disable;
}
struct configuration_modify_backup_policy_response
diff --git a/src/client/replication_ddl_client.cpp
b/src/client/replication_ddl_client.cpp
index d2086e8fb..c0d82ad67 100644
--- a/src/client/replication_ddl_client.cpp
+++ b/src/client/replication_ddl_client.cpp
@@ -1079,11 +1079,13 @@ error_with<query_backup_status_response>
replication_ddl_client::query_backup(in
return call_rpc_sync(query_backup_status_rpc(std::move(req),
RPC_CM_QUERY_BACKUP_STATUS));
}
-dsn::error_code replication_ddl_client::disable_backup_policy(const
std::string &policy_name)
+dsn::error_code replication_ddl_client::disable_backup_policy(const
std::string &policy_name,
+ bool force)
{
auto req = std::make_shared<configuration_modify_backup_policy_request>();
req->policy_name = policy_name;
req->__set_is_disable(true);
+ req->__set_force_disable(force);
auto resp_task = request_meta(RPC_CM_MODIFY_BACKUP_POLICY, req);
diff --git a/src/client/replication_ddl_client.h
b/src/client/replication_ddl_client.h
index b36b2cf32..4e91636cc 100644
--- a/src/client/replication_ddl_client.h
+++ b/src/client/replication_ddl_client.h
@@ -181,7 +181,7 @@ public:
dsn::error_code ls_backup_policy(bool json);
- dsn::error_code disable_backup_policy(const std::string &policy_name);
+ dsn::error_code disable_backup_policy(const std::string &policy_name, bool
force);
dsn::error_code enable_backup_policy(const std::string &policy_name);
diff --git a/src/meta/meta_backup_service.cpp b/src/meta/meta_backup_service.cpp
index 6423e40b1..95a53d70e 100644
--- a/src/meta/meta_backup_service.cpp
+++ b/src/meta/meta_backup_service.cpp
@@ -1634,9 +1634,16 @@ void
backup_service::modify_backup_policy(configuration_modify_backup_policy_rpc
if (request.__isset.is_disable) {
if (request.is_disable) {
if (is_under_backup) {
- LOG_INFO("{}: policy is under backuping, not allow to disable",
- cur_policy.policy_name);
- response.err = ERR_BUSY;
+ if (request.__isset.force_disable && request.force_disable) {
+ LOG_INFO("{}: policy is under backuping, force to disable",
+ cur_policy.policy_name);
+ cur_policy.is_disable = true;
+ have_modify_policy = true;
+ } else {
+ LOG_INFO("{}: policy is under backuping, not allow to
disable",
+ cur_policy.policy_name);
+ response.err = ERR_BUSY;
+ }
} else if (!cur_policy.is_disable) {
LOG_INFO("{}: policy is marked to disable",
cur_policy.policy_name);
cur_policy.is_disable = true;
diff --git a/src/shell/commands.h b/src/shell/commands.h
index 24754aa84..3ea3c132c 100644
--- a/src/shell/commands.h
+++ b/src/shell/commands.h
@@ -229,6 +229,7 @@ bool ls_backup_policy(command_executor *e, shell_context
*sc, arguments args);
bool modify_backup_policy(command_executor *e, shell_context *sc, arguments
args);
+extern const std::string disable_backup_policy_help;
bool disable_backup_policy(command_executor *e, shell_context *sc, arguments
args);
bool enable_backup_policy(command_executor *e, shell_context *sc, arguments
args);
diff --git a/src/shell/commands/cold_backup.cpp
b/src/shell/commands/cold_backup.cpp
index 35cc2ff2e..0a154d461 100644
--- a/src/shell/commands/cold_backup.cpp
+++ b/src/shell/commands/cold_backup.cpp
@@ -20,6 +20,7 @@
// IWYU pragma: no_include <bits/getopt_core.h>
#include <boost/cstdint.hpp>
#include <boost/lexical_cast.hpp>
+// IWYU pragma: no_include <ext/alloc_traits.h>
#include <getopt.h>
#include <inttypes.h>
#include <stdio.h>
@@ -31,6 +32,7 @@
#include <memory>
#include <set>
#include <string>
+#include <utility>
#include <vector>
#include "client/replication_ddl_client.h"
@@ -162,7 +164,7 @@ bool query_backup_policy(command_executor *e, shell_context
*sc, arguments args)
const std::string query_backup_policy_help =
"<-p|--policy_name> [-b|--backup_info_cnt] [-j|--json]";
argh::parser cmd(args.argc, args.argv,
argh::parser::PREFER_PARAM_FOR_UNREG_OPTION);
- RETURN_FALSE_IF_NOT(cmd.params().size() >= 1,
+ RETURN_FALSE_IF_NOT(!cmd.params().empty(),
"invalid command, should be in the form of '{}'",
query_backup_policy_help);
@@ -303,37 +305,32 @@ bool modify_backup_policy(command_executor *e,
shell_context *sc, arguments args
return true;
}
+const std::string disable_backup_policy_help = "<-p|--policy_name str>
[-f|--force]";
bool disable_backup_policy(command_executor *e, shell_context *sc, arguments
args)
{
- static struct option long_options[] = {{"policy_name", required_argument,
0, 'p'},
- {0, 0, 0, 0}};
+ const argh::parser cmd(args.argc, args.argv,
argh::parser::PREFER_PARAM_FOR_UNREG_OPTION);
+ // TODO(yingchun): make the following code as a function.
+ RETURN_FALSE_IF_NOT(cmd.pos_args().size() == 1 && cmd.pos_args()[0] ==
"disable_backup_policy",
+ "invalid command, should be in the form of '{}'",
+ disable_backup_policy_help);
+ RETURN_FALSE_IF_NOT(cmd.flags().empty() ||
+ (cmd.flags().size() == 1 &&
+ (cmd.flags().count("force") == 1 ||
cmd.flags().count("f") == 1)),
+ "invalid command, should be in the form of '{}'",
+ disable_backup_policy_help);
+ RETURN_FALSE_IF_NOT(cmd.params().size() == 1 &&
(cmd.params().begin()->first == "policy_name" ||
+
cmd.params().begin()->first == "p"),
+ "invalid command, should be in the form of '{}'",
+ disable_backup_policy_help);
- std::string policy_name;
- optind = 0;
- while (true) {
- int option_index = 0;
- int c;
- c = getopt_long(args.argc, args.argv, "p:", long_options,
&option_index);
- if (c == -1)
- break;
- switch (c) {
- case 'p':
- policy_name = optarg;
- break;
- default:
- return false;
- }
- }
+ const std::string policy_name = cmd({"-p", "--policy_name"}).str();
+ RETURN_FALSE_IF_NOT(!policy_name.empty(), "invalid command, policy_name
should not be empty");
- if (policy_name.empty()) {
- fprintf(stderr, "empty policy name\n");
- return false;
- }
+ const bool force = cmd[{"-f", "--force"}];
- ::dsn::error_code ret = sc->ddl_client->disable_backup_policy(policy_name);
- if (ret != dsn::ERR_OK) {
- fprintf(stderr, "disable backup policy failed, with err = %s\n",
ret.to_string());
- }
+ const auto ret = sc->ddl_client->disable_backup_policy(policy_name, force);
+ RETURN_FALSE_IF_NOT(
+ ret == dsn::ERR_OK, "disable backup policy failed, with err = {}",
ret.to_string());
return true;
}
diff --git a/src/shell/main.cpp b/src/shell/main.cpp
index 34bddc4c6..cb333a198 100644
--- a/src/shell/main.cpp
+++ b/src/shell/main.cpp
@@ -436,7 +436,7 @@ static command_executor commands[] = {
{
"disable_backup_policy",
"stop policy continue backup",
- "<-p|--policy_name str>",
+ disable_backup_policy_help.c_str(),
disable_backup_policy,
},
{
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
