fix(duplication): dup would get stuck in DS_PREPARE indefinitely on…

…ce some error occurred during creating follower table (apache#2144)

Fix apache#2146.

As is described in issue, once there are some errors occurred during the follower
table is being created, the follower cluster might replied with `ERR_APP_EXIST`
to the source cluster. Then, the source cluster would continue to send the same
request to create table on the follower cluster again. As such, the dup would get
stuck in the current status, namely `DS_PREPARE`, and could not forward to the
next status.

The core idea to solve this problem is that we should make the request that create
table on follower cluster idempotent. The follower cluster should process the repeated
requests correctly. To implement this, an environment variable marking creating status
is added and sent together with the request to the follower cluster. The follower cluster
would choose to commit it to meta data on both remote storage and local memory, or
just ignore it, or reply to source cluster with some error code.

.clang-tidy

@@ -20,7 +20,7 @@
 CheckOptions: []
 # Disable some checks that are not useful for us now.
 # They are sorted by names, and should be consistent to build_tools/clang_tidy.py.
-Checks: 'abseil-*,boost-*,bugprone-*,cert-*,clang-analyzer-*,concurrency-*,cppcoreguidelines-*,darwin-*,fuchsia-*,google-*,hicpp-*,linuxkernel-*,llvm-*,misc-*,modernize-*,performance-*,portability-*,readability-*,-bugprone-easily-swappable-parameters,-bugprone-lambda-function-name,-bugprone-macro-parentheses,-cert-err58-cpp,-concurrency-mt-unsafe,-cppcoreguidelines-avoid-c-arrays,-cppcoreguidelines-avoid-magic-numbers,-cppcoreguidelines-avoid-non-const-global-variables,-cppcoreguidelines-macro-usage,-cppcoreguidelines-non-private-member-variables-in-classes,-cppcoreguidelines-owning-memory,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-type-const-cast,-cppcoreguidelines-pro-type-union-access,-fuchsia-default-arguments-calls,-fuchsia-overloaded-operator,-fuchsia-statically-constructed-objects,-google-readability-avoid-underscore-in-googletest-name,-hicpp-avoid-c-arrays,-hicpp-named-parameter,-hicpp-no-array-decay,-llvm-include-order,-misc-definitions-in-headers,-misc-non-private-member-variables-in-classes,-modernize-avoid-c-arrays,-modernize-replace-disallow-copy-and-assign-macro,-modernize-use-trailing-return-type,-readability-function-cognitive-complexity,-readability-identifier-length,-readability-magic-numbers,-readability-named-parameter'
+Checks: 'abseil-*,boost-*,bugprone-*,cert-*,clang-analyzer-*,concurrency-*,cppcoreguidelines-*,darwin-*,fuchsia-*,google-*,hicpp-*,linuxkernel-*,llvm-*,misc-*,modernize-*,performance-*,portability-*,readability-*,-bugprone-easily-swappable-parameters,-bugprone-lambda-function-name,-bugprone-macro-parentheses,-cert-err58-cpp,-concurrency-mt-unsafe,-cppcoreguidelines-avoid-c-arrays,-cppcoreguidelines-avoid-magic-numbers,-cppcoreguidelines-avoid-non-const-global-variables,-cppcoreguidelines-macro-usage,-cppcoreguidelines-non-private-member-variables-in-classes,-cppcoreguidelines-owning-memory,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-type-const-cast,-cppcoreguidelines-pro-type-union-access,-fuchsia-default-arguments-calls,-fuchsia-overloaded-operator,-fuchsia-statically-constructed-objects,-google-readability-avoid-underscore-in-googletest-name,-hicpp-avoid-c-arrays,-hicpp-named-parameter,-hicpp-no-array-decay,-llvm-include-order,-misc-definitions-in-headers,-misc-non-private-member-variables-in-classes,-modernize-avoid-c-arrays,-modernize-replace-disallow-copy-and-assign-macro,-modernize-use-trailing-return-type,-performance-unnecessary-value-param,-readability-function-cognitive-complexity,-readability-identifier-length,-readability-magic-numbers,-readability-named-parameter'
 ExtraArgs:
 ExtraArgsBefore: []
 FormatStyle: none

build_tools/clang_tidy.py

@@ -90,6 +90,7 @@ def tidy_on_path(path):
                            "-modernize-avoid-c-arrays,"
                            "-modernize-replace-disallow-copy-and-assign-macro,"
                            "-modernize-use-trailing-return-type,"
+                           "-performance-unnecessary-value-param,"
                            "-readability-function-cognitive-complexity,"
                            "-readability-identifier-length,"
                            "-readability-magic-numbers,"

src/common/duplication_common.cpp

@@ -55,12 +55,17 @@ namespace replication {
 
 const std::string duplication_constants::kDuplicationCheckpointRootDir /*NOLINT*/ = "duplication";
 const std::string duplication_constants::kClustersSectionName /*NOLINT*/ = "pegasus.clusters";
-const std::string duplication_constants::kDuplicationEnvMasterClusterKey /*NOLINT*/ =
+const std::string duplication_constants::kEnvMasterClusterKey /*NOLINT*/ =
     "duplication.master_cluster";
-const std::string duplication_constants::kDuplicationEnvMasterMetasKey /*NOLINT*/ =
-    "duplication.master_metas";
-const std::string duplication_constants::kDuplicationEnvMasterAppNameKey /*NOLINT*/ =
+const std::string duplication_constants::kEnvMasterMetasKey /*NOLINT*/ = "duplication.master_metas";
+const std::string duplication_constants::kEnvMasterAppNameKey /*NOLINT*/ =
     "duplication.master_app_name";
+const std::string duplication_constants::kEnvFollowerAppStatusKey /*NOLINT*/
+    = "duplication.follower_app_status";
+const std::string duplication_constants::kEnvFollowerAppStatusCreating /*NOLINT*/
+    = "creating";
+const std::string duplication_constants::kEnvFollowerAppStatusCreated /*NOLINT*/
+    = "created";
 
 /*extern*/ const char *duplication_status_to_string(duplication_status::type status)
 {

src/common/duplication_common.h

@@ -81,12 +81,16 @@ struct duplication_constants
     const static std::string kDuplicationCheckpointRootDir;
     const static std::string kClustersSectionName;
     // These will fill into app env and mark one app as a "follower app" and record master info
-    const static std::string kDuplicationEnvMasterClusterKey;
-    const static std::string kDuplicationEnvMasterMetasKey;
-    const static std::string kDuplicationEnvMasterAppNameKey;
+    const static std::string kEnvMasterClusterKey;
+    const static std::string kEnvMasterMetasKey;
+    const static std::string kEnvMasterAppNameKey;
+    const static std::string kEnvFollowerAppStatusKey;
+    const static std::string kEnvFollowerAppStatusCreating;
+    const static std::string kEnvFollowerAppStatusCreated;
 };
 
 USER_DEFINED_ENUM_FORMATTER(duplication_fail_mode::type)
 USER_DEFINED_ENUM_FORMATTER(duplication_status::type)
+
 } // namespace replication
 } // namespace dsn

src/meta/app_env_validator.cpp

@@ -26,6 +26,7 @@
 #include <utility>
 #include <vector>
 
+#include "common/duplication_common.h"
 #include "common/replica_envs.h"
 #include "http/http_status_code.h"
 #include "utils/fmt_logging.h"
@@ -59,7 +60,7 @@ bool app_env_validator::validate_app_envs(const std::map<std::string, std::strin
         }
         std::string hint_message;
         if (!validate_app_env(key, value, hint_message)) {
-            LOG_WARNING("app env '{}={}' is invalid, hint_message: {}", key, value, hint_message);
+            LOG_ERROR("app env '{}={}' is invalid, hint_message: {}", key, value, hint_message);
             return false;
         }
     }
@@ -339,7 +340,12 @@ void app_env_validator::register_all_validators()
           [](int64_t new_value) { return new_value == -1 || new_value >= 1; }}},
         {replica_envs::MANUAL_COMPACT_PERIODIC_BOTTOMMOST_LEVEL_COMPACTION, mcblc},
         {replica_envs::REPLICA_ACCESS_CONTROLLER_ALLOWED_USERS, {ValueType::kString}},
-        {replica_envs::REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES, {ValueType::kString}}};
+        {replica_envs::REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES, {ValueType::kString}},
+        {duplication_constants::kEnvMasterClusterKey, {ValueType::kString}},
+        {duplication_constants::kEnvMasterMetasKey, {ValueType::kString}},
+        {duplication_constants::kEnvMasterAppNameKey, {ValueType::kString}},
+        {duplication_constants::kEnvFollowerAppStatusKey, {ValueType::kString}},
+    };
 }
 
 const std::unordered_map<app_env_validator::ValueType, std::string>