KUDU-745 / KUDU-1463 tablet's table id attribute is empty string

This fixes the issue where the table_id attribute of a tablet in /metrics was an empty string. As part of this patch, I also ended up picking a couple of magic table_id's. The significant one is that the table_id of sys.catalog is "sys.catalog.id". Change-Id: I1795a176312f6c2a55c34b7b684ee408f1cf8732 Reviewed-on: http://gerrit.cloudera.org:8080/3183 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon <[email protected]>
jhx1008 · May 25, 2016 · c11b8cf · c11b8cf
1 parent 57c4b47
commit c11b8cf
Show file tree

Hide file tree

Showing 10 changed files with 20 additions and 5 deletions.
diff --git a/src/kudu/consensus/log-test-base.h b/src/kudu/consensus/log-test-base.h
@@ -69,6 +69,7 @@ using tablet::OperationResultPB;
 using tablet::MemStoreTargetPB;
 
 const char* kTestTable = "test-log-table";
+const char* kTestTableId = "test-log-table-id";
 const char* kTestTablet = "test-log-tablet";
 const bool APPEND_SYNC = true;
 const bool APPEND_ASYNC = false;

diff --git a/src/kudu/master/sys_catalog.cc b/src/kudu/master/sys_catalog.cc
@@ -149,6 +149,7 @@ Status SysCatalogTable::CreateNew(FsManager *fs_manager) {
   RETURN_NOT_OK(tablet::TabletMetadata::CreateNew(fs_manager,
                                                   kSysCatalogTabletId,
                                                   table_name(),
+                                                  table_id(),
                                                   schema, partition_schema,
                                                   partitions[0],
                                                   tablet::TABLET_DATA_READY,

diff --git a/src/kudu/master/sys_catalog.h b/src/kudu/master/sys_catalog.h
@@ -119,6 +119,7 @@ class SysCatalogTable {
   friend class CatalogManager;
 
   const char *table_name() const { return "sys.catalog"; }
+  const char *table_id() const { return "sys.catalog.id"; }
 
   // Return the schema of the table.
   // NOTE: This is the "server-side" schema, so it must have the column IDs.

diff --git a/src/kudu/tablet/tablet-harness.h b/src/kudu/tablet/tablet-harness.h
@@ -89,6 +89,7 @@ class TabletHarness {
     RETURN_NOT_OK(TabletMetadata::LoadOrCreate(fs_manager_.get(),
                                                options_.tablet_id,
                                                "KuduTableTest",
+                                               "KuduTableTestId",
                                                schema_,
                                                partition.first,
                                                partition.second,

diff --git a/src/kudu/tablet/tablet_bootstrap-test.cc b/src/kudu/tablet/tablet_bootstrap-test.cc
@@ -76,6 +76,7 @@ class BootstrapTest : public LogTestBase {
     RETURN_NOT_OK(TabletMetadata::LoadOrCreate(fs_manager_.get(),
                                                log::kTestTablet,
                                                log::kTestTable,
+                                               log::kTestTableId,
                                                schema,
                                                partition.first,
                                                partition.second,

diff --git a/src/kudu/tablet/tablet_metadata.cc b/src/kudu/tablet/tablet_metadata.cc
@@ -69,6 +69,7 @@ const int64 kNoDurableMemStore = -1;
 Status TabletMetadata::CreateNew(FsManager* fs_manager,
                                  const string& tablet_id,
                                  const string& table_name,
+                                 const string& table_id,
                                  const Schema& schema,
                                  const PartitionSchema& partition_schema,
                                  const Partition& partition,
@@ -83,6 +84,7 @@ Status TabletMetadata::CreateNew(FsManager* fs_manager,
   scoped_refptr<TabletMetadata> ret(new TabletMetadata(fs_manager,
                                                        tablet_id,
                                                        table_name,
+                                                       table_id,
                                                        schema,
                                                        partition_schema,
                                                        partition,
@@ -104,6 +106,7 @@ Status TabletMetadata::Load(FsManager* fs_manager,
 Status TabletMetadata::LoadOrCreate(FsManager* fs_manager,
                                     const string& tablet_id,
                                     const string& table_name,
+                                    const string& table_id,
                                     const Schema& schema,
                                     const PartitionSchema& partition_schema,
                                     const Partition& partition,
@@ -118,7 +121,7 @@ Status TabletMetadata::LoadOrCreate(FsManager* fs_manager,
     }
     return Status::OK();
   } else if (s.IsNotFound()) {
-    return CreateNew(fs_manager, tablet_id, table_name, schema,
+    return CreateNew(fs_manager, tablet_id, table_name, table_id, schema,
                      partition_schema, partition, initial_tablet_data_state,
                      metadata);
   } else {
@@ -207,8 +210,8 @@ Status TabletMetadata::DeleteSuperBlock() {
 }
 
 TabletMetadata::TabletMetadata(FsManager* fs_manager, string tablet_id,
-                               string table_name, const Schema& schema,
-                               PartitionSchema partition_schema,
+                               string table_name, string table_id,
+                               const Schema& schema, PartitionSchema partition_schema,
                                Partition partition,
                                const TabletDataState& tablet_data_state)
     : state_(kNotWrittenYet),
@@ -219,6 +222,7 @@ TabletMetadata::TabletMetadata(FsManager* fs_manager, string tablet_id,
       last_durable_mrs_id_(kNoDurableMemStore),
       schema_(new Schema(schema)),
       schema_version_(0),
+      table_id_(std::move(table_id)),
       table_name_(std::move(table_name)),
       partition_schema_(std::move(partition_schema)),
       tablet_data_state_(tablet_data_state),

diff --git a/src/kudu/tablet/tablet_metadata.h b/src/kudu/tablet/tablet_metadata.h
@@ -65,6 +65,7 @@ class TabletMetadata : public RefCountedThreadSafe<TabletMetadata> {
   static Status CreateNew(FsManager* fs_manager,
                           const std::string& tablet_id,
                           const std::string& table_name,
+                          const std::string& table_id,
                           const Schema& schema,
                           const PartitionSchema& partition_schema,
                           const Partition& partition,
@@ -84,6 +85,7 @@ class TabletMetadata : public RefCountedThreadSafe<TabletMetadata> {
   static Status LoadOrCreate(FsManager* fs_manager,
                              const std::string& tablet_id,
                              const std::string& table_name,
+                             const std::string& table_id,
                              const Schema& schema,
                              const PartitionSchema& partition_schema,
                              const Partition& partition,
@@ -233,8 +235,9 @@ class TabletMetadata : public RefCountedThreadSafe<TabletMetadata> {
   // TODO: get rid of this many-arg constructor in favor of just passing in a
   // SuperBlock, which already contains all of these fields.
   TabletMetadata(FsManager* fs_manager, std::string tablet_id,
-                 std::string table_name, const Schema& schema,
-                 PartitionSchema partition_schema, Partition partition,
+                 std::string table_name, std::string table_id,
+                 const Schema& schema, PartitionSchema partition_schema,
+                 Partition partition,
                  const TabletDataState& tablet_data_state);
 
   // Constructor for loading an existing tablet.

diff --git a/src/kudu/tserver/remote_bootstrap_client.cc b/src/kudu/tserver/remote_bootstrap_client.cc
@@ -233,6 +233,7 @@ Status RemoteBootstrapClient::Start(const string& bootstrap_peer_uuid,
     // Create the superblock on disk.
     RETURN_NOT_OK(TabletMetadata::CreateNew(fs_manager_, tablet_id_,
                                             superblock_->table_name(),
+                                            superblock_->table_id(),
                                             schema,
                                             partition_schema,
                                             partition,

diff --git a/src/kudu/tserver/tablet_server-test.cc b/src/kudu/tserver/tablet_server-test.cc
@@ -192,6 +192,7 @@ TEST_F(TabletServerTest, TestWebPages) {
 
     // Check entity attributes.
     ASSERT_STR_CONTAINS(buf.ToString(), "\"table_name\": \"TestTable\"");
+    ASSERT_STR_CONTAINS(buf.ToString(), "\"table_id\": \"TestTable\"");
 
     // Check for the existence of some particular metrics for which we've had early-retirement
     // bugs in the past.

diff --git a/src/kudu/tserver/ts_tablet_manager.cc b/src/kudu/tserver/ts_tablet_manager.cc
@@ -285,6 +285,7 @@ Status TSTabletManager::CreateNewTablet(const string& table_id,
     TabletMetadata::CreateNew(fs_manager_,
                               tablet_id,
                               table_name,
+                              table_id,
                               schema,
                               partition_schema,
                               partition,