WL#14870 BUG#33516540 BUG#33199178 BUG#35993066 BUG#35996854 BUG#35996514 BUG#35997891 BUG#35998172 BUG#36000967 AdminAPI: ReplicaSet rescan, dissolve and describe

This patch adds the following new commands to ReplicaSet:
 - dissolve() to dissolve the ReplicaSet
 - rescan() to update the ReplicaSet metadata, which includes updating
server ids, adding new instances (part of the topology but not present
in the MD, removing obsolete instances (not part of the topology but
still in the MD) and storing the current replication accounts.
 - describe() to output a description (JSON) of the ReplicaSet

It also deprecates the "addInstances" and "removeInstances" options from
Cluster.rescan() while adding two new ones, "addUnmanaged" and
"removeObsolete", replacing the deprecated ones with simpler options.

Regarding BUG#33516540, a new check was also added to status() to check
if the replication users aren't present in the MD, which might happen if
upgrading from a version older than 8.0.27. This causes unused
replication accounts to not be removed from the ReplicaSet after
removeInstance() removed the corresponding member. This new check in
status() informs the user to run rescan() which fixes the issue by
storing the account in the MD.

Regarding BUG#33199178, which occurs when calling setPrimaryInstance()
if the server_id var changed (e.g.: due an OS update), it's also fixed
by invoking rescan(), by storing the correct server_id in the MD.

Change-Id: Ia3ce3741ed2fbd8b3888475c51090b77614e7b7d

Author: joaosigma

modules/adminapi/CMakeLists.txt

@@ -75,8 +75,11 @@ file(GLOB adminapi_module_SOURCES
       "cluster_set/create_replica_cluster.cc"
       "cluster_set/status.cc"
       "replica_set/api_options.cc"
+      "replica_set/describe.cc"
+      "replica_set/dissolve.cc"
       "replica_set/replica_set_impl.cc"
       "replica_set/replica_set_status.cc"
+      "replica_set/rescan.cc"
       "cluster/add_instance.cc"
       "cluster/add_replica_instance.cc"
       "cluster/rejoin_instance.cc"

modules/adminapi/cluster/api_options.cc

@@ -22,25 +22,26 @@
  */
 #include "modules/adminapi/cluster/api_options.h"
 
-#include <cinttypes>
-#include <utility>
 #include <vector>
 
 #include "adminapi/common/api_options.h"
 #include "adminapi/common/async_topology.h"
 #include "modules/adminapi/common/common.h"
-#include "mysqlshdk/include/scripting/type_info/custom.h"
-#include "mysqlshdk/include/scripting/type_info/generic.h"
 #include "mysqlshdk/include/shellcore/console.h"
-#include "mysqlshdk/libs/db/utils_connection.h"
-#include "mysqlshdk/libs/utils/utils_file.h"
-#include "shellcore/shell_options.h"
-#include "utils/utils_net.h"
 
 namespace mysqlsh {
 namespace dba {
 namespace cluster {
 
+namespace {
+void throw_rescan_mixing_exception() {
+  throw shcore::Exception::argument_error(shcore::str_format(
+      "Options '%s' and '%s' are mutually exclusive with deprecated options "
+      "'%s' and '%s'. Mixing either one from both groups isn't allowed.",
+      kAddUnmanaged, kRemoveObsolete, kAddInstances, kRemoveInstances));
+}
+}  // namespace
+
 const shcore::Option_pack_def<Add_instance_options>
     &Add_instance_options::options() {
   static const auto opts =
@@ -143,6 +144,8 @@ const shcore::Option_pack_def<Rescan_options> &Rescan_options::options() {
                     &Rescan_options::set_update_topology_mode)
           .optional(kAddInstances, &Rescan_options::set_list_option)
           .optional(kRemoveInstances, &Rescan_options::set_list_option)
+          .optional(kAddUnmanaged, &Rescan_options::set_bool_option)
+          .optional(kRemoveObsolete, &Rescan_options::set_bool_option)
           .optional(kUpgradeCommProtocol,
                     &Rescan_options::upgrade_comm_protocol)
           .optional(kUpdateViewChangeUuid,
@@ -160,8 +163,32 @@ void Rescan_options::set_update_topology_mode(bool /*value*/) {
   console->print_info();
 }
 
+void Rescan_options::set_bool_option(const std::string &option, bool value) {
+  if (m_used_deprecated.has_value() && !m_used_deprecated.value())
+    throw_rescan_mixing_exception();
+
+  m_used_deprecated = true;
+
+  if (option == kAddUnmanaged)
+    auto_add = value;
+  else
+    auto_remove = value;
+}
+
 void Rescan_options::set_list_option(const std::string &option,
                                      const shcore::Value &value) {
+  if (m_used_deprecated.has_value() && m_used_deprecated.value())
+    throw_rescan_mixing_exception();
+
+  m_used_deprecated = false;
+
+  auto console = mysqlsh::current_console();
+  console->print_info(shcore::str_format(
+      "The '%s' and '%s' options are deprecated. Please use '%s' and/or '%s' "
+      "instead.",
+      kAddInstances, kRemoveInstances, kAddUnmanaged, kRemoveObsolete));
+  console->print_info();
+
   std::vector<mysqlshdk::db::Connection_options> *instances_list;
 
   // Selects the target list

modules/adminapi/cluster/api_options.h

@@ -89,16 +89,20 @@ struct Options_options {
 
 struct Rescan_options : public Interactive_option {
   static const shcore::Option_pack_def<Rescan_options> &options();
+  void set_bool_option(const std::string &option, bool value);
   void set_list_option(const std::string &option, const shcore::Value &value);
   void set_update_topology_mode(bool value);
 
-  mysqlshdk::null_bool update_topology_mode;
+  std::optional<bool> update_topology_mode;
   std::vector<mysqlshdk::db::Connection_options> add_instances_list;
   std::vector<mysqlshdk::db::Connection_options> remove_instances_list;
   bool auto_add = false;
   bool auto_remove = false;
   bool upgrade_comm_protocol = false;
-  mysqlshdk::null_bool update_view_change_uuid;
+  std::optional<bool> update_view_change_uuid;
+
+ private:
+  std::optional<bool> m_used_deprecated;
 };
 
 struct Set_primary_instance_options {

modules/adminapi/cluster/describe.cc

@@ -87,7 +87,7 @@ shcore::Array_t Describe::get_topology() {
     shcore::Dictionary_t member = shcore::make_dict();
     feed_metadata_info(member, instance_def);
 
-    instances_list->push_back(shcore::Value(member));
+    instances_list->push_back(shcore::Value(std::move(member)));
   }
 
   return instances_list;
@@ -96,9 +96,7 @@ shcore::Array_t Describe::get_topology() {
 shcore::Dictionary_t Describe::collect_replicaset_description() {
   shcore::Dictionary_t ret = shcore::make_dict();
 
-  // Set Cluster name and topologyMode
   (*ret)["name"] = shcore::Value("default");
-
   (*ret)["topologyMode"] = shcore::Value(m_cluster.get_topology_type());
 
   // Get and set the topology (all instances)
@@ -107,54 +105,13 @@ shcore::Dictionary_t Describe::collect_replicaset_description() {
   return ret;
 }
 
-shcore::Value Describe::get_default_replicaset_description() {
-  // Get the Default Cluster description
-  shcore::Dictionary_t replicaset_dict;
-
-  replicaset_dict = collect_replicaset_description();
-
-  // Check if the Cluster group session is established to an instance with
-  // a state different than
-  //   - Online R/W
-  //   - Online R/O
-  //
-  // Possibly with the state:
-  //
-  //   - RECOVERING
-  //   - OFFLINE
-  //   - ERROR
-  //
-  // If that's the case, a warning must be added to the resulting JSON object
-
-  if (auto group_instance = m_cluster.get_cluster_server()) {
-    auto state = get_replication_group_state(
-        *group_instance, get_gr_instance_type(*group_instance));
-
-    bool warning = (state.source_state != ManagedInstance::OnlineRW &&
-                    state.source_state != ManagedInstance::OnlineRO);
-    if (warning) {
-      std::string warning_msg =
-          "The cluster description may be inaccurate as it was generated from "
-          "an instance in ";
-      warning_msg.append(ManagedInstance::describe(
-          static_cast<ManagedInstance::State>(state.source_state)));
-      warning_msg.append(" state");
-      (*replicaset_dict)["warning"] = shcore::Value(warning_msg);
-    }
-  }
-
-  return shcore::Value(replicaset_dict);
-}
-
 shcore::Value Describe::execute() {
   shcore::Dictionary_t dict = shcore::make_dict();
-
   (*dict)["clusterName"] = shcore::Value(m_cluster.get_name());
+  (*dict)["defaultReplicaSet"] =
+      shcore::Value(collect_replicaset_description());
 
-  // Get the default replicaSet description
-  (*dict)["defaultReplicaSet"] = get_default_replicaset_description();
-
-  return shcore::Value(dict);
+  return shcore::Value(std::move(dict));
 }
 
 void Describe::finish() {

modules/adminapi/cluster/rescan.cc

@@ -223,7 +223,7 @@ void Rescan::prepare() {
                                     Precondition_checker::k_min_cs_version;
 
   // Validate the usage of the option 'updateViewChangeUuid'
-  if (m_options.update_view_change_uuid.get_safe() &&
+  if (m_options.update_view_change_uuid.value_or(false) &&
       !m_is_view_change_uuid_supported) {
     throw shcore::Exception::argument_error(
         "The Cluster cannot be configured to use "
@@ -1052,7 +1052,7 @@ shcore::Value Rescan::execute() {
         "group_replication_view_change_uuid", "");
 
     if (view_change_uuid == "AUTOMATIC") {
-      if (m_options.update_view_change_uuid.is_null() &&
+      if (!m_options.update_view_change_uuid.has_value() &&
           m_cluster->get_lowest_instance_version() <
               k_view_change_uuid_deprecated) {
         console->print_note(
@@ -1073,7 +1073,7 @@ shcore::Value Rescan::execute() {
               "Use the 'updateViewChangeUuid' option to generate and "
               "configure a value for the Cluster.");
         }
-      } else if (m_options.update_view_change_uuid.get_safe()) {
+      } else if (m_options.update_view_change_uuid.value_or(false)) {
         ensure_view_change_uuid_set();
       }
     } else {

modules/adminapi/common/common.cc

@@ -1888,32 +1888,15 @@ bool wait_for_gtid_set_safe(const mysqlshdk::mysql::IInstance &target_instance,
                             const std::string &gtid_set,
                             const std::string &channel_name, int timeout,
                             bool cancelable) {
-  size_t total_transactions_primary, missing_transactions;
-  // The GTID wait will not abort if an error occurs, so calling it without a
-  // timeout will just freeze forever. To prevent that, we call the wait with
-  // smaller incremental timeouts and wait for errors during that.
-
-  // wait for at most 10s at a time
-  int incremental_timeout = std::min(timeout, 10);
-  if (incremental_timeout == 0) incremental_timeout = 2;
-
-  int time_elapsed = 0;
-
   bool stop = false;
-
   shcore::Interrupt_handler intr(
       [&stop]() {
         stop = true;
         return true;
       },
       !cancelable);
 
-  bool sync_res;
-
   // Get the Progress_reporting style option in use
-  auto progress_reporting = current_shell_options()->get().progress_reporting;
-  std::unique_ptr<mysqlshdk::textui::Progress_vt100> progress_bar;
-
   const auto update_progress = [](mysqlshdk::textui::Progress_vt100 *bar,
                                   uint64_t total, size_t missing) {
     bar->set_current(total - missing);
@@ -1923,10 +1906,26 @@ bool wait_for_gtid_set_safe(const mysqlshdk::mysql::IInstance &target_instance,
   // Get the total count of GTID_EXECUTED from the primary
   auto gtid_set_primary =
       mysqlshdk::mysql::Gtid_set::from_normalized_string(gtid_set);
-  total_transactions_primary = gtid_set_primary.count();
+  auto total_transactions_primary =
+      static_cast<size_t>(gtid_set_primary.count());
+
+  // calculate missing transactions
+  const auto calc_missing_transactions = [&target_instance,
+                                          &gtid_set_primary]() -> size_t {
+    auto gtid_set_target =
+        mysqlshdk::mysql::Gtid_set::from_gtid_executed(target_instance);
+
+    return mysqlshdk::mysql::estimate_gtid_set_size(
+        gtid_set_primary.subtract(gtid_set_target, target_instance));
+  };
 
   using Progress_reporting = Shell_options::Storage::Progress_reporting;
 
+  auto progress_reporting = current_shell_options()->get().progress_reporting;
+
+  std::unique_ptr<mysqlshdk::textui::Progress_vt100> progress_bar;
+  shcore::Scoped_callback end_progress_bar;
+
   if (progress_reporting == Progress_reporting::PROGRESSBAR) {
     // Init the progress bar
     progress_bar = std::make_unique<mysqlshdk::textui::Progress_vt100>(0);
@@ -1937,43 +1936,42 @@ bool wait_for_gtid_set_safe(const mysqlshdk::mysql::IInstance &target_instance,
     // Always terminate the progress bar when going out of the scope of the
     // function. That must happen regardless if the function ended successfully
     // or not, or because of an exception, to ensure the output is cleared out.
-    shcore::Scoped_callback end_progress_bar(
-        [&progress_bar]() { progress_bar->end(); });
+    end_progress_bar =
+        shcore::Scoped_callback([&progress_bar]() { progress_bar->end(); });
 
     // Update the progress bar to show the current state of the transactions
     // missing
-    auto gtid_set_target =
-        mysqlshdk::mysql::Gtid_set::from_gtid_executed(target_instance);
-    missing_transactions = mysqlshdk::mysql::estimate_gtid_set_size(
-        gtid_set_primary.subtract(gtid_set_target, target_instance));
-
     update_progress(progress_bar.get(), total_transactions_primary,
-                    missing_transactions);
+                    calc_missing_transactions());
   }
 
+  // The GTID wait will not abort if an error occurs, so calling it without a
+  // timeout will just freeze forever. To prevent that, we call the wait with
+  // smaller incremental timeouts and wait for errors during that.
+  //
+  // wait for at most 10s at a time
+  int incremental_timeout = std::min(timeout, 10);
+  if (incremental_timeout == 0) incremental_timeout = 2;
+
+  bool sync_res;
+  int time_elapsed = 0;
   do {
     // Every minute, get the executed gtid_set at the target instance to check
     // how many transactions are still missing to provide that info to the user
     if (time_elapsed != 0 && time_elapsed % 60 == 0) {
-      auto gtid_set_target =
-          mysqlshdk::mysql::Gtid_set::from_gtid_executed(target_instance);
-
-      missing_transactions = mysqlshdk::mysql::estimate_gtid_set_size(
-          gtid_set_primary.subtract(gtid_set_target, target_instance));
-
       switch (progress_reporting) {
         case Progress_reporting::PROGRESSBAR: {
+          assert(progress_bar);
           update_progress(progress_bar.get(), total_transactions_primary,
-                          missing_transactions);
+                          calc_missing_transactions());
           break;
         }
         case Progress_reporting::SIMPLE: {
           // Simple progress reporting, just print info about the transactions
           // left
-          mysqlsh::current_console()->print_info(
-              "** " + std::to_string(missing_transactions) + " of " +
-              std::to_string(total_transactions_primary) +
-              " transactions left");
+          mysqlsh::current_console()->print_info(shcore::str_format(
+              "** %zu of %zu transactions left", calc_missing_transactions(),
+              total_transactions_primary));
           break;
         }
         default:
@@ -1996,6 +1994,7 @@ bool wait_for_gtid_set_safe(const mysqlshdk::mysql::IInstance &target_instance,
     } else {
       // wait succeeded, get out of here
       if (progress_reporting == Progress_reporting::PROGRESSBAR) {
+        assert(progress_bar);
         update_progress(progress_bar.get(), total_transactions_primary, 0);
       }
       break;

modules/adminapi/common/common.h

@@ -261,6 +261,8 @@ inline constexpr const char kForce[] = "force";
 inline constexpr const char kAdoptFromGR[] = "adoptFromGR";
 inline constexpr const char kAddInstances[] = "addInstances";
 inline constexpr const char kRemoveInstances[] = "removeInstances";
+inline constexpr const char kAddUnmanaged[] = "addUnmanaged";
+inline constexpr const char kRemoveObsolete[] = "removeObsolete";
 inline constexpr const char kRejoinInstances[] = "rejoinInstances";
 inline constexpr const char kWaitRecovery[] = "waitRecovery";
 inline constexpr const char kRecoveryProgress[] = "recoveryProgress";

modules/adminapi/common/metadata_storage.cc

@@ -1323,6 +1323,18 @@ void MetadataStorage::update_instance_repl_account(
   execute_sql(query);
 }
 
+void MetadataStorage::update_instance_uuid(Cluster_id cluster_id,
+                                           Instance_id instance_id,
+                                           std::string_view instance_uuid) {
+  auto query =
+      "UPDATE mysql_innodb_cluster_metadata.instances SET mysql_server_uuid = "
+      "? WHERE (cluster_id = ?) AND (instance_id = ?);"_sql
+      << instance_uuid << cluster_id << instance_id;
+  query.done();
+
+  execute_sql(query);
+}
+
 std::pair<std::string, std::string> MetadataStorage::get_instance_repl_account(
     const std::string &instance_uuid, Cluster_type type,
     Replica_type replica_type) {
@@ -1594,7 +1606,7 @@ size_t MetadataStorage::iterate_recovery_account(
   return num_accounts;
 }
 
-void MetadataStorage::remove_instance(const std::string &instance_address,
+void MetadataStorage::remove_instance(std::string_view instance_address,
                                       Transaction_undo *undo) {
   // Remove the instance
   auto query = ("DELETE FROM mysql_innodb_cluster_metadata.instances "