Bug#37372020 Routing Guideline destination based on custom server label not working

Issue: Label used for guideline matching is not fetched from metadata but instead it is created like the md default label. Custom labels do not work. Fix: Fetch server label from metadata as part of instance information. Change-Id: Idf4c109e587ac31a14d73487e49af1ba01e66612

Author: Tomasz Stepniak

router/src/metadata_cache/include/mysqlrouter/metadata_cache_datatypes.h

@@ -107,7 +107,7 @@ class METADATA_CACHE_EXPORT ManagedInstance {
  const std::string &p_mysql_server_uuid,
  const ServerMode p_mode, const ServerRole p_role,
  const std::string &p_host, const uint16_t p_port,
- const uint16_t p_xport);
+ const uint16_t p_xport, std::string label);
 
  ManagedInstance(const ManagedInstance &) = default;
  ManagedInstance &operator=(const ManagedInstance &) = default;
@@ -149,7 +149,9 @@ class METADATA_CACHE_EXPORT ManagedInstance {
  /** Server tags as defined in the metadata, parsed as a key-value pairs */
  std::map<std::string, std::string, std::less<>> tags{};
 
- uint32_t version;
+ uint32_t version{0};
+
+ std::string label;
 
  std::string to_string() const {
  std::string result = "uuid: " + mysql_server_uuid + "\n";

router/src/metadata_cache/src/cluster_metadata.cc

@@ -370,7 +370,7 @@ void ClusterMetadata::report_guideline_name(
  sqlstring query(
  "UPDATE mysql_innodb_cluster_metadata.v2_routers SET "
  "attributes = JSON_SET(attributes, '$.CurrentRoutingGuideline', ?) "
- "where router_id = ? ");
+ "WHERE router_id = ? ");
 
  if (guideline_name.empty()) {
  query << sqlstring::null;

router/src/metadata_cache/src/cluster_metadata_ar.cc

@@ -181,7 +181,7 @@ metadata_cache::ClusterTopology ARClusterMetadata::fetch_topology_from_member(
  // comparing to other members view of the world
  std::string query =
  "select C.cluster_id, C.cluster_name, M.member_id, I.endpoint, "
- "I.xendpoint, M.member_role, I.attributes from "
+ "I.xendpoint, M.member_role, I.attributes, I.label from "
  "mysql_innodb_cluster_metadata.v2_ar_members M join "
  "mysql_innodb_cluster_metadata.v2_instances I on I.instance_id = "
  "M.instance_id join mysql_innodb_cluster_metadata.v2_ar_clusters C on "
@@ -192,10 +192,10 @@ metadata_cache::ClusterTopology ARClusterMetadata::fetch_topology_from_member(
  }
 
  auto result_processor = [&cluster](const MySQLSession::Row &row) -> bool {
- if (row.size() != 7) {
+ if (row.size() != 8) {
  throw metadata_cache::metadata_error(
  "Unexpected number of fields in the resultset. "
- "Expected = 7, got = " +
+ "Expected = 8, got = " +
  std::to_string(row.size()));
  }
 
@@ -218,6 +218,7 @@ metadata_cache::ClusterTopology ARClusterMetadata::fetch_topology_from_member(
  }
 
  set_instance_attributes(instance, as_string(row[6]));
+ instance.label = as_string(row[7]);
 
  std::string warning;
  if (instance.type == mysqlrouter::InstanceType::AsyncMember) {

router/src/metadata_cache/src/cluster_metadata_gr.cc

@@ -1044,7 +1044,7 @@ GRMetadataBackendV2::fetch_instances_from_metadata_server(
  // obtained from one of the nodes belonging to a quorum.
  std::string query(
  "select C.cluster_id, C.cluster_name, I.mysql_server_uuid, I.endpoint, "
- "I.xendpoint, I.attributes "
+ "I.xendpoint, I.attributes, I.label "
  "from "
  "mysql_innodb_cluster_metadata.v2_instances I join "
  "mysql_innodb_cluster_metadata.v2_gr_clusters C on I.cluster_id = "
@@ -1053,10 +1053,10 @@ GRMetadataBackendV2::fetch_instances_from_metadata_server(
 
  metadata_cache::ManagedCluster cluster;
  auto result_processor = [&cluster](const MySQLSession::Row &row) -> bool {
- if (row.size() != 6) {
+ if (row.size() != 7) {
  throw metadata_cache::metadata_error(
  "Unexpected number of fields in the resultset. "
- "Expected = 6, got = " +
+ "Expected = 7, got = " +
  std::to_string(row.size()));
  }
 
@@ -1067,6 +1067,7 @@ GRMetadataBackendV2::fetch_instances_from_metadata_server(
  return true; // next row
  }
  set_instance_attributes(instance, as_string(row[5]));
+ instance.label = as_string(row[6]);
 
  cluster.id = as_string(row[0]);
  cluster.name = as_string(row[1]);
@@ -1224,6 +1225,7 @@ GRClusterSetMetadataBackend::update_clusterset_topology_from_metadata_server(
 
  std::string query =
  "select I.mysql_server_uuid, I.endpoint, I.xendpoint, I.attributes, "
+ "I.label, "
  "C.cluster_id, C.cluster_name, CSM.member_role, CSM.invalidated, "
  "CS.domain_name "
  "from mysql_innodb_cluster_metadata.v2_instances I join "
@@ -1246,10 +1248,11 @@ GRClusterSetMetadataBackend::update_clusterset_topology_from_metadata_server(
  const std::string node_addr_classic = as_string(row[1]);
  const std::string node_addr_x = as_string(row[2]);
  const std::string node_attributes = as_string(row[3]);
- const std::string cluster_id = as_string(row[4]);
- const std::string cluster_name = as_string(row[5]);
- const bool cluster_is_primary = as_string(row[6]) == "PRIMARY";
- const bool cluster_is_invalidated = strtoui_checked(row[7]) == 1;
+ const std::string label = as_string(row[4]);
+ const std::string cluster_id = as_string(row[5]);
+ const std::string cluster_name = as_string(row[6]);
+ const bool cluster_is_primary = as_string(row[7]) == "PRIMARY";
+ const bool cluster_is_invalidated = strtoui_checked(row[8]) == 1;
 
  if (result.clusters_data.empty() ||
  result.clusters_data.back().name != cluster_name) {
@@ -1271,15 +1274,16 @@ GRClusterSetMetadataBackend::update_clusterset_topology_from_metadata_server(
  metadata_cache::ServerRole::Secondary,
  uri_classic.host,
  uri_classic.port,
- uri_x.port};
+ uri_x.port,
+ label};
 
  set_instance_attributes(instance, node_attributes);
 
  if (instance.type == mysqlrouter::InstanceType::GroupMember ||
  instance.type == mysqlrouter::InstanceType::ReadReplica) {
  result.clusters_data.back().members.push_back(instance);
  if (result.name.empty()) {
- result.name = as_string(row[8]);
+ result.name = as_string(row[9]);
  }
  } else {
  log_warning("Ignoring unsupported instance %s:%d, type: %s",

router/src/metadata_cache/src/metadata_cache.cc

@@ -250,13 +250,13 @@ bool metadata_cache::ManagedInstance::operator==(
  disconnect_existing_sessions_when_hidden ==
  other.disconnect_existing_sessions_when_hidden &&
  ignore == other.ignore && tags == other.tags &&
- version == other.version;
+ version == other.version && label == other.label;
 }
 
 metadata_cache::ManagedInstance::ManagedInstance(
  mysqlrouter::InstanceType p_type, const std::string &p_mysql_server_uuid,
  const ServerMode p_mode, const ServerRole p_role, const std::string &p_host,
- const uint16_t p_port, const uint16_t p_xport)
+ const uint16_t p_port, const uint16_t p_xport, std::string label)
  : type(p_type),
  mysql_server_uuid(p_mysql_server_uuid),
  mode(p_mode),
@@ -266,7 +266,8 @@ metadata_cache::ManagedInstance::ManagedInstance(
  xport(p_xport),
  hidden(mysqlrouter::kNodeTagHiddenDefault),
  disconnect_existing_sessions_when_hidden(
- mysqlrouter::kNodeTagDisconnectWhenHiddenDefault) {}
+ mysqlrouter::kNodeTagDisconnectWhenHiddenDefault),
+ label(std::move(label)) {}
 
 metadata_cache::ManagedInstance::ManagedInstance(
  mysqlrouter::InstanceType p_type)

router/src/metadata_cache/tests/helper/mock_metadata.cc

@@ -44,18 +44,24 @@ MockNG::MockNG(
  ms1.port = 3306;
  ms1.xport = 33060;
  ms1.mode = metadata_cache::ServerMode::ReadWrite;
+ ms1.label = ms1.host + ":" + std::to_string(ms1.port);
+ ms1.tags = {};
 
  ms2.mysql_server_uuid = "instance-2";
  ms2.host = "host-2";
  ms2.port = 3306;
  ms2.xport = 33060;
  ms2.mode = metadata_cache::ServerMode::ReadOnly;
+ ms2.label = ms2.host + ":" + std::to_string(ms2.port);
+ ms2.tags = {};
 
  ms3.mysql_server_uuid = "instance-3";
  ms3.host = "host-3";
  ms3.port = 3306;
  ms3.xport = 33060;
  ms3.mode = metadata_cache::ServerMode::ReadOnly;
+ ms3.label = ms3.host + ":" + std::to_string(ms3.port);
+ ms3.tags = {};
 
  cluster_instances_vector.push_back(ms1);
  cluster_instances_vector.push_back(ms2);

router/src/router/src/cluster_metadata.cc

@@ -1027,7 +1027,7 @@ static std::vector<std::string> do_get_routing_mode_queries(
  MySQLSession *mysql) {
  const std::string fetch_instances_query =
  "select C.cluster_id, C.cluster_name, I.mysql_server_uuid, "
- "I.endpoint, I.xendpoint, I.attributes "
+ "I.endpoint, I.xendpoint, I.attributes, I.label "
  "from mysql_innodb_cluster_metadata.v2_instances I join "
  "mysql_innodb_cluster_metadata.v2_gr_clusters C on I.cluster_id = "
  "C.cluster_id where C.cluster_name = " +
@@ -1122,7 +1122,7 @@ std::string ClusterMetadataAR::get_cluster_type_specific_id() {
 std::vector<std::string> ClusterMetadataAR::get_routing_mode_queries() {
  return {// source: ClusterMetadata::fetch_instances_from_metadata_server()
  "select C.cluster_id, C.cluster_name, I.mysql_server_uuid, "
- "I.endpoint, I.xendpoint, I.attributes from "
+ "I.endpoint, I.xendpoint, I.attributes, I.label from "
  "mysql_innodb_cluster_metadata.v2_instances I join "
  "mysql_innodb_cluster_metadata.v2_gr_clusters C on I.cluster_id = "
  "C.cluster_id where C.cluster_name = " +

router/src/routing/src/dest_metadata_cache.cc

@@ -291,10 +291,7 @@ DestMetadataCacheManager::get_nodes_from_topology(
  instance_info.cluster_set_name = clusterset_name;
  instance_info.cluster_role = cluster_role;
  instance_info.cluster_name = cluster_name;
- const auto label_port =
- cluster_member.port == 0 ? cluster_member.xport : cluster_member.port;
- instance_info.label =
- cluster_member.host + ":" + std::to_string(label_port);
+ instance_info.label = cluster_member.label;
  instance_info.cluster_is_invalidated = cluster.is_invalidated;
  instance_info.version = cluster_member.version;
 

router/src/routing/tests/test_metadata_cache_group.cc

@@ -203,9 +203,9 @@ TEST_F(DestMetadataCacheTest, AllowedNodesNoPrimary) {
 
  fill_instance_vector({
  {GR, "uuid1", ServerMode::ReadWrite, ServerRole::Primary, "3306", 3306,
- 33060},
+ 33060, "label"},
  {GR, "uuid2", ServerMode::ReadOnly, ServerRole::Secondary, "3307", 3307,
- 33070},
+ 33070, "label"},
  });
 
  EXPECT_CALL(metadata_cache_api_, add_acceptor_handler_listener(_));
@@ -215,9 +215,9 @@ TEST_F(DestMetadataCacheTest, AllowedNodesNoPrimary) {
  // new metadata - no primary
  fill_instance_vector({
  {GR, "uuid1", ServerMode::ReadOnly, ServerRole::Secondary, "3306", 3306,
- 33060},
+ 33060, "label"},
  {GR, "uuid2", ServerMode::ReadOnly, ServerRole::Secondary, "3307", 3307,
- 33070},
+ 33070, "label"},
  });
 
  bool callback_called{false};
@@ -255,9 +255,9 @@ TEST_F(DestMetadataCacheTest, AllowedNodes2Primaries) {
 
  InstanceVector instances{
  {GR, "uuid1", ServerMode::ReadWrite, ServerRole::Primary, "3306", 3306,
- 33060},
+ 33060, "label"},
  {GR, "uuid2", ServerMode::ReadOnly, ServerRole::Secondary, "3307", 3307,
- 33070},
+ 33070, "label"},
  };
 
  fill_instance_vector(instances);
@@ -315,9 +315,9 @@ TEST_F(DestMetadataCacheTest, AllowedNodesNoSecondaries) {
 
  InstanceVector instances{
  {GR, "uuid1", ServerMode::ReadWrite, ServerRole::Primary, "3306", 3306,
- 33060},
+ 33060, "label"},
  {GR, "uuid2", ServerMode::ReadOnly, ServerRole::Secondary, "3307", 3307,
- 33070},
+ 33070, "label"},
  };
 
  fill_instance_vector(instances);
@@ -372,9 +372,9 @@ TEST_F(DestMetadataCacheTest, AllowedNodesSecondaryDisconnectToPromoted) {
 
  InstanceVector instances{
  {GR, "uuid1", ServerMode::ReadWrite, ServerRole::Primary, "3306", 3306,
- 33060},
+ 33060, "label"},
  {GR, "uuid2", ServerMode::ReadOnly, ServerRole::Secondary, "3307", 3307,
- 33070},
+ 33070, "label"},
  };
 
  fill_instance_vector(instances);
@@ -434,9 +434,9 @@ TEST_F(DestMetadataCacheTest, AllowedNodesSecondaryDisconnectToPromotedTwice) {
 
  InstanceVector instances{
  {GR, "uuid1", ServerMode::ReadWrite, ServerRole::Primary, "3306", 3306,
- 33060},
+ 33060, "label"},
  {GR, "uuid2", ServerMode::ReadOnly, ServerRole::Secondary, "3307", 3307,
- 33070},
+ 33070, "label"},
  };
 
  fill_instance_vector(instances);
@@ -488,9 +488,9 @@ TEST_F(DestMetadataCacheTest,
 
  fill_instance_vector({
  {GR, "uuid1", ServerMode::ReadWrite, ServerRole::Primary, "3306", 3306,
- 33060},
+ 33060, "label"},
  {GR, "uuid2", ServerMode::ReadOnly, ServerRole::Secondary, "3307", 3307,
- 33070},
+ 33070, "label"},
  });
 
  EXPECT_CALL(metadata_cache_api_, add_acceptor_handler_listener(_));
@@ -542,9 +542,9 @@ TEST_F(DestMetadataCacheTest,
 
  fill_instance_vector({
  {GR, "uuid1", ServerMode::ReadWrite, ServerRole::Primary, "3306", 3306,
- 33060},
+ 33060, "label"},
  {GR, "uuid2", ServerMode::ReadOnly, ServerRole::Secondary, "3307", 3307,
- 33070},
+ 33070, "label"},
  });
 
  EXPECT_CALL(metadata_cache_api_, add_acceptor_handler_listener(_));