[BACKPORT 2.14.10][#17712] DocDB: Fix system partitions query to not …

…return empty namespace name when its not set at table level Summary: Original commit: 218253c / D26033 Tables created before version 2.3 will have empty namespace name in master state, which in turn breaks the client partition routing since client receive empty namespace name. This change fixes the issue by resolving the namespace id to namespace name when namespace name is not set. Thanks @jhe for your help with validating the fix. Jira: DB-6814 Test Plan: Jenkins @jhe helped validate this change by creating a table in older version 2.2 and then upgrading the system. Before fix (notice that only the newly created table has a keyspace_name): ``` ycqlsh> select * from system.partitions; keyspace_name | table_name | start_key | end_key | id | replica_addresses ---------------+---------------------------------+-----------+---------+--------------------------------------+------------------------- | resource_role_permissions_index | 0x | 0x | 536e2bf0-825f-7283-9746-471b31c671c7 | {'127.0.0.1': 'LEADER'} | columns | 0x | 0x | 4418bfc9-dc72-72b4-894c-0112777c724c | {'127.0.0.1': 'LEADER'} | aggregates | 0x | 0x | 7c150dd2-a045-c495-8c4c-d2c966b6b11a | {'127.0.0.1': 'LEADER'} | size_estimates | 0x | 0x | 18e36d79-fb60-9c93-534d-8a21a1f10e98 | {'127.0.0.1': 'LEADER'} | keyspaces | 0x | 0x | f99f0aa3-1cc8-ad85-c544-b88dbe4c7d36 | {'127.0.0.1': 'LEADER'} | types | 0x | 0x | 1f94be70-fb7d-e6a4-6a47-187b194841b9 | {'127.0.0.1': 'LEADER'} jhe | test_after | 0x | 0x8000 | 27fabca6-ae97-55bc-5c41-3899f4891790 | {'127.0.0.1': 'LEADER'} jhe | test_after | 0x8000 | 0x | 8644a3cd-4c6f-bba7-fd47-f7761c24c5bb | {'127.0.0.1': 'LEADER'} | partitions | 0x | 0x | c3abda51-6757-ffb1-d14e-466c4d912cd9 | {'127.0.0.1': 'LEADER'} | triggers | 0x | 0x | e656d4d5-ce96-3b89-d94c-f4e913cc1fac | {'127.0.0.1': 'LEADER'} | role_permissions | 0x | 0x | 8bd3a5af-71ea-f6a2-0f42-a4027e760c30 | {'127.0.0.1': 'LEADER'} | views | 0x | 0x | 5d3c58c3-2ecc-4eb7-374f-88d4fa49efe0 | {'127.0.0.1': 'LEADER'} | peers | 0x | 0x | 178e5231-95bb-739d-aa46-f5497f2adb3b | {'127.0.0.1': 'LEADER'} | local | 0x | 0x | 3fc04471-ae7a-2a9c-8a4f-e173b41aabeb | {'127.0.0.1': 'LEADER'} | functions | 0x | 0x | c8b1b8ec-69f7-13a9-d741-a8617a7aed38 | {'127.0.0.1': 'LEADER'} | roles | 0x | 0x | d65d943f-65d4-93be-3d41-2669a0d323d2 | {'127.0.0.1': 'LEADER'} | tables | 0x | 0x | a2447e71-a6b9-bea4-f147-036c487133ac | {'127.0.0.1': 'LEADER'} | indexes | 0x | 0x | 91891ec0-8bb3-d281-2e4c-6f7062879838 | {'127.0.0.1': 'LEADER'} | test | 0x | 0x7fff | 491d7947-4fd1-6685-da40-7da00ad14d93 | {'127.0.0.1': 'LEADER'} | test | 0x7fff | 0x | 16d0567a-90cb-99a0-f645-89db0e90f473 | {'127.0.0.1': 'LEADER'} ``` With the fix (all tables have a keyspace_name): ``` ycqlsh> select * from system.partitions; keyspace_name | table_name | start_key | end_key | id | replica_addresses ---------------+---------------------------------+-----------+---------+--------------------------------------+--------------------------- system_auth | resource_role_permissions_index | 0x | 0x | 536e2bf0-825f-7283-9746-471b31c671c7 | {'127.0.0.1': 'LEADER'} system_schema | columns | 0x | 0x | 4418bfc9-dc72-72b4-894c-0112777c724c | {'127.0.0.1': 'LEADER'} system_schema | aggregates | 0x | 0x | 7c150dd2-a045-c495-8c4c-d2c966b6b11a | {'127.0.0.1': 'LEADER'} system | size_estimates | 0x | 0x | 18e36d79-fb60-9c93-534d-8a21a1f10e98 | {'127.0.0.1': 'LEADER'} system_schema | keyspaces | 0x | 0x | f99f0aa3-1cc8-ad85-c544-b88dbe4c7d36 | {'127.0.0.1': 'LEADER'} system_schema | types | 0x | 0x | 1f94be70-fb7d-e6a4-6a47-187b194841b9 | {'127.0.0.1': 'LEADER'} jhe | test_after | 0x | 0x8000 | 27fabca6-ae97-55bc-5c41-3899f4891790 | {'127.0.0.1': 'FOLLOWER'} jhe | test_after | 0x8000 | 0x | 8644a3cd-4c6f-bba7-fd47-f7761c24c5bb | {'127.0.0.1': 'FOLLOWER'} system | partitions | 0x | 0x | c3abda51-6757-ffb1-d14e-466c4d912cd9 | {'127.0.0.1': 'LEADER'} system_schema | triggers | 0x | 0x | e656d4d5-ce96-3b89-d94c-f4e913cc1fac | {'127.0.0.1': 'LEADER'} system_auth | role_permissions | 0x | 0x | 8bd3a5af-71ea-f6a2-0f42-a4027e760c30 | {'127.0.0.1': 'LEADER'} system_schema | views | 0x | 0x | 5d3c58c3-2ecc-4eb7-374f-88d4fa49efe0 | {'127.0.0.1': 'LEADER'} system | peers | 0x | 0x | 178e5231-95bb-739d-aa46-f5497f2adb3b | {'127.0.0.1': 'LEADER'} system | local | 0x | 0x | 3fc04471-ae7a-2a9c-8a4f-e173b41aabeb | {'127.0.0.1': 'LEADER'} system_schema | functions | 0x | 0x | c8b1b8ec-69f7-13a9-d741-a8617a7aed38 | {'127.0.0.1': 'LEADER'} system_auth | roles | 0x | 0x | d65d943f-65d4-93be-3d41-2669a0d323d2 | {'127.0.0.1': 'LEADER'} system_schema | tables | 0x | 0x | a2447e71-a6b9-bea4-f147-036c487133ac | {'127.0.0.1': 'LEADER'} system_schema | indexes | 0x | 0x | 91891ec0-8bb3-d281-2e4c-6f7062879838 | {'127.0.0.1': 'LEADER'} jhe | test | 0x | 0x7fff | 491d7947-4fd1-6685-da40-7da00ad14d93 | {'127.0.0.1': 'LEADER'} jhe | test | 0x7fff | 0x | 16d0567a-90cb-99a0-f645-89db0e90f473 | {'127.0.0.1': 'FOLLOWER'} ``` Reviewers: jhe, bogdan, slingam Reviewed By: bogdan Subscribers: bogdan, jhe, ybase, yql Differential Revision: https://phorge.dev.yugabyte.com/D26161
yugabyte · Jun 14, 2023 · e632119 · e632119
1 parent 1b4120a
commit e632119
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 6 deletions.
diff --git a/src/yb/master/catalog_entity_info.cc b/src/yb/master/catalog_entity_info.cc
@@ -347,6 +347,8 @@ const NamespaceId TableInfo::namespace_id() const {
   return LockForRead()->namespace_id();
 }
 
+// namespace_name can be null if table was created on version < 2.3.0 (see GH17713/GH17712 for more
+// details)
 const NamespaceName TableInfo::namespace_name() const {
   return LockForRead()->namespace_name();
 }

diff --git a/src/yb/master/catalog_manager.cc b/src/yb/master/catalog_manager.cc
@@ -6710,6 +6710,8 @@ scoped_refptr<TableInfo> CatalogManager::GetTableInfoUnlocked(const TableId& tab
 
 std::vector<TableInfoPtr> CatalogManager::GetTables(GetTablesMode mode) {
   std::vector<TableInfoPtr> result;
+  // Note: TableInfoPtr has a namespace_name field which was introduced in version 2.3.0. The data
+  // for this field is not backfilled (see GH17713/GH17712 for more details).
   {
     SharedLock lock(mutex_);
     result.reserve(table_ids_map_->size());

diff --git a/src/yb/master/yql_partitions_vtable.cc b/src/yb/master/yql_partitions_vtable.cc
@@ -204,12 +204,22 @@ Result<YQLPartitionsVTable::TabletData> YQLPartitionsVTable::GetTabletData(
     const scoped_refptr<TabletInfo>& tablet,
     DnsLookupMap* dns_lookups,
     google::protobuf::Arena* arena) const {
-  auto data = TabletData {
-    .namespace_name = tablet->table()->namespace_name(),
-    .table_name = tablet->table()->name(),
-    .table_id = tablet->table()->id(),
-    .tablet_id = tablet->tablet_id(),
-    .locations = google::protobuf::Arena::Create<TabletLocationsPB>(arena),
+  // Resolve namespace name - namespace name field was introduced in 2.3.0, therefore tables created
+  // with older version will not have namespace_name set (GH17713 tracks the fix for
+  // migration/backfill in memory state). This workaround ensures that we send correct information
+  // to client as part of system.partition request.
+  auto namespace_name = tablet->table()->namespace_name();
+  if (namespace_name.empty()) {
+    namespace_name = VERIFY_RESULT(master_->catalog_manager()->FindNamespaceById(
+                                       tablet->table()->namespace_id()))->name();
+  }
+
+  auto data = TabletData{
+      .namespace_name = namespace_name,
+      .table_name = tablet->table()->name(),
+      .table_id = tablet->table()->id(),
+      .tablet_id = tablet->tablet_id(),
+      .locations = google::protobuf::Arena::Create<TabletLocationsPB>(arena),
   };
 
   auto s = master_->catalog_manager()->GetTabletLocations(tablet, data.locations);