From 0e4987915a150642de8603abc0c685a4c99187aa Mon Sep 17 00:00:00 2001
From: Yingchun Lai <laiyingchun@apache.org>
Date: Mon, 4 Dec 2023 14:10:24 +0800
Subject: [PATCH] refactor(backup&restore): add more logs when backup or
 restore failed (#1702)

Add more logs when backup or restore failed, it's useful for debuging and troubleshooting.
---
 .../block_service/block_service_manager.cpp   | 10 +++----
 src/rdsn/src/meta/meta_backup_service.cpp     | 27 +++++++++++++++++++
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/src/rdsn/src/block_service/block_service_manager.cpp b/src/rdsn/src/block_service/block_service_manager.cpp
index fb63e6b976..65471f417c 100644
--- a/src/rdsn/src/block_service/block_service_manager.cpp
+++ b/src/rdsn/src/block_service/block_service_manager.cpp
@@ -74,7 +74,7 @@ block_filesystem *block_service_manager::get_or_create_block_filesystem(const st
     if (fs == nullptr) {
         derror_f("acquire block filesystem failed, provider = {}, provider_type = {}",
                  provider,
-                 std::string(provider_type));
+                 provider_type);
         return nullptr;
     }
 
@@ -85,13 +85,13 @@ block_filesystem *block_service_manager::get_or_create_block_filesystem(const st
     utils::split_args(arguments, args);
     dsn::error_code err = fs->initialize(args);
 
+    const auto provider_desc = fmt::format(
+        "provider = {}, provider_type = {}, args = {}", provider, provider_type, arguments);
     if (dsn::ERR_OK == err) {
-        ddebug_f("create block filesystem ok for provider {}", provider);
+        ddebug_f("create block filesystem ok for {}", provider_desc);
         _fs_map.emplace(provider, std::unique_ptr<block_filesystem>(fs));
     } else {
-        derror_f("create block file system err {} for provider {}",
-                 std::string(err.to_string()),
-                 provider);
+        derror_f("create block filesystem failed for {}, error = {}", provider_desc, err);
         delete fs;
         fs = nullptr;
     }
diff --git a/src/rdsn/src/meta/meta_backup_service.cpp b/src/rdsn/src/meta/meta_backup_service.cpp
index b8dc3ef364..ffc2a1e122 100644
--- a/src/rdsn/src/meta/meta_backup_service.cpp
+++ b/src/rdsn/src/meta/meta_backup_service.cpp
@@ -25,6 +25,7 @@
 #include <dsn/tool-api/rpc_address.h>
 #include <dsn/tool-api/rpc_message.h>
 #include <dsn/tool-api/task_code.h>
+#include <dsn/utility/defer.h>
 #include <dsn/utility/filesystem.h>
 #include <dsn/utility/output_utils.h>
 #include <dsn/utils/time_utils.h>
@@ -1240,6 +1241,11 @@ void backup_service::add_backup_policy(dsn::message_ex *msg)
 {
     configuration_add_backup_policy_request request;
     configuration_add_backup_policy_response response;
+    auto log_on_failed = dsn::defer([&response]() {
+        if (!response.hint_message.empty()) {
+            dwarn_f(response.hint_message);
+        }
+    });
 
     dsn::message_ex *copied_msg = message_ex::copy_message_no_reply(*msg);
     ::dsn::unmarshall(msg, request);
@@ -1423,6 +1429,11 @@ void backup_service::query_backup_policy(query_backup_policy_rpc rpc)
 {
     const configuration_query_backup_policy_request &request = rpc.request();
     configuration_query_backup_policy_response &response = rpc.response();
+    auto log_on_failed = dsn::defer([&response]() {
+        if (!response.hint_msg.empty()) {
+            dwarn_f(response.hint_msg);
+        }
+    });
 
     response.err = ERR_OK;
 
@@ -1494,6 +1505,12 @@ void backup_service::modify_backup_policy(configuration_modify_backup_policy_rpc
     configuration_modify_backup_policy_response &response = rpc.response();
     response.err = ERR_OK;
 
+    auto log_on_failed = dsn::defer([&response]() {
+        if (!response.hint_message.empty()) {
+            dwarn_f(response.hint_message);
+        }
+    });
+
     std::shared_ptr<policy_context> context_ptr;
     {
         zauto_lock _(_lock);
@@ -1649,6 +1666,11 @@ void backup_service::start_backup_app(start_backup_app_rpc rpc)
 {
     const start_backup_app_request &request = rpc.request();
     start_backup_app_response &response = rpc.response();
+    auto log_on_failed = dsn::defer([&response]() {
+        if (!response.hint_message.empty()) {
+            dwarn_f(response.hint_message);
+        }
+    });
 
     int32_t app_id = request.app_id;
     std::shared_ptr<backup_engine> engine = std::make_shared<backup_engine>(this);
@@ -1714,6 +1736,11 @@ void backup_service::query_backup_status(query_backup_status_rpc rpc)
 {
     const query_backup_status_request &request = rpc.request();
     query_backup_status_response &response = rpc.response();
+    auto log_on_failed = dsn::defer([&response]() {
+        if (!response.hint_message.empty()) {
+            dwarn_f(response.hint_message);
+        }
+    });
 
     int32_t app_id = request.app_id;
     {