feat(backup): Add --force option for 'disable_backup_policy' shell co…

…mmand (apache#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

@@ -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

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);
 

src/client/replication_ddl_client.h

@@ -181,7 +181,7 @@ class replication_ddl_client
 
     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);
 

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;

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);

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;
 }
 

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,
     },
     {