[docdb][colocation] Dropped colocated table still referenced in TabletInfo

[jaki]

Jira Link: DB-1190

Description

When doing DROP TABLE on a colocated user table, the TabletInfo PB's table_ids repeated field still contains the table id of the dropped user table. It doesn't look like there's any attempt to remove it, unlike on DROP DATABASE where DeleteYsqlDBTables calls RemoveTableIdsFromTabletInfo.

@frozenspider encountered the failure related to this. I produced a repro:

First, apply (on commit 040673f):

diff --git i/src/yb/master/catalog_manager.cc w/src/yb/master/catalog_manager.cc
index ac3d572b44..ee9f632aa8 100644
--- i/src/yb/master/catalog_manager.cc
+++ w/src/yb/master/catalog_manager.cc
@@ -5463,6 +5463,12 @@ Status CatalogManager::GetColocatedTabletSchema(const GetColocatedTabletSchemaRe
                       STATUS(InvalidArgument, "Table provided is not a parent colocated table"));
   }
 
+  auto colocated_tablet = parent_colocated_table->GetColocatedTablet();
+  {
+    auto l = colocated_tablet->LockForRead();
+    LOG(WARNING) << l.data().pb.DebugString();
+  }
+
   // Next get all the user tables that are in the database.
   ListTablesRequestPB listTablesReq;
   ListTablesResponsePB ListTablesResp;
diff --git i/src/yb/tools/yb-admin_cli.cc w/src/yb/tools/yb-admin_cli.cc
index dde1dd3fb7..4494408536 100644
--- i/src/yb/tools/yb-admin_cli.cc
+++ w/src/yb/tools/yb-admin_cli.cc
@@ -866,6 +866,17 @@ void ClusterAdminCli::RegisterCommandHandlers(ClusterAdminClientClass* client) {
         return Status::OK();
       });
 
+  Register(
+      "get_colocated_tablet_schema", " <parent_table_id>",
+      [client](const CLIArguments& args) -> Status {
+        if (args.size() != 1) {
+          return ClusterAdminCli::kInvalidArguments;
+        }
+        RETURN_NOT_OK_PREPEND(client->GetColocatedTabletSchema(args[0]),
+                              Substitute("Unable to get colocated table schema for $0", args[0]));
+        return Status::OK();
+      });
+
   RegisterJson("ddl_log", "", std::bind(&DdlLog, client, _1));
 
   Register(
diff --git i/src/yb/tools/yb-admin_client.cc w/src/yb/tools/yb-admin_client.cc
index 317c4641b3..7a78c557ca 100644
--- i/src/yb/tools/yb-admin_client.cc
+++ w/src/yb/tools/yb-admin_client.cc
@@ -46,6 +46,7 @@
 #include "yb/client/client.h"
 #include "yb/client/table.h"
 #include "yb/client/table_creator.h"
+#include "yb/client/table_info.h"
 
 #include "yb/common/json_util.h"
 #include "yb/common/redis_constants_common.h"
@@ -1849,6 +1850,18 @@ Status ClusterAdminClient::GetYsqlCatalogVersion() {
   return Status::OK();
 }
 
+Status ClusterAdminClient::GetColocatedTabletSchema(const TableId& table_id) {
+  auto infos = std::make_shared<std::vector<client::YBTableInfo>>();
+  Synchronizer sync;
+  RETURN_NOT_OK(yb_client_->GetColocatedTabletSchemaById(table_id, infos, sync.AsStatusCallback()));
+  RETURN_NOT_OK(sync.Wait());
+  for (const auto& info : *infos) {
+    cout << info.table_name.ToString() << " (id=" << info.table_id << "): "
+         << info.schema.ToString() << "\n";
+  }
+  return Status::OK();
+}
+
 Result<rapidjson::Document> ClusterAdminClient::DdlLog() {
   RpcController rpc;
   rpc.set_timeout(timeout_);
diff --git i/src/yb/tools/yb-admin_client.h w/src/yb/tools/yb-admin_client.h
index a858e8c934..f4ea4fb148 100644
--- i/src/yb/tools/yb-admin_client.h
+++ w/src/yb/tools/yb-admin_client.h
@@ -280,6 +280,8 @@ class ClusterAdminClient {
 
   CHECKED_STATUS GetYsqlCatalogVersion();
 
+  CHECKED_STATUS GetColocatedTabletSchema(const TableId& table_id);
+
   Result<rapidjson::Document> DdlLog();
 
   // Upgrade YSQL cluster (all databases) to the latest version, applying necessary migrations.

Then, create a fresh cluster, create and connect to a colocated database, create two tables, drop the first table, and run yb-admin -master_addresses=... get_colocated_tablet_schema $(yb-admin -master_addresses=... list_tables include_table_id | grep colocated | awk '{print$2}').

yb-admin should return the second user table:

co.second (id=00004000000030008000000000004004): Schema [
        10:ybrowid[binary NOT NULL NOT A PARTITION KEY],
        11:i[int32 NULLABLE NOT A PARTITION KEY]
]
properties: contain_counters: false is_transactional: true consistency_level: STRONG use_mangled_column_name: false is_ysql_catalog_table: false retain_delete_markers: false

Master logs should show two user tables and the parent table, like

table_ids: "00004000000030008000000000000000.colocated.parent.uuid"
table_ids: "00004000000030008000000000004001"
table_ids: "00004000000030008000000000004004"

00004000000030008000000000004001 was the first table and should have been removed from the colocated tablet's TabletInfo on master. Since this is persisted, a restart of master will still have it. The tablet on tserver seems to be fine at doing the ChangeMetadata MetadataChange::REMOVE_TABLE and updating its kvstore.

Creating a new table with the same id (using WITH (table_oid = ...) doesn't seem to have issues with some basic testing. However, if any code from master reads the table_ids of the colocated tablet TabletInfo, it should not assume that those tables are colocated!

keywords: colocated tablet, colocated user table, table_ids repeated protobuf field, metadata, PersistentTabletInfo, tablegroup

[yifanguan]

Nice work from Zach. Commit 98645ac solves this issue because it changes the way we persist the info of the set of tables hosted on a colocated tablet on master.
Close this issue as resolved.