fix: wrong max_table_id log in remote catalog manager (#1516)

* fix: wrong max_table_id log in remote catalog manager * chore: update link in CONTRIBUTING.md * chore: add a new const MAX_SYS_TABLE_ID
GreptimeTeam · May 5, 2023 · 224ec9b · 224ec9b
1 parent d86b338
commit 224ec9b
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 15 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -107,6 +107,6 @@ The core team will be thrilled if you participate in any way you like. When you
 
 Also, see some extra GreptimeDB content:
 
-- [GreptimeDB Docs](https://greptime.com/docs)
-- [Learn GreptimeDB](https://greptime.com/products/db)
+- [GreptimeDB Docs](https://docs.greptime.com/)
+- [Learn GreptimeDB](https://greptime.com/product/db)
 - [Greptime Inc. Website](https://greptime.com)
diff --git a/src/catalog/src/remote/manager.rs b/src/catalog/src/remote/manager.rs
@@ -19,7 +19,7 @@ use std::sync::Arc;
 
 use async_stream::stream;
 use async_trait::async_trait;
-use common_catalog::consts::{MIN_USER_TABLE_ID, MITO_ENGINE};
+use common_catalog::consts::{MAX_SYS_TABLE_ID, MITO_ENGINE};
 use common_telemetry::{debug, error, info, warn};
 use dashmap::DashMap;
 use futures::Stream;
@@ -173,33 +173,38 @@ impl RemoteCatalogManager {
     }
 
     /// Fetch catalogs/schemas/tables from remote catalog manager along with max table id allocated.
-    async fn initiate_catalogs(&self) -> Result<(HashMap<String, CatalogProviderRef>, TableId)> {
+    async fn initiate_catalogs(&self) -> Result<HashMap<String, CatalogProviderRef>> {
         let mut res = HashMap::new();
-        let max_table_id = MIN_USER_TABLE_ID - 1;
 
         let mut catalogs = self.iter_remote_catalogs().await;
         while let Some(r) = catalogs.next().await {
+            let mut max_table_id = MAX_SYS_TABLE_ID;
             let CatalogKey { catalog_name, .. } = r?;
             info!("Fetch catalog from metasrv: {}", catalog_name);
             let catalog = res
                 .entry(catalog_name.clone())
                 .or_insert_with(|| self.new_catalog_provider(&catalog_name))
                 .clone();
 
-            self.initiate_schemas(catalog_name, catalog, max_table_id)
+            self.initiate_schemas(&catalog_name, catalog, &mut max_table_id)
                 .await?;
+
+            info!(
+                "Catalog name: {}, max table id allocated: {}",
+                &catalog_name, max_table_id
+            );
         }
 
-        Ok((res, max_table_id))
+        Ok(res)
     }
 
     async fn initiate_schemas(
         &self,
-        catalog_name: String,
+        catalog_name: &str,
         catalog: CatalogProviderRef,
-        max_table_id: TableId,
+        max_table_id: &mut TableId,
     ) -> Result<()> {
-        let mut schemas = self.iter_remote_schemas(&catalog_name).await;
+        let mut schemas = self.iter_remote_schemas(catalog_name).await;
         while let Some(r) = schemas.next().await {
             let SchemaKey {
                 catalog_name,
@@ -235,7 +240,7 @@ impl RemoteCatalogManager {
         catalog_name: &'a str,
         schema_name: &'a str,
         schema: SchemaProviderRef,
-        mut max_table_id: TableId,
+        max_table_id: &mut TableId,
     ) -> Result<()> {
         info!("initializing tables in {}.{}", catalog_name, schema_name);
         let mut table_num = 0;
@@ -260,7 +265,7 @@ impl RemoteCatalogManager {
             let table_id = table_info.ident.table_id;
             schema.register_table(table_name.clone(), table_ref).await?;
             info!("Registered table {}", table_name);
-            max_table_id = max_table_id.max(table_id);
+            *max_table_id = (*max_table_id).max(table_id);
             table_num += 1;
         }
 
@@ -402,7 +407,7 @@ async fn open_or_create_table(
 #[async_trait::async_trait]
 impl CatalogManager for RemoteCatalogManager {
     async fn start(&self) -> Result<()> {
-        let (catalogs, max_table_id) = self.initiate_catalogs().await?;
+        let catalogs = self.initiate_catalogs().await?;
         info!(
             "Initialized catalogs: {:?}",
             catalogs.keys().cloned().collect::<Vec<_>>()
@@ -415,8 +420,6 @@ impl CatalogManager for RemoteCatalogManager {
             });
         }
 
-        info!("Max table id allocated: {}", max_table_id);
-
         let mut system_table_requests = self.system_table_requests.lock().await;
         let engine = self
             .engine_manager

diff --git a/src/common/catalog/src/consts.rs b/src/common/catalog/src/consts.rs
@@ -21,6 +21,8 @@ pub const DEFAULT_SCHEMA_NAME: &str = "public";
 /// Reserves [0,MIN_USER_TABLE_ID) for internal usage.
 /// User defined table id starts from this value.
 pub const MIN_USER_TABLE_ID: u32 = 1024;
+/// the max system table id
+pub const MAX_SYS_TABLE_ID: u32 = MIN_USER_TABLE_ID - 1;
 /// system_catalog table id
 pub const SYSTEM_CATALOG_TABLE_ID: u32 = 0;
 /// scripts table id