fix: physical table statistics info (#4975)

* fix: physical table statistics info * refactor: is_physical_table * fix: remove file
GreptimeTeam · Nov 13, 2024 · d7c3c8e · d7c3c8e
1 parent f4b9eac
commit d7c3c8e
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 28 deletions.
diff --git a/src/catalog/src/system_schema/information_schema/tables.rs b/src/catalog/src/system_schema/information_schema/tables.rs
@@ -259,21 +259,22 @@ impl InformationSchemaTablesBuilder {
                 let table_info = table.table_info();
 
                 // TODO(dennis): make it working for metric engine
-                let table_region_stats = if table_info.meta.engine == MITO_ENGINE {
-                    let region_ids = table_info
-                        .meta
-                        .region_numbers
-                        .iter()
-                        .map(|n| RegionId::new(table_info.ident.table_id, *n))
-                        .collect::<HashSet<_>>();
-
-                    region_stats
-                        .iter()
-                        .filter(|stat| region_ids.contains(&stat.id))
-                        .collect::<Vec<_>>()
-                } else {
-                    vec![]
-                };
+                let table_region_stats =
+                    if table_info.meta.engine == MITO_ENGINE || table_info.is_physical_table() {
+                        let region_ids = table_info
+                            .meta
+                            .region_numbers
+                            .iter()
+                            .map(|n| RegionId::new(table_info.ident.table_id, *n))
+                            .collect::<HashSet<_>>();
+
+                        region_stats
+                            .iter()
+                            .filter(|stat| region_ids.contains(&stat.id))
+                            .collect::<Vec<_>>()
+                    } else {
+                        vec![]
+                    };
 
                 self.add_table(
                     &predicates,

diff --git a/src/common/meta/src/datanode.rs b/src/common/meta/src/datanode.rs
@@ -77,7 +77,7 @@ pub struct RegionStat {
     pub rcus: i64,
     /// The write capacity units during this period
     pub wcus: i64,
-    /// Approximate bytes of this region
+    /// Approximate disk bytes of this region, including sst, index, manifest and wal
     pub approximate_bytes: u64,
     /// The engine name.
     pub engine: String,

diff --git a/src/metric-engine/src/engine/create.rs b/src/metric-engine/src/engine/create.rs
@@ -31,7 +31,6 @@ use store_api::metric_engine_consts::{
     METADATA_SCHEMA_KEY_COLUMN_INDEX, METADATA_SCHEMA_KEY_COLUMN_NAME,
     METADATA_SCHEMA_TIMESTAMP_COLUMN_INDEX, METADATA_SCHEMA_TIMESTAMP_COLUMN_NAME,
     METADATA_SCHEMA_VALUE_COLUMN_INDEX, METADATA_SCHEMA_VALUE_COLUMN_NAME,
-    PHYSICAL_TABLE_METADATA_KEY,
 };
 use store_api::mito_engine_options::{APPEND_MODE_KEY, TTL_KEY};
 use store_api::region_engine::RegionEngine;
@@ -61,7 +60,7 @@ impl MetricEngineInner {
     ) -> Result<AffectedRows> {
         Self::verify_region_create_request(&request)?;
 
-        let result = if request.options.contains_key(PHYSICAL_TABLE_METADATA_KEY) {
+        let result = if request.is_physical_table() {
             self.create_physical_region(region_id, request).await
         } else if request.options.contains_key(LOGICAL_TABLE_METADATA_KEY) {
             let physical_region_id = self.create_logical_region(region_id, request).await?;
@@ -355,12 +354,11 @@ impl MetricEngineInner {
 
         // check if required table option is present
         ensure!(
-            request.options.contains_key(PHYSICAL_TABLE_METADATA_KEY)
-                || request.options.contains_key(LOGICAL_TABLE_METADATA_KEY),
+            request.is_physical_table() || request.options.contains_key(LOGICAL_TABLE_METADATA_KEY),
             MissingRegionOptionSnafu {}
         );
         ensure!(
-            !(request.options.contains_key(PHYSICAL_TABLE_METADATA_KEY)
+            !(request.is_physical_table()
                 && request.options.contains_key(LOGICAL_TABLE_METADATA_KEY)),
             ConflictRegionOptionSnafu {}
         );
@@ -543,7 +541,7 @@ impl MetricEngineInner {
 
 #[cfg(test)]
 mod test {
-    use store_api::metric_engine_consts::METRIC_ENGINE_NAME;
+    use store_api::metric_engine_consts::{METRIC_ENGINE_NAME, PHYSICAL_TABLE_METADATA_KEY};
 
     use super::*;
     use crate::engine::MetricEngine;

diff --git a/src/metric-engine/src/engine/open.rs b/src/metric-engine/src/engine/open.rs
@@ -18,9 +18,7 @@ use common_telemetry::info;
 use mito2::engine::MITO_ENGINE_NAME;
 use object_store::util::join_dir;
 use snafu::ResultExt;
-use store_api::metric_engine_consts::{
-    DATA_REGION_SUBDIR, METADATA_REGION_SUBDIR, PHYSICAL_TABLE_METADATA_KEY,
-};
+use store_api::metric_engine_consts::{DATA_REGION_SUBDIR, METADATA_REGION_SUBDIR};
 use store_api::region_engine::RegionEngine;
 use store_api::region_request::{AffectedRows, RegionOpenRequest, RegionRequest};
 use store_api::storage::RegionId;
@@ -46,9 +44,7 @@ impl MetricEngineInner {
         region_id: RegionId,
         request: RegionOpenRequest,
     ) -> Result<AffectedRows> {
-        let is_opening_physical_region = request.options.contains_key(PHYSICAL_TABLE_METADATA_KEY);
-
-        if is_opening_physical_region {
+        if request.is_physical_table() {
             // open physical region and recover states
             self.open_physical_region(region_id, request).await?;
             self.recover_states(region_id).await?;

diff --git a/src/store-api/src/region_request.rs b/src/store-api/src/region_request.rs
@@ -38,6 +38,7 @@ use crate::metadata::{
     InvalidRegionOptionChangeRequestSnafu, InvalidRegionRequestSnafu, MetadataError,
     RegionMetadata, Result,
 };
+use crate::metric_engine_consts::PHYSICAL_TABLE_METADATA_KEY;
 use crate::mito_engine_options::{
     TTL_KEY, TWCS_MAX_ACTIVE_WINDOW_FILES, TWCS_MAX_ACTIVE_WINDOW_RUNS,
     TWCS_MAX_INACTIVE_WINDOW_FILES, TWCS_MAX_INACTIVE_WINDOW_RUNS, TWCS_MAX_OUTPUT_FILE_SIZE,
@@ -306,6 +307,11 @@ impl RegionCreateRequest {
 
         Ok(())
     }
+
+    /// Returns true when the region belongs to the metric engine's physical table.
+    pub fn is_physical_table(&self) -> bool {
+        self.options.contains_key(PHYSICAL_TABLE_METADATA_KEY)
+    }
 }
 
 #[derive(Debug, Clone, Default)]
@@ -324,6 +330,13 @@ pub struct RegionOpenRequest {
     pub skip_wal_replay: bool,
 }
 
+impl RegionOpenRequest {
+    /// Returns true when the region belongs to the metric engine's physical table.
+    pub fn is_physical_table(&self) -> bool {
+        self.options.contains_key(PHYSICAL_TABLE_METADATA_KEY)
+    }
+}
+
 /// Close region request.
 #[derive(Debug)]
 pub struct RegionCloseRequest {}

diff --git a/src/table/src/metadata.rs b/src/table/src/metadata.rs
@@ -26,6 +26,7 @@ use datatypes::schema::{
 use derive_builder::Builder;
 use serde::{Deserialize, Serialize};
 use snafu::{ensure, OptionExt, ResultExt};
+use store_api::metric_engine_consts::PHYSICAL_TABLE_METADATA_KEY;
 use store_api::mito_engine_options::{COMPACTION_TYPE, COMPACTION_TYPE_TWCS};
 use store_api::region_request::ChangeOption;
 use store_api::storage::{ColumnDescriptor, ColumnDescriptorBuilder, ColumnId, RegionId};
@@ -797,6 +798,14 @@ impl TableInfo {
     pub fn full_table_name(&self) -> String {
         common_catalog::format_full_table_name(&self.catalog_name, &self.schema_name, &self.name)
     }
+
+    /// Returns true when the table is the metric engine's physical table.
+    pub fn is_physical_table(&self) -> bool {
+        self.meta
+            .options
+            .extra_options
+            .contains_key(PHYSICAL_TABLE_METADATA_KEY)
+    }
 }
 
 impl TableInfoBuilder {