fix(restore): fix not found error when restore app metadata (#822)

src/client/replication_ddl_client.cpp

@@ -994,9 +994,7 @@ dsn::error_code replication_ddl_client::do_restore(const std::string &backup_pro
         configuration_create_app_response resp;
         dsn::unmarshall(resp_task->get_response(), resp);
         if (resp.err == ERR_OBJECT_NOT_FOUND) {
-            std::cout << "app metadata is damaged on cold backup media, restore app failed"
-                      << std::endl;
-            return ERR_OK;
+            std::cout << "restore app failed: couldn't find valid app metadata" << std::endl;
         } else if (resp.err == ERR_OK) {
             std::cout << "\t"
                       << "new app_id = " << resp.appid << std::endl;

src/meta/meta_service.h

@@ -233,6 +233,7 @@ class meta_service : public serverlet<meta_service>
     friend class meta_split_service_test;
     friend class meta_test_base;
     friend class policy_context_test;
+    friend class server_state_restore_test;
     friend class test::test_checker;
 
     replication_options _opts;

src/meta/server_state_restore.cpp

@@ -1,32 +1,23 @@
-/*
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
 
- * The MIT License (MIT)
- *
- * Copyright (c) 2015 Microsoft Corporation
- *
- * -=- Robust Distributed System Nucleus (rDSN) -=-
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-#include <dsn/dist/block_service.h>
 #include <boost/lexical_cast.hpp>
+#include <dsn/dist/block_service.h>
+#include <dsn/dist/fmt_logging.h>
 #include <dsn/utility/filesystem.h>
 
 #include "block_service/block_service_manager.h"
@@ -56,10 +47,9 @@ void server_state::sync_app_from_backup_media(
         return;
     }
 
-    std::string cluster_root = request.cluster_name;
-    std::string backup_root;
+    std::string backup_root = request.cluster_name;
     if (request.__isset.restore_path) {
-        backup_root = dsn::utils::filesystem::path_combine(request.restore_path, cluster_root);
+        backup_root = dsn::utils::filesystem::path_combine(request.restore_path, backup_root);
     }
     if (!request.policy_name.empty()) {
         backup_root = dsn::utils::filesystem::path_combine(backup_root, request.policy_name);
@@ -69,7 +59,7 @@ void server_state::sync_app_from_backup_media(
 
     error_code err = ERR_OK;
     block_file_ptr file_handle = nullptr;
-    ddebug("in sync_app_from_backup_media and start to create file(%s)", app_metadata.c_str());
+    ddebug_f("start to create metadata file {}", app_metadata);
     blk_fs
         ->create_file(create_file_request{app_metadata, true},
                       TASK_CODE_EXEC_INLINED,
@@ -78,10 +68,9 @@ void server_state::sync_app_from_backup_media(
                           file_handle = resp.file_handle;
                       })
         ->wait();
-    ddebug("after create app_metadata file(%s)", app_metadata.c_str());
 
     if (err != ERR_OK) {
-        derror("create file failed for meta entry(%s)", app_metadata.c_str());
+        derror_f("create metadata file {} failed.", app_metadata);
         callback_tsk->enqueue_with(err, dsn::blob());
         return;
     }
@@ -90,8 +79,6 @@ void server_state::sync_app_from_backup_media(
         read_request{0, -1}, TASK_CODE_EXEC_INLINED, [callback_tsk](const read_response &resp) {
             callback_tsk->enqueue_with(resp.err, resp.buffer);
         });
-    ddebug("after read app_metadata");
-    return;
 }
 
 std::pair<dsn::error_code, std::shared_ptr<app_state>> server_state::restore_app_info(
@@ -102,7 +89,7 @@ std::pair<dsn::error_code, std::shared_ptr<app_state>> server_state::restore_app
     dsn::app_info info;
     if (!::dsn::json::json_forwarder<dsn::app_info>::decode(app_info, info)) {
         std::string b_str(app_info.data(), app_info.length());
-        derror("restore app failed, because app_metadata is damaged, app_info(%s)", b_str.c_str());
+        derror_f("decode app_info '{}' failed", b_str);
         // NOTICE : maybe find a better error_code to replace err_corruption
         res.first = ERR_CORRUPTION;
         return res;

src/meta/test/server_state_restore_test.cpp

@@ -31,8 +31,7 @@ class server_state_restore_test : public meta_test_base
 {
 public:
     server_state_restore_test()
-        : _mock_backup_id(dsn_now_ms()),
-          _old_app_name("test_table"),
+        : _old_app_name("test_table"),
           _new_app_name("new_table"),
           _cluster_name("onebox"),
           _provider("local_service")
@@ -43,38 +42,81 @@ class server_state_restore_test : public meta_test_base
     {
         meta_test_base::SetUp();
 
-        // create an app with 8 partitions.
+        // create a test app with 8 partitions.
         create_app(_old_app_name);
     }
 
-    void test_restore_app_info(const std::string user_specified_restore_path = "")
+    start_backup_app_response start_backup(int64_t app_id,
+                                           const std::string user_specified_path = "")
     {
-        int64_t old_app_id;
-        dsn::blob app_info_data;
-        {
-            zauto_read_lock l;
-            _ss->lock_read(l);
-            const std::shared_ptr<app_state> &app = _ss->get_app(_old_app_name);
-            old_app_id = app->app_id;
-            app_info_data = dsn::json::json_forwarder<app_info>::encode(*app);
+        auto request = dsn::make_unique<start_backup_app_request>();
+        request->app_id = app_id;
+        request->backup_provider_type = _provider;
+        if (!user_specified_path.empty()) {
+            request->__set_backup_path(user_specified_path);
         }
 
+        start_backup_app_rpc rpc(std::move(request), RPC_CM_START_BACKUP_APP);
+        _ms->_backup_handler =
+            std::make_shared<backup_service>(_ms.get(), "mock_policy_root", _cluster_name, nullptr);
+        _ms->_backup_handler->start_backup_app(rpc);
+        wait_all();
+        return rpc.response();
+    }
+
+    configuration_restore_request create_restore_request(
+        int32_t old_app_id, int64_t backup_id, const std::string user_specified_restore_path = "")
+    {
         configuration_restore_request req;
         req.app_id = old_app_id;
         req.app_name = _old_app_name;
         req.new_app_name = _new_app_name;
-        req.time_stamp = _mock_backup_id;
+        req.time_stamp = backup_id;
         req.cluster_name = _cluster_name;
         req.backup_provider_name = _provider;
         if (!user_specified_restore_path.empty()) {
             req.__set_restore_path(user_specified_restore_path);
         }
-        int32_t new_app_id = _ss->next_app_id();
+        return req;
+    }
+
+    void test_restore_app(const std::string user_specified_path = "")
+    {
+        int32_t old_app_id;
+        {
+            zauto_read_lock l;
+            _ss->lock_read(l);
+            const std::shared_ptr<app_state> &app = _ss->get_app(_old_app_name);
+            old_app_id = app->app_id;
+        }
+
+        // test backup app
+        auto backup_resp = start_backup(old_app_id, user_specified_path);
+        ASSERT_EQ(ERR_OK, backup_resp.err);
+        ASSERT_TRUE(backup_resp.__isset.backup_id);
+        int64_t backup_id = backup_resp.backup_id;
 
+        // test sync_app_from_backup_media()
+        auto req = create_restore_request(old_app_id, backup_id, user_specified_path);
         dsn::message_ex *msg = dsn::message_ex::create_request(RPC_CM_START_RESTORE);
         dsn::marshall(msg, req);
-        auto pair = _ss->restore_app_info(msg, req, app_info_data);
+        error_code ret = ERR_UNKNOWN;
+        dsn::blob app_info;
+        _ss->sync_app_from_backup_media(
+            req, [&ret, &app_info](dsn::error_code err, const dsn::blob &app_info_data) {
+                ret = err;
+                app_info = app_info_data;
+            });
+        while (ret == ERR_UNKNOWN) {
+            // sleep 10 ms.
+            usleep(10 * 1000);
+        }
+        ASSERT_EQ(ERR_OK, ret);
+        ASSERT_LT(0, app_info.length());
 
+        // test restore_app_info()
+        int32_t new_app_id = _ss->next_app_id();
+        auto pair = _ss->restore_app_info(msg, req, app_info);
         ASSERT_EQ(ERR_OK, pair.first);
         const std::shared_ptr<app_state> &new_app = pair.second;
         ASSERT_EQ(new_app_id, new_app->app_id);
@@ -96,27 +138,26 @@ class server_state_restore_test : public meta_test_base
         ASSERT_EQ(std::to_string(old_app_id), it->second);
         it = new_app->envs.find(backup_restore_constant::BACKUP_ID);
         ASSERT_NE(new_app->envs.end(), it);
-        ASSERT_EQ(std::to_string(_mock_backup_id), it->second);
-        if (!user_specified_restore_path.empty()) {
+        ASSERT_EQ(std::to_string(backup_id), it->second);
+        if (!user_specified_path.empty()) {
             it = new_app->envs.find(backup_restore_constant::RESTORE_PATH);
             ASSERT_NE(new_app->envs.end(), it);
-            ASSERT_EQ(user_specified_restore_path, it->second);
+            ASSERT_EQ(user_specified_path, it->second);
         }
     }
 
 protected:
-    int64_t _mock_backup_id;
     const std::string _old_app_name;
     const std::string _new_app_name;
     const std::string _cluster_name;
     const std::string _provider;
 };
 
-TEST_F(server_state_restore_test, test_restore_app) { test_restore_app_info(); }
+TEST_F(server_state_restore_test, test_restore_app) { test_restore_app(); }
 
 TEST_F(server_state_restore_test, test_restore_app_with_specific_path)
 {
-    test_restore_app_info("test_path");
+    test_restore_app("test_path");
 }
 
 } // namespace replication