From a77267f2c0b62a2e333ef196a1349e2b921aa0c4 Mon Sep 17 00:00:00 2001 From: discord9 Date: Tue, 3 Dec 2024 13:29:10 +0800 Subject: [PATCH 01/21] feat: ttl zero filter --- src/operator/src/insert.rs | 31 +++++++++++++++++++++++++++++-- src/session/src/context.rs | 20 +++++++++++++++++++- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/operator/src/insert.rs b/src/operator/src/insert.rs index 4637f7fd10bb..3fda3c32f624 100644 --- a/src/operator/src/insert.rs +++ b/src/operator/src/insert.rs @@ -44,6 +44,7 @@ use store_api::metric_engine_consts::{ }; use store_api::mito_engine_options::{APPEND_MODE_KEY, MERGE_MODE_KEY}; use store_api::storage::{RegionId, TableId}; +use table::metadata::TableInfo; use table::requests::{InsertRequest as TableInsertRequest, AUTO_CREATE_TABLE_KEY, TTL_KEY}; use table::table_reference::TableReference; use table::TableRef; @@ -320,7 +321,7 @@ impl Inserter { } let write_tasks = self - .group_requests_by_peer(requests) + .group_requests_by_peer(requests, ctx) .await? .into_iter() .map(|(peer, inserts)| { @@ -418,12 +419,17 @@ impl Inserter { async fn group_requests_by_peer( &self, requests: RegionInsertRequests, + ctx: &QueryContextRef, ) -> Result> { // group by region ids first to reduce repeatedly call `find_region_leader` // TODO(discord9): determine if a addition clone is worth it let mut requests_per_region: HashMap = HashMap::new(); - + let ttl_zero_regions = ctx.ttl_zero_regions(); for req in requests.requests { + // filter out ttl zero regions + if ttl_zero_regions.contains(&req.region_id) { + continue; + } let region_id = RegionId::from_u64(req.region_id); requests_per_region .entry(region_id) @@ -463,6 +469,16 @@ impl Inserter { auto_create_table_type: AutoCreateTableType, statement_executor: &StatementExecutor, ) -> Result> { + fn add_ttl_zero_table(ctx: &QueryContextRef, table_info: &TableInfo) { + let ttl = table_info.meta.options.ttl; + if let Some(ttl) = ttl { + if ttl.is_zero() { + ctx.add_ttl_zero_regions( + table_info.region_ids().into_iter().map(|i| i.as_u64()), + ); + } + } + } let _timer = crate::metrics::CREATE_ALTER_ON_DEMAND .with_label_values(&[auto_create_table_type.as_str()]) .start_timer(); @@ -494,6 +510,7 @@ impl Inserter { ), })?; let table_info = table.table_info(); + add_ttl_zero_table(ctx, &table_info); table_name_to_ids.insert(table_info.name.clone(), table_info.table_id()); } return Ok(table_name_to_ids); @@ -558,6 +575,16 @@ impl Inserter { } } + for table_name in table_name_to_ids.keys() { + let table = if let Some(table) = self.get_table(catalog, &schema, table_name).await? { + table + } else { + continue; + }; + let table_info = table.table_info(); + add_ttl_zero_table(ctx, &table_info); + } + Ok(table_name_to_ids) } diff --git a/src/session/src/context.rs b/src/session/src/context.rs index 4e681253c100..08beb617370d 100644 --- a/src/session/src/context.rs +++ b/src/session/src/context.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::HashMap; +use std::collections::{BTreeSet, HashMap}; use std::fmt::{Display, Formatter}; use std::net::SocketAddr; use std::sync::{Arc, RwLock}; @@ -59,6 +59,8 @@ pub struct QueryContext { #[derive(Debug, Builder, Clone, Default)] pub struct QueryContextMutableFields { warning: Option, + /// regions with ttl=0 + ttl_zero_regions: BTreeSet, } impl Display for QueryContext { @@ -284,6 +286,22 @@ impl QueryContext { self.mutable_query_context_data.write().unwrap().warning = Some(msg); } + pub fn add_ttl_zero_regions(&self, region_ids: impl Iterator) { + self.mutable_query_context_data + .write() + .unwrap() + .ttl_zero_regions + .extend(region_ids); + } + + pub fn ttl_zero_regions(&self) -> BTreeSet { + self.mutable_query_context_data + .read() + .unwrap() + .ttl_zero_regions + .clone() + } + pub fn query_timeout(&self) -> Option { self.mutable_session_data.read().unwrap().query_timeout } From 772462c595ca35fc23e7146752035f4863c1f678 Mon Sep 17 00:00:00 2001 From: discord9 Date: Tue, 3 Dec 2024 17:56:45 +0800 Subject: [PATCH 02/21] refactor: use TimeToLive enum --- Cargo.lock | 3 + src/common/base/Cargo.toml | 2 + src/common/base/src/lib.rs | 2 + src/common/base/src/ttl.rs | 108 ++++++++++++++++++++++ src/common/meta/src/ddl/alter_database.rs | 17 ++-- src/common/meta/src/key/schema_name.rs | 74 +++++++++++---- src/common/meta/src/rpc/ddl.rs | 13 +-- src/metric-engine/src/engine/alter.rs | 2 +- src/mito2/src/compaction.rs | 26 +++--- src/mito2/src/compaction/compactor.rs | 5 +- src/mito2/src/compaction/window.rs | 2 +- src/mito2/src/engine/alter_test.rs | 12 +-- src/mito2/src/engine/create_test.rs | 4 +- src/mito2/src/engine/open_test.rs | 4 +- src/mito2/src/region/options.rs | 15 ++- src/mito2/src/worker/handle_alter.rs | 6 +- src/operator/src/insert.rs | 9 +- src/query/src/sql/show_create_table.rs | 12 +-- src/session/Cargo.toml | 1 + src/session/src/context.rs | 7 +- src/store-api/src/region_request.rs | 15 ++- src/table/src/metadata.rs | 6 +- src/table/src/requests.rs | 45 +++++---- 23 files changed, 254 insertions(+), 136 deletions(-) create mode 100644 src/common/base/src/ttl.rs diff --git a/Cargo.lock b/Cargo.lock index 9a7a6b3eb834..6c64f5084a3d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1899,6 +1899,8 @@ dependencies = [ "common-macro", "common-test-util", "futures", + "humantime", + "humantime-serde", "paste", "pin-project", "serde", @@ -10908,6 +10910,7 @@ dependencies = [ name = "session" version = "0.11.0" dependencies = [ + "ahash 0.8.11", "api", "arc-swap", "auth", diff --git a/src/common/base/Cargo.toml b/src/common/base/Cargo.toml index 465599974dae..8563697d95b1 100644 --- a/src/common/base/Cargo.toml +++ b/src/common/base/Cargo.toml @@ -15,6 +15,8 @@ bytes.workspace = true common-error.workspace = true common-macro.workspace = true futures.workspace = true +humantime.workspace = true +humantime-serde.workspace = true paste = "1.0" pin-project.workspace = true serde = { version = "1.0", features = ["derive"] } diff --git a/src/common/base/src/lib.rs b/src/common/base/src/lib.rs index 62a801d9462d..e81f819fd233 100644 --- a/src/common/base/src/lib.rs +++ b/src/common/base/src/lib.rs @@ -19,8 +19,10 @@ pub mod range_read; #[allow(clippy::all)] pub mod readable_size; pub mod secrets; +pub mod ttl; pub type AffectedRows = usize; pub use bit_vec::BitVec; pub use plugins::Plugins; +pub use ttl::TimeToLive; diff --git a/src/common/base/src/ttl.rs b/src/common/base/src/ttl.rs new file mode 100644 index 000000000000..0abb7139986d --- /dev/null +++ b/src/common/base/src/ttl.rs @@ -0,0 +1,108 @@ +// Copyright 2023 Greptime Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::fmt::Display; +use std::time::Duration; + +use serde::{Deserialize, Serialize}; + +/// Time To Live + +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default, Serialize, Deserialize)] +pub enum TimeToLive { + /// immediately throw away on insert + Immediate, + /// Duration to keep the data, this duration should be non-zero + #[serde(with = "humantime_serde")] + Duration(Duration), + /// Keep the data forever + #[default] + Forever, +} + +impl Display for TimeToLive { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + TimeToLive::Immediate => write!(f, "immediate"), + TimeToLive::Duration(d) => write!(f, "Duration({})", d.as_secs()), + TimeToLive::Forever => write!(f, "forever"), + } + } +} + +impl TimeToLive { + /// Parse a string into TimeToLive + pub fn from_humantime_or_str(s: &str) -> Result { + match s { + "immediate" => Ok(TimeToLive::Immediate), + "forever" | "" => Ok(TimeToLive::Forever), + _ => { + let d = humantime::parse_duration(s).map_err(|e| e.to_string())?; + Ok(TimeToLive::Duration(d)) + } + } + } + + /// Print TimeToLive as string + /// + /// omit forever variant + pub fn as_repr_opt(&self) -> Option { + match self { + TimeToLive::Immediate => Some("immediate".to_string()), + TimeToLive::Duration(d) => Some(humantime::format_duration(*d).to_string()), + TimeToLive::Forever => None, + } + } + + pub fn is_immediate(&self) -> bool { + matches!(self, TimeToLive::Immediate) + } + + pub fn is_forever(&self) -> bool { + matches!(self, TimeToLive::Forever) + } + + pub fn get_duration(&self) -> Option { + match self { + TimeToLive::Duration(d) => Some(*d), + _ => None, + } + } +} + +impl From for TimeToLive { + fn from(duration: Duration) -> Self { + if duration.is_zero() { + // compatibility with old code, and inline with cassandra's behavior when ttl set to 0 + TimeToLive::Forever + } else { + TimeToLive::Duration(duration) + } + } +} + +impl From> for TimeToLive { + fn from(duration: Option) -> Self { + match duration { + Some(d) => TimeToLive::from(d), + None => TimeToLive::Forever, + } + } +} + +impl From for TimeToLive { + fn from(duration: humantime::Duration) -> Self { + Self::from(*duration) + } +} diff --git a/src/common/meta/src/ddl/alter_database.rs b/src/common/meta/src/ddl/alter_database.rs index 5e992a7d4e81..a005fa32ab65 100644 --- a/src/common/meta/src/ddl/alter_database.rs +++ b/src/common/meta/src/ddl/alter_database.rs @@ -13,6 +13,7 @@ // limitations under the License. use async_trait::async_trait; +use common_base::TimeToLive; use common_procedure::error::{FromJsonSnafu, Result as ProcedureResult, ToJsonSnafu}; use common_procedure::{Context as ProcedureContext, LockKey, Procedure, Status}; use common_telemetry::tracing::info; @@ -46,11 +47,7 @@ fn build_new_schema_value( for option in options.0.iter() { match option { SetDatabaseOption::Ttl(ttl) => { - if ttl.is_zero() { - value.ttl = None; - } else { - value.ttl = Some(*ttl); - } + value.ttl = *ttl; } } } @@ -58,7 +55,7 @@ fn build_new_schema_value( AlterDatabaseKind::UnsetDatabaseOptions(keys) => { for key in keys.0.iter() { match key { - UnsetDatabaseOption::Ttl => value.ttl = None, + UnsetDatabaseOption::Ttl => value.ttl = TimeToLive::Forever, } } } @@ -220,6 +217,8 @@ impl AlterDatabaseData { mod tests { use std::time::Duration; + use common_base::TimeToLive; + use crate::ddl::alter_database::build_new_schema_value; use crate::key::schema_name::SchemaNameValue; use crate::rpc::ddl::{ @@ -230,12 +229,12 @@ mod tests { #[test] fn test_build_new_schema_value() { let set_ttl = AlterDatabaseKind::SetDatabaseOptions(SetDatabaseOptions(vec![ - SetDatabaseOption::Ttl(Duration::from_secs(10)), + SetDatabaseOption::Ttl(Duration::from_secs(10).into()), ])); let current_schema_value = SchemaNameValue::default(); let new_schema_value = build_new_schema_value(current_schema_value.clone(), &set_ttl).unwrap(); - assert_eq!(new_schema_value.ttl, Some(Duration::from_secs(10))); + assert_eq!(new_schema_value.ttl, Duration::from_secs(10).into()); let unset_ttl_alter_kind = AlterDatabaseKind::UnsetDatabaseOptions(UnsetDatabaseOptions(vec![ @@ -243,6 +242,6 @@ mod tests { ])); let new_schema_value = build_new_schema_value(current_schema_value, &unset_ttl_alter_kind).unwrap(); - assert_eq!(new_schema_value.ttl, None); + assert_eq!(new_schema_value.ttl, TimeToLive::Forever); } } diff --git a/src/common/meta/src/key/schema_name.rs b/src/common/meta/src/key/schema_name.rs index 1ec8c17eb5a1..bdc287e638a8 100644 --- a/src/common/meta/src/key/schema_name.rs +++ b/src/common/meta/src/key/schema_name.rs @@ -15,8 +15,8 @@ use std::collections::HashMap; use std::fmt::Display; use std::sync::Arc; -use std::time::Duration; +use common_base::TimeToLive; use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; use futures::stream::BoxStream; use humantime_serde::re::humantime; @@ -57,15 +57,13 @@ impl Default for SchemaNameKey<'_> { #[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)] pub struct SchemaNameValue { #[serde(default)] - #[serde(with = "humantime_serde")] - pub ttl: Option, + pub ttl: TimeToLive, } impl Display for SchemaNameValue { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - if let Some(ttl) = self.ttl { - let ttl = humantime::format_duration(ttl); - write!(f, "ttl='{ttl}'")?; + if let Some(ttl) = self.ttl.as_repr_opt() { + write!(f, "ttl='{}'", ttl)?; } Ok(()) @@ -88,7 +86,8 @@ impl TryFrom<&HashMap> for SchemaNameValue { }) }) .transpose()? - .map(|ttl| ttl.into()); + .map(|ttl| ttl.into()) + .unwrap_or_default(); Ok(Self { ttl }) } } @@ -96,11 +95,8 @@ impl TryFrom<&HashMap> for SchemaNameValue { impl From for HashMap { fn from(value: SchemaNameValue) -> Self { let mut opts = HashMap::new(); - if let Some(ttl) = value.ttl { - opts.insert( - OPT_KEY_TTL.to_string(), - format!("{}", humantime::format_duration(ttl)), - ); + if let Some(ttl) = value.ttl.as_repr_opt() { + opts.insert(OPT_KEY_TTL.to_string(), ttl); } opts } @@ -313,19 +309,32 @@ impl<'a> From<&'a SchemaName> for SchemaNameKey<'a> { #[cfg(test)] mod tests { + use std::time::Duration; use super::*; use crate::kv_backend::memory::MemoryKvBackend; #[test] fn test_display_schema_value() { - let schema_value = SchemaNameValue { ttl: None }; + let schema_value = SchemaNameValue { + ttl: TimeToLive::Forever, + }; assert_eq!("", schema_value.to_string()); let schema_value = SchemaNameValue { - ttl: Some(Duration::from_secs(9)), + ttl: Duration::from_secs(9).into(), }; assert_eq!("ttl='9s'", schema_value.to_string()); + + let schema_value = SchemaNameValue { + ttl: Duration::from_secs(0).into(), + }; + assert_eq!("", schema_value.to_string()); + + let schema_value = SchemaNameValue { + ttl: TimeToLive::Immediate, + }; + assert_eq!("ttl='immediate'", schema_value.to_string()); } #[test] @@ -338,17 +347,44 @@ mod tests { assert_eq!(key, parsed); let value = SchemaNameValue { - ttl: Some(Duration::from_secs(10)), + ttl: Duration::from_secs(10).into(), }; let mut opts: HashMap = HashMap::new(); opts.insert("ttl".to_string(), "10s".to_string()); let from_value = SchemaNameValue::try_from(&opts).unwrap(); assert_eq!(value, from_value); - let parsed = SchemaNameValue::try_from_raw_value("{\"ttl\":\"10s\"}".as_bytes()).unwrap(); + let parsed = SchemaNameValue::try_from_raw_value( + serde_json::json!({"ttl": {"Duration": "10s"}}) + .to_string() + .as_bytes(), + ) + .unwrap(); assert_eq!(Some(value), parsed); + + let imme = SchemaNameValue { + ttl: TimeToLive::Immediate, + }; + let parsed = SchemaNameValue::try_from_raw_value( + serde_json::json!({"ttl": "Immediate"}) + .to_string() + .as_bytes(), + ) + .unwrap(); + assert_eq!(Some(imme), parsed); + + let forever = SchemaNameValue { + ttl: TimeToLive::Forever, + }; + let parsed = SchemaNameValue::try_from_raw_value( + serde_json::json!({"ttl": "Forever"}).to_string().as_bytes(), + ) + .unwrap(); + assert_eq!(Some(forever), parsed); + let none = SchemaNameValue::try_from_raw_value("null".as_bytes()).unwrap(); assert!(none.is_none()); + let err_empty = SchemaNameValue::try_from_raw_value("".as_bytes()); assert!(err_empty.is_err()); } @@ -374,7 +410,7 @@ mod tests { let current_schema_value = manager.get(schema_key).await.unwrap().unwrap(); let new_schema_value = SchemaNameValue { - ttl: Some(Duration::from_secs(10)), + ttl: Duration::from_secs(10).into(), }; manager .update(schema_key, ¤t_schema_value, &new_schema_value) @@ -388,10 +424,10 @@ mod tests { .unwrap(); let new_schema_value = SchemaNameValue { - ttl: Some(Duration::from_secs(40)), + ttl: Duration::from_secs(40).into(), }; let incorrect_schema_value = SchemaNameValue { - ttl: Some(Duration::from_secs(20)), + ttl: Duration::from_secs(20).into(), } .try_as_raw_value() .unwrap(); diff --git a/src/common/meta/src/rpc/ddl.rs b/src/common/meta/src/rpc/ddl.rs index 562ecb8ee660..03828a632e25 100644 --- a/src/common/meta/src/rpc/ddl.rs +++ b/src/common/meta/src/rpc/ddl.rs @@ -14,7 +14,6 @@ use std::collections::{HashMap, HashSet}; use std::result; -use std::time::Duration; use api::v1::alter_database_expr::Kind as PbAlterDatabaseKind; use api::v1::meta::ddl_task_request::Task; @@ -36,7 +35,7 @@ use api::v1::{ }; use base64::engine::general_purpose; use base64::Engine as _; -use humantime_serde::re::humantime; +use common_base::TimeToLive; use prost::Message; use serde::{Deserialize, Serialize}; use serde_with::{serde_as, DefaultOnNull}; @@ -1009,12 +1008,8 @@ impl TryFrom for SetDatabaseOption { fn try_from(PbOption { key, value }: PbOption) -> Result { match key.to_ascii_lowercase().as_str() { TTL_KEY => { - let ttl = if value.is_empty() { - Duration::from_secs(0) - } else { - humantime::parse_duration(&value) - .map_err(|_| InvalidSetDatabaseOptionSnafu { key, value }.build())? - }; + let ttl = TimeToLive::from_humantime_or_str(&value) + .map_err(|_| InvalidSetDatabaseOptionSnafu { key, value }.build())?; Ok(SetDatabaseOption::Ttl(ttl)) } @@ -1025,7 +1020,7 @@ impl TryFrom for SetDatabaseOption { #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] pub enum SetDatabaseOption { - Ttl(Duration), + Ttl(TimeToLive), } #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] diff --git a/src/metric-engine/src/engine/alter.rs b/src/metric-engine/src/engine/alter.rs index b6108f133a38..159b0068a0ab 100644 --- a/src/metric-engine/src/engine/alter.rs +++ b/src/metric-engine/src/engine/alter.rs @@ -207,7 +207,7 @@ mod test { let alter_region_option_request = RegionAlterRequest { schema_version: 0, kind: AlterKind::SetRegionOptions { - options: vec![SetRegionOption::TTL(Duration::from_secs(500))], + options: vec![SetRegionOption::TTL(Duration::from_secs(500).into())], }, }; let result = engine_inner diff --git a/src/mito2/src/compaction.rs b/src/mito2/src/compaction.rs index 44aa03a67df8..af61342e2932 100644 --- a/src/mito2/src/compaction.rs +++ b/src/mito2/src/compaction.rs @@ -24,10 +24,10 @@ mod window; use std::collections::HashMap; use std::sync::Arc; -use std::time::{Duration, Instant}; +use std::time::Instant; use api::v1::region::compact_request; -use common_base::Plugins; +use common_base::{Plugins, TimeToLive}; use common_meta::key::SchemaMetadataManagerRef; use common_telemetry::{debug, error, info, warn}; use common_time::range::TimestampRange; @@ -273,7 +273,7 @@ impl CompactionScheduler { .await .unwrap_or_else(|e| { warn!(e; "Failed to get ttl for region: {}", region_id); - None + TimeToLive::default() }); debug!( @@ -437,18 +437,20 @@ impl PendingCompaction { /// Finds TTL of table by first examine table options then database options. async fn find_ttl( table_id: TableId, - table_ttl: Option, + table_ttl: TimeToLive, schema_metadata_manager: &SchemaMetadataManagerRef, -) -> Result> { - if let Some(table_ttl) = table_ttl { - return Ok(Some(table_ttl)); +) -> Result { + // If table TTL is not forever, we use it. + if !table_ttl.is_forever() { + return Ok(table_ttl); } let ttl = schema_metadata_manager .get_schema_options_by_table_id(table_id) .await .context(GetSchemaMetadataSnafu)? - .and_then(|options| options.ttl); + .map(|options| options.ttl) + .unwrap_or_default(); Ok(ttl) } @@ -654,12 +656,8 @@ fn ts_to_lit(ts: Timestamp, ts_col_unit: TimeUnit) -> Result { } /// Finds all expired SSTs across levels. -fn get_expired_ssts( - levels: &[LevelMeta], - ttl: Option, - now: Timestamp, -) -> Vec { - let Some(ttl) = ttl else { +fn get_expired_ssts(levels: &[LevelMeta], ttl: TimeToLive, now: Timestamp) -> Vec { + let Some(ttl) = ttl.get_duration() else { return vec![]; }; diff --git a/src/mito2/src/compaction/compactor.rs b/src/mito2/src/compaction/compactor.rs index 3e0228a4b2a4..0fca2c754833 100644 --- a/src/mito2/src/compaction/compactor.rs +++ b/src/mito2/src/compaction/compactor.rs @@ -16,6 +16,7 @@ use std::sync::Arc; use std::time::Duration; use api::v1::region::compact_request; +use common_base::TimeToLive; use common_meta::key::SchemaMetadataManagerRef; use common_telemetry::{info, warn}; use object_store::manager::ObjectStoreManagerRef; @@ -63,7 +64,7 @@ pub struct CompactionRegion { pub(crate) manifest_ctx: Arc, pub(crate) current_version: VersionRef, pub(crate) file_purger: Option>, - pub(crate) ttl: Option, + pub(crate) ttl: TimeToLive, } /// OpenCompactionRegionRequest represents the request to open a compaction region. @@ -180,7 +181,7 @@ pub async fn open_compaction_region( .await .unwrap_or_else(|e| { warn!(e; "Failed to get ttl for region: {}", region_metadata.region_id); - None + TimeToLive::default() }); Ok(CompactionRegion { region_id: req.region_id, diff --git a/src/mito2/src/compaction/window.rs b/src/mito2/src/compaction/window.rs index 9d6207bba298..507130052956 100644 --- a/src/mito2/src/compaction/window.rs +++ b/src/mito2/src/compaction/window.rs @@ -253,7 +253,7 @@ mod tests { truncated_entry_id: None, compaction_time_window: None, options: RegionOptions { - ttl, + ttl: ttl.into(), compaction: Default::default(), storage: None, append_mode: false, diff --git a/src/mito2/src/engine/alter_test.rs b/src/mito2/src/engine/alter_test.rs index 069d64fb5a87..2841d3e2ed05 100644 --- a/src/mito2/src/engine/alter_test.rs +++ b/src/mito2/src/engine/alter_test.rs @@ -604,7 +604,7 @@ async fn test_alter_region_ttl_options() { let alter_ttl_request = RegionAlterRequest { schema_version: 0, kind: AlterKind::SetRegionOptions { - options: vec![SetRegionOption::TTL(Duration::from_secs(500))], + options: vec![SetRegionOption::TTL(Duration::from_secs(500).into())], }, }; let alter_job = tokio::spawn(async move { @@ -617,14 +617,8 @@ async fn test_alter_region_ttl_options() { alter_job.await.unwrap(); let check_ttl = |engine: &MitoEngine, expected: &Duration| { - let current_ttl = engine - .get_region(region_id) - .unwrap() - .version() - .options - .ttl - .unwrap(); - assert_eq!(*expected, current_ttl); + let current_ttl = engine.get_region(region_id).unwrap().version().options.ttl; + assert_eq!(current_ttl, (*expected).into()); }; // Verify the ttl. check_ttl(&engine, &Duration::from_secs(500)); diff --git a/src/mito2/src/engine/create_test.rs b/src/mito2/src/engine/create_test.rs index 9ce3c53b7661..b3409a276409 100644 --- a/src/mito2/src/engine/create_test.rs +++ b/src/mito2/src/engine/create_test.rs @@ -165,8 +165,8 @@ async fn test_engine_create_with_options() { assert!(engine.is_region_exists(region_id)); let region = engine.get_region(region_id).unwrap(); assert_eq!( - Duration::from_secs(3600 * 24 * 10), - region.version().options.ttl.unwrap() + region.version().options.ttl, + Duration::from_secs(3600 * 24 * 10).into() ); } diff --git a/src/mito2/src/engine/open_test.rs b/src/mito2/src/engine/open_test.rs index 8fd084a24ffa..1964d012beab 100644 --- a/src/mito2/src/engine/open_test.rs +++ b/src/mito2/src/engine/open_test.rs @@ -180,8 +180,8 @@ async fn test_engine_region_open_with_options() { let region = engine.get_region(region_id).unwrap(); assert_eq!( - Duration::from_secs(3600 * 24 * 4), - region.version().options.ttl.unwrap() + region.version().options.ttl, + Duration::from_secs(3600 * 24 * 4).into() ); } diff --git a/src/mito2/src/region/options.rs b/src/mito2/src/region/options.rs index 4abc5925b705..390774c50f84 100644 --- a/src/mito2/src/region/options.rs +++ b/src/mito2/src/region/options.rs @@ -20,6 +20,7 @@ use std::collections::HashMap; use std::time::Duration; use common_base::readable_size::ReadableSize; +use common_base::TimeToLive; use common_wal::options::{WalOptions, WAL_OPTIONS_KEY}; use serde::de::Error as _; use serde::{Deserialize, Deserializer, Serialize}; @@ -55,8 +56,7 @@ pub enum MergeMode { #[serde(default)] pub struct RegionOptions { /// Region SST files TTL. - #[serde(with = "humantime_serde")] - pub ttl: Option, + pub ttl: TimeToLive, /// Compaction options. pub compaction: CompactionOptions, /// Custom storage. Uses default storage if it is `None`. @@ -252,8 +252,7 @@ impl Default for TwcsOptions { #[serde(default)] struct RegionOptionsWithoutEnum { /// Region SST files TTL. - #[serde(with = "humantime_serde")] - ttl: Option, + ttl: TimeToLive, storage: Option, #[serde_as(as = "DisplayFromStr")] append_mode: bool, @@ -458,7 +457,7 @@ mod tests { let map = make_map(&[("ttl", "7d")]); let options = RegionOptions::try_from(&map).unwrap(); let expect = RegionOptions { - ttl: Some(Duration::from_secs(3600 * 24 * 7)), + ttl: Duration::from_secs(3600 * 24 * 7).into(), ..Default::default() }; assert_eq!(expect, options); @@ -621,7 +620,7 @@ mod tests { ]); let options = RegionOptions::try_from(&map).unwrap(); let expect = RegionOptions { - ttl: Some(Duration::from_secs(3600 * 24 * 7)), + ttl: Duration::from_secs(3600 * 24 * 7).into(), compaction: CompactionOptions::Twcs(TwcsOptions { max_active_window_runs: 8, max_active_window_files: 11, @@ -654,7 +653,7 @@ mod tests { #[test] fn test_region_options_serde() { let options = RegionOptions { - ttl: Some(Duration::from_secs(3600 * 24 * 7)), + ttl: Duration::from_secs(3600 * 24 * 7).into(), compaction: CompactionOptions::Twcs(TwcsOptions { max_active_window_runs: 8, max_active_window_files: usize::MAX, @@ -722,7 +721,7 @@ mod tests { }"#; let got: RegionOptions = serde_json::from_str(region_options_json_str).unwrap(); let options = RegionOptions { - ttl: Some(Duration::from_secs(3600 * 24 * 7)), + ttl: Duration::from_secs(3600 * 24 * 7).into(), compaction: CompactionOptions::Twcs(TwcsOptions { max_active_window_runs: 8, max_active_window_files: 11, diff --git a/src/mito2/src/worker/handle_alter.rs b/src/mito2/src/worker/handle_alter.rs index 3908cee98be0..c5c45f2d440d 100644 --- a/src/mito2/src/worker/handle_alter.rs +++ b/src/mito2/src/worker/handle_alter.rs @@ -189,11 +189,7 @@ impl RegionWorkerLoop { "Update region ttl: {}, previous: {:?} new: {:?}", region.region_id, current_options.ttl, new_ttl ); - if new_ttl.is_zero() { - current_options.ttl = None; - } else { - current_options.ttl = Some(new_ttl); - } + current_options.ttl = new_ttl; } SetRegionOption::Twsc(key, value) => { let Twcs(options) = &mut current_options.compaction; diff --git a/src/operator/src/insert.rs b/src/operator/src/insert.rs index 3fda3c32f624..7817cbdce8cb 100644 --- a/src/operator/src/insert.rs +++ b/src/operator/src/insert.rs @@ -471,12 +471,9 @@ impl Inserter { ) -> Result> { fn add_ttl_zero_table(ctx: &QueryContextRef, table_info: &TableInfo) { let ttl = table_info.meta.options.ttl; - if let Some(ttl) = ttl { - if ttl.is_zero() { - ctx.add_ttl_zero_regions( - table_info.region_ids().into_iter().map(|i| i.as_u64()), - ); - } + + if ttl.is_immediate() { + ctx.add_ttl_zero_regions(table_info.region_ids().into_iter().map(|i| i.as_u64())); } } let _timer = crate::metrics::CREATE_ALTER_ON_DEMAND diff --git a/src/query/src/sql/show_create_table.rs b/src/query/src/sql/show_create_table.rs index 68be43785703..5004c89c9591 100644 --- a/src/query/src/sql/show_create_table.rs +++ b/src/query/src/sql/show_create_table.rs @@ -21,7 +21,6 @@ use datatypes::schema::{ ColumnDefaultConstraint, ColumnSchema, SchemaRef, COLUMN_FULLTEXT_OPT_KEY_ANALYZER, COLUMN_FULLTEXT_OPT_KEY_CASE_SENSITIVE, COMMENT_KEY, }; -use humantime::format_duration; use snafu::ResultExt; use sql::ast::{ColumnDef, ColumnOption, ColumnOptionDef, Expr, Ident, ObjectName}; use sql::dialect::GreptimeDbDialect; @@ -46,13 +45,10 @@ fn create_sql_options(table_meta: &TableMeta, schema_options: Option, /// regions with ttl=0 - ttl_zero_regions: BTreeSet, + ttl_zero_regions: HashSet, } impl Display for QueryContext { @@ -294,7 +295,7 @@ impl QueryContext { .extend(region_ids); } - pub fn ttl_zero_regions(&self) -> BTreeSet { + pub fn ttl_zero_regions(&self) -> HashSet { self.mutable_query_context_data .read() .unwrap() diff --git a/src/store-api/src/region_request.rs b/src/store-api/src/region_request.rs index dc37e4a2cce4..74190a8a3af9 100644 --- a/src/store-api/src/region_request.rs +++ b/src/store-api/src/region_request.rs @@ -14,7 +14,6 @@ use std::collections::HashMap; use std::fmt::{self, Display}; -use std::time::Duration; use api::helper::ColumnDataTypeWrapper; use api::v1::add_column_location::LocationType; @@ -26,6 +25,7 @@ use api::v1::region::{ }; use api::v1::{self, Analyzer, Option as PbOption, Rows, SemanticType}; pub use common_base::AffectedRows; +use common_base::TimeToLive; use datatypes::data_type::ConcreteDataType; use datatypes::schema::FulltextOptions; use serde::{Deserialize, Serialize}; @@ -746,7 +746,7 @@ impl From for ModifyColumnType { #[derive(Debug, Eq, PartialEq, Clone, Serialize, Deserialize)] pub enum SetRegionOption { - TTL(Duration), + TTL(TimeToLive), // Modifying TwscOptions with values as (option name, new value). Twsc(String, String), } @@ -758,12 +758,9 @@ impl TryFrom<&PbOption> for SetRegionOption { let PbOption { key, value } = value; match key.as_str() { TTL_KEY => { - let ttl = if value.is_empty() { - Duration::from_secs(0) - } else { - humantime::parse_duration(value) - .map_err(|_| InvalidSetRegionOptionRequestSnafu { key, value }.build())? - }; + let ttl = TimeToLive::from_humantime_or_str(value) + .map_err(|_| InvalidSetRegionOptionRequestSnafu { key, value }.build())?; + Ok(Self::TTL(ttl)) } TWCS_MAX_ACTIVE_WINDOW_RUNS @@ -798,7 +795,7 @@ impl From<&UnsetRegionOption> for SetRegionOption { UnsetRegionOption::TwcsTimeWindow => { SetRegionOption::Twsc(unset_option.to_string(), String::new()) } - UnsetRegionOption::Ttl => SetRegionOption::TTL(Duration::default()), + UnsetRegionOption::Ttl => SetRegionOption::TTL(Default::default()), } } } diff --git a/src/table/src/metadata.rs b/src/table/src/metadata.rs index 7f4ddb409acd..36873c9d737f 100644 --- a/src/table/src/metadata.rs +++ b/src/table/src/metadata.rs @@ -225,11 +225,7 @@ impl TableMeta { for request in requests { match request { SetRegionOption::TTL(new_ttl) => { - if new_ttl.is_zero() { - new_options.ttl = None; - } else { - new_options.ttl = Some(*new_ttl); - } + new_options.ttl = *new_ttl; } SetRegionOption::Twsc(key, value) => { if !value.is_empty() { diff --git a/src/table/src/requests.rs b/src/table/src/requests.rs index 4c273e03b785..4c2433f8a017 100644 --- a/src/table/src/requests.rs +++ b/src/table/src/requests.rs @@ -17,9 +17,9 @@ use std::collections::HashMap; use std::fmt; use std::str::FromStr; -use std::time::Duration; use common_base::readable_size::ReadableSize; +use common_base::TimeToLive; use common_datasource::object_store::s3::is_supported_in_s3; use common_query::AddColumnLocation; use common_time::range::TimestampRange; @@ -74,8 +74,7 @@ pub struct TableOptions { /// Memtable size of memtable. pub write_buffer_size: Option, /// Time-to-live of table. Expired data will be automatically purged. - #[serde(with = "humantime_serde")] - pub ttl: Option, + pub ttl: TimeToLive, /// Extra options that may not applicable to all table engines. pub extra_options: HashMap, } @@ -109,17 +108,14 @@ impl TableOptions { } if let Some(ttl) = kvs.get(TTL_KEY) { - let ttl_value = ttl - .parse::() - .map_err(|_| { - ParseTableOptionSnafu { - key: TTL_KEY, - value: ttl, - } - .build() - })? - .into(); - options.ttl = Some(ttl_value); + let ttl_value = TimeToLive::from_humantime_or_str(ttl).map_err(|_| { + ParseTableOptionSnafu { + key: TTL_KEY, + value: ttl, + } + .build() + })?; + options.ttl = ttl_value; } options.extra_options = HashMap::from_iter( @@ -138,8 +134,8 @@ impl fmt::Display for TableOptions { key_vals.push(format!("{}={}", WRITE_BUFFER_SIZE_KEY, size)); } - if let Some(ttl) = self.ttl { - key_vals.push(format!("{}={}", TTL_KEY, humantime::Duration::from(ttl))); + if let Some(ttl) = self.ttl.as_repr_opt() { + key_vals.push(format!("{}={}", TTL_KEY, ttl)); } for (k, v) in &self.extra_options { @@ -159,8 +155,7 @@ impl From<&TableOptions> for HashMap { write_buffer_size.to_string(), ); } - if let Some(ttl) = opts.ttl { - let ttl_str = humantime::format_duration(ttl).to_string(); + if let Some(ttl_str) = opts.ttl.as_repr_opt() { let _ = res.insert(TTL_KEY.to_string(), ttl_str); } res.extend( @@ -326,6 +321,8 @@ pub struct CopyDatabaseRequest { #[cfg(test)] mod tests { + use std::time::Duration; + use super::*; #[test] @@ -343,7 +340,7 @@ mod tests { fn test_serialize_table_options() { let options = TableOptions { write_buffer_size: None, - ttl: Some(Duration::from_secs(1000)), + ttl: Some(Duration::from_secs(1000)).into(), extra_options: HashMap::new(), }; let serialized = serde_json::to_string(&options).unwrap(); @@ -355,7 +352,7 @@ mod tests { fn test_convert_hashmap_between_table_options() { let options = TableOptions { write_buffer_size: Some(ReadableSize::mb(128)), - ttl: Some(Duration::from_secs(1000)), + ttl: Some(Duration::from_secs(1000)).into(), extra_options: HashMap::new(), }; let serialized_map = HashMap::from(&options); @@ -364,7 +361,7 @@ mod tests { let options = TableOptions { write_buffer_size: None, - ttl: None, + ttl: Default::default(), extra_options: HashMap::new(), }; let serialized_map = HashMap::from(&options); @@ -373,7 +370,7 @@ mod tests { let options = TableOptions { write_buffer_size: Some(ReadableSize::mb(128)), - ttl: Some(Duration::from_secs(1000)), + ttl: Some(Duration::from_secs(1000)).into(), extra_options: HashMap::from([("a".to_string(), "A".to_string())]), }; let serialized_map = HashMap::from(&options); @@ -385,7 +382,7 @@ mod tests { fn test_table_options_to_string() { let options = TableOptions { write_buffer_size: Some(ReadableSize::mb(128)), - ttl: Some(Duration::from_secs(1000)), + ttl: Some(Duration::from_secs(1000)).into(), extra_options: HashMap::new(), }; @@ -396,7 +393,7 @@ mod tests { let options = TableOptions { write_buffer_size: Some(ReadableSize::mb(128)), - ttl: Some(Duration::from_secs(1000)), + ttl: Some(Duration::from_secs(1000)).into(), extra_options: HashMap::from([("a".to_string(), "A".to_string())]), }; From 9a76968adf4ee0aeed10738169b7d959aafbb4fd Mon Sep 17 00:00:00 2001 From: discord9 Date: Tue, 3 Dec 2024 20:10:53 +0800 Subject: [PATCH 03/21] fix: unit test --- Cargo.lock | 1 + src/common/base/Cargo.toml | 1 + src/common/base/src/ttl.rs | 77 ++++++++++++++++++++++- src/common/meta/src/ddl/alter_database.rs | 4 +- src/common/meta/src/key/schema_name.rs | 12 ++-- 5 files changed, 84 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6c64f5084a3d..0590893e4dca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1904,6 +1904,7 @@ dependencies = [ "paste", "pin-project", "serde", + "serde_json", "snafu 0.8.5", "tokio", "toml 0.8.19", diff --git a/src/common/base/Cargo.toml b/src/common/base/Cargo.toml index 8563697d95b1..2631e6f56c1a 100644 --- a/src/common/base/Cargo.toml +++ b/src/common/base/Cargo.toml @@ -26,4 +26,5 @@ zeroize = { version = "1.6", default-features = false, features = ["alloc"] } [dev-dependencies] common-test-util.workspace = true +serde_json.workspace = true toml.workspace = true diff --git a/src/common/base/src/ttl.rs b/src/common/base/src/ttl.rs index 0abb7139986d..93fcb7fbfe90 100644 --- a/src/common/base/src/ttl.rs +++ b/src/common/base/src/ttl.rs @@ -15,20 +15,65 @@ use std::fmt::Display; use std::time::Duration; +use serde::de::Visitor; use serde::{Deserialize, Serialize}; /// Time To Live -#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default, Serialize, Deserialize)] +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default)] pub enum TimeToLive { /// immediately throw away on insert Immediate, /// Duration to keep the data, this duration should be non-zero - #[serde(with = "humantime_serde")] Duration(Duration), /// Keep the data forever #[default] Forever, + // TODO(discord9): add a new variant + // that can't be overridden by database level ttl? call it ForceForever? +} + +impl Serialize for TimeToLive { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + match self { + Self::Immediate => serializer.serialize_str("immediate"), + Self::Duration(d) => humantime_serde::serialize(d, serializer), + Self::Forever => serializer.serialize_str("forever"), + } + } +} + +impl<'de> Deserialize<'de> for TimeToLive { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + struct StrVisitor; + impl Visitor<'_> for StrVisitor { + type Value = TimeToLive; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("a string of time, 'immediate', 'forever' or null") + } + + fn visit_unit(self) -> Result { + Ok(TimeToLive::Forever) + } + + fn visit_str(self, value: &str) -> Result + where + E: serde::de::Error, + { + TimeToLive::from_humantime_or_str(value).map_err(serde::de::Error::custom) + } + } + // deser a string or null + let any = deserializer.deserialize_any(StrVisitor)?; + Ok(any) + } } impl Display for TimeToLive { @@ -106,3 +151,31 @@ impl From for TimeToLive { Self::from(*duration) } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_serde() { + let cases = vec![ + ("\"immediate\"", TimeToLive::Immediate), + ("\"forever\"", TimeToLive::Forever), + ("\"10d\"", Duration::from_secs(86400 * 10).into()), + ( + "\"10000 years\"", + humantime::parse_duration("10000 years").unwrap().into(), + ), + ("null", TimeToLive::Forever), + ]; + + for (s, expected) in cases { + let serialized = serde_json::to_string(&expected).unwrap(); + let deserialized: TimeToLive = serde_json::from_str(&serialized).unwrap(); + assert_eq!(deserialized, expected); + + let deserialized: TimeToLive = serde_json::from_str(s).unwrap(); + assert_eq!(deserialized, expected); + } + } +} diff --git a/src/common/meta/src/ddl/alter_database.rs b/src/common/meta/src/ddl/alter_database.rs index a005fa32ab65..e2cd760e6190 100644 --- a/src/common/meta/src/ddl/alter_database.rs +++ b/src/common/meta/src/ddl/alter_database.rs @@ -55,7 +55,7 @@ fn build_new_schema_value( AlterDatabaseKind::UnsetDatabaseOptions(keys) => { for key in keys.0.iter() { match key { - UnsetDatabaseOption::Ttl => value.ttl = TimeToLive::Forever, + UnsetDatabaseOption::Ttl => value.ttl = TimeToLive::default(), } } } @@ -242,6 +242,6 @@ mod tests { ])); let new_schema_value = build_new_schema_value(current_schema_value, &unset_ttl_alter_kind).unwrap(); - assert_eq!(new_schema_value.ttl, TimeToLive::Forever); + assert_eq!(new_schema_value.ttl, TimeToLive::default()); } } diff --git a/src/common/meta/src/key/schema_name.rs b/src/common/meta/src/key/schema_name.rs index bdc287e638a8..c21d320478e1 100644 --- a/src/common/meta/src/key/schema_name.rs +++ b/src/common/meta/src/key/schema_name.rs @@ -317,7 +317,7 @@ mod tests { #[test] fn test_display_schema_value() { let schema_value = SchemaNameValue { - ttl: TimeToLive::Forever, + ttl: TimeToLive::default(), }; assert_eq!("", schema_value.to_string()); @@ -355,9 +355,7 @@ mod tests { assert_eq!(value, from_value); let parsed = SchemaNameValue::try_from_raw_value( - serde_json::json!({"ttl": {"Duration": "10s"}}) - .to_string() - .as_bytes(), + serde_json::json!({"ttl": "10s"}).to_string().as_bytes(), ) .unwrap(); assert_eq!(Some(value), parsed); @@ -366,7 +364,7 @@ mod tests { ttl: TimeToLive::Immediate, }; let parsed = SchemaNameValue::try_from_raw_value( - serde_json::json!({"ttl": "Immediate"}) + serde_json::json!({"ttl": "immediate"}) .to_string() .as_bytes(), ) @@ -374,10 +372,10 @@ mod tests { assert_eq!(Some(imme), parsed); let forever = SchemaNameValue { - ttl: TimeToLive::Forever, + ttl: TimeToLive::default(), }; let parsed = SchemaNameValue::try_from_raw_value( - serde_json::json!({"ttl": "Forever"}).to_string().as_bytes(), + serde_json::json!({"ttl": "forever"}).to_string().as_bytes(), ) .unwrap(); assert_eq!(Some(forever), parsed); From 60509a546586841a6c905573d841fb9c1a2d9d24 Mon Sep 17 00:00:00 2001 From: discord9 Date: Wed, 4 Dec 2024 12:55:09 +0800 Subject: [PATCH 04/21] tests: sqlness --- src/common/base/src/ttl.rs | 7 +- src/operator/src/insert.rs | 22 +- .../src/req_convert/insert/stmt_to_region.rs | 2 + .../standalone/common/ttl/show_ttl.result | 313 ++++++++++++++++++ .../cases/standalone/common/ttl/show_ttl.sql | 67 ++++ .../standalone/common/ttl/ttl_imme.result | 57 ++++ .../cases/standalone/common/ttl/ttl_imme.sql | 17 + 7 files changed, 473 insertions(+), 12 deletions(-) create mode 100644 tests/cases/standalone/common/ttl/show_ttl.result create mode 100644 tests/cases/standalone/common/ttl/show_ttl.sql create mode 100644 tests/cases/standalone/common/ttl/ttl_imme.result create mode 100644 tests/cases/standalone/common/ttl/ttl_imme.sql diff --git a/src/common/base/src/ttl.rs b/src/common/base/src/ttl.rs index 93fcb7fbfe90..e4609bf145e9 100644 --- a/src/common/base/src/ttl.rs +++ b/src/common/base/src/ttl.rs @@ -80,7 +80,7 @@ impl Display for TimeToLive { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { TimeToLive::Immediate => write!(f, "immediate"), - TimeToLive::Duration(d) => write!(f, "Duration({})", d.as_secs()), + TimeToLive::Duration(d) => write!(f, "{}", humantime::Duration::from(*d)), TimeToLive::Forever => write!(f, "forever"), } } @@ -94,14 +94,14 @@ impl TimeToLive { "forever" | "" => Ok(TimeToLive::Forever), _ => { let d = humantime::parse_duration(s).map_err(|e| e.to_string())?; - Ok(TimeToLive::Duration(d)) + Ok(TimeToLive::from(d)) } } } /// Print TimeToLive as string /// - /// omit forever variant + /// omit `Forever`` variant pub fn as_repr_opt(&self) -> Option { match self { TimeToLive::Immediate => Some("immediate".to_string()), @@ -114,6 +114,7 @@ impl TimeToLive { matches!(self, TimeToLive::Immediate) } + /// Is the default value, which is `Forever` pub fn is_forever(&self) -> bool { matches!(self, TimeToLive::Forever) } diff --git a/src/operator/src/insert.rs b/src/operator/src/insert.rs index 7817cbdce8cb..65ed5678e133 100644 --- a/src/operator/src/insert.rs +++ b/src/operator/src/insert.rs @@ -244,6 +244,7 @@ impl Inserter { table_name: common_catalog::format_full_table_name(catalog, schema, table_name), })?; let table_info = table.table_info(); + check_ttl_zero_table(&ctx, &table_info); let inserts = TableToRegion::new(&table_info, &self.partition_manager) .convert(request) @@ -266,6 +267,15 @@ impl Inserter { } } +/// Check and add regions with ttl=0 to context +pub(crate) fn check_ttl_zero_table(ctx: &QueryContextRef, table_info: &TableInfo) { + let ttl = table_info.meta.options.ttl; + + if ttl.is_immediate() { + ctx.add_ttl_zero_regions(table_info.region_ids().into_iter().map(|i| i.as_u64())); + } +} + impl Inserter { async fn do_request( &self, @@ -469,13 +479,6 @@ impl Inserter { auto_create_table_type: AutoCreateTableType, statement_executor: &StatementExecutor, ) -> Result> { - fn add_ttl_zero_table(ctx: &QueryContextRef, table_info: &TableInfo) { - let ttl = table_info.meta.options.ttl; - - if ttl.is_immediate() { - ctx.add_ttl_zero_regions(table_info.region_ids().into_iter().map(|i| i.as_u64())); - } - } let _timer = crate::metrics::CREATE_ALTER_ON_DEMAND .with_label_values(&[auto_create_table_type.as_str()]) .start_timer(); @@ -507,7 +510,7 @@ impl Inserter { ), })?; let table_info = table.table_info(); - add_ttl_zero_table(ctx, &table_info); + check_ttl_zero_table(ctx, &table_info); table_name_to_ids.insert(table_info.name.clone(), table_info.table_id()); } return Ok(table_name_to_ids); @@ -572,6 +575,7 @@ impl Inserter { } } + // add ttl zero regions to context for table_name in table_name_to_ids.keys() { let table = if let Some(table) = self.get_table(catalog, &schema, table_name).await? { table @@ -579,7 +583,7 @@ impl Inserter { continue; }; let table_info = table.table_info(); - add_ttl_zero_table(ctx, &table_info); + check_ttl_zero_table(ctx, &table_info); } Ok(table_name_to_ids) diff --git a/src/operator/src/req_convert/insert/stmt_to_region.rs b/src/operator/src/req_convert/insert/stmt_to_region.rs index 8124edc19514..028e6e025e06 100644 --- a/src/operator/src/req_convert/insert/stmt_to_region.rs +++ b/src/operator/src/req_convert/insert/stmt_to_region.rs @@ -32,6 +32,7 @@ use crate::error::{ ColumnNotFoundSnafu, InvalidSqlSnafu, MissingInsertBodySnafu, ParseSqlSnafu, Result, SchemaReadOnlySnafu, TableNotFoundSnafu, }; +use crate::insert::check_ttl_zero_table; use crate::req_convert::common::partitioner::Partitioner; use crate::req_convert::insert::semantic_type; @@ -65,6 +66,7 @@ impl<'a> StatementToRegion<'a> { let table = self.get_table(&catalog, &schema, &table_name).await?; let table_schema = table.schema(); let table_info = table.table_info(); + check_ttl_zero_table(query_ctx, &table_info); ensure!( !common_catalog::consts::is_readonly_schema(&schema), diff --git a/tests/cases/standalone/common/ttl/show_ttl.result b/tests/cases/standalone/common/ttl/show_ttl.result new file mode 100644 index 000000000000..650c49b2ad7a --- /dev/null +++ b/tests/cases/standalone/common/ttl/show_ttl.result @@ -0,0 +1,313 @@ +CREATE DATABASE test_ttl_db WITH (ttl = '1 second'); + +Affected Rows: 1 + +USE test_ttl_db; + +Affected Rows: 0 + +CREATE TABLE test_ttl(ts TIMESTAMP TIME INDEX, val INT); + +Affected Rows: 0 + +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | WITH( | +| | ttl = '1s' | +| | ) | ++----------+-----------------------------------------+ + +SHOW CREATE DATABASE test_ttl_db; + ++-------------+-------------------------------------------+ +| Database | Create Database | ++-------------+-------------------------------------------+ +| test_ttl_db | CREATE DATABASE IF NOT EXISTS test_ttl_db | +| | WITH( | +| | ttl = '1s' | +| | ) | ++-------------+-------------------------------------------+ + +ALTER DATABASE test_ttl_db SET ttl = '1 day'; + +Affected Rows: 0 + +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | WITH( | +| | ttl = '1day' | +| | ) | ++----------+-----------------------------------------+ + +SHOW CREATE DATABASE test_ttl_db; + ++-------------+-------------------------------------------+ +| Database | Create Database | ++-------------+-------------------------------------------+ +| test_ttl_db | CREATE DATABASE IF NOT EXISTS test_ttl_db | +| | WITH( | +| | ttl = '1day' | +| | ) | ++-------------+-------------------------------------------+ + +ALTER TABLE test_ttl SET 'ttl' = '6 hours'; + +Affected Rows: 0 + +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | WITH( | +| | ttl = '6h' | +| | ) | ++----------+-----------------------------------------+ + +ALTER TABLE test_ttl SET 'ttl' = 'immediate'; + +Affected Rows: 0 + +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | WITH( | +| | ttl = 'immediate' | +| | ) | ++----------+-----------------------------------------+ + +ALTER TABLE test_ttl SET 'ttl' = '0s'; + +Affected Rows: 0 + +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | WITH( | +| | ttl = '1day' | +| | ) | ++----------+-----------------------------------------+ + +ALTER TABLE test_ttl SET 'ttl' = 'forever'; + +Affected Rows: 0 + +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | WITH( | +| | ttl = '1day' | +| | ) | ++----------+-----------------------------------------+ + +SHOW CREATE DATABASE test_ttl_db; + ++-------------+-------------------------------------------+ +| Database | Create Database | ++-------------+-------------------------------------------+ +| test_ttl_db | CREATE DATABASE IF NOT EXISTS test_ttl_db | +| | WITH( | +| | ttl = '1day' | +| | ) | ++-------------+-------------------------------------------+ + +ALTER TABLE test_ttl UNSET 'ttl'; + +Affected Rows: 0 + +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | WITH( | +| | ttl = '1day' | +| | ) | ++----------+-----------------------------------------+ + +ALTER DATABASE test_ttl_db SET 'ttl' = 'forever'; + +Affected Rows: 0 + +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | | ++----------+-----------------------------------------+ + +SHOW CREATE DATABASE test_ttl_db; + ++-------------+-------------------------------------------+ +| Database | Create Database | ++-------------+-------------------------------------------+ +| test_ttl_db | CREATE DATABASE IF NOT EXISTS test_ttl_db | ++-------------+-------------------------------------------+ + +ALTER DATABASE test_ttl_db SET 'ttl' = '0s'; + +Affected Rows: 0 + +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | | ++----------+-----------------------------------------+ + +SHOW CREATE DATABASE test_ttl_db; + ++-------------+-------------------------------------------+ +| Database | Create Database | ++-------------+-------------------------------------------+ +| test_ttl_db | CREATE DATABASE IF NOT EXISTS test_ttl_db | ++-------------+-------------------------------------------+ + +ALTER DATABASE test_ttl_db SET 'ttl' = 'immediate'; + +Affected Rows: 0 + +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | WITH( | +| | ttl = 'immediate' | +| | ) | ++----------+-----------------------------------------+ + +SHOW CREATE DATABASE test_ttl_db; + ++-------------+-------------------------------------------+ +| Database | Create Database | ++-------------+-------------------------------------------+ +| test_ttl_db | CREATE DATABASE IF NOT EXISTS test_ttl_db | +| | WITH( | +| | ttl = 'immediate' | +| | ) | ++-------------+-------------------------------------------+ + +ALTER DATABASE test_ttl_db UNSET 'ttl'; + +Affected Rows: 0 + +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | | ++----------+-----------------------------------------+ + +SHOW CREATE DATABASE test_ttl_db; + ++-------------+-------------------------------------------+ +| Database | Create Database | ++-------------+-------------------------------------------+ +| test_ttl_db | CREATE DATABASE IF NOT EXISTS test_ttl_db | ++-------------+-------------------------------------------+ + +DROP TABLE test_ttl; + +Affected Rows: 0 + +USE public; + +Affected Rows: 0 + +DROP DATABASE test_ttl_db; + +Affected Rows: 0 + diff --git a/tests/cases/standalone/common/ttl/show_ttl.sql b/tests/cases/standalone/common/ttl/show_ttl.sql new file mode 100644 index 000000000000..83629b820fc4 --- /dev/null +++ b/tests/cases/standalone/common/ttl/show_ttl.sql @@ -0,0 +1,67 @@ +CREATE DATABASE test_ttl_db WITH (ttl = '1 second'); + +USE test_ttl_db; + +CREATE TABLE test_ttl(ts TIMESTAMP TIME INDEX, val INT); + +SHOW CREATE TABLE test_ttl; + +SHOW CREATE DATABASE test_ttl_db; + +ALTER DATABASE test_ttl_db SET ttl = '1 day'; + +SHOW CREATE TABLE test_ttl; + +SHOW CREATE DATABASE test_ttl_db; + +ALTER TABLE test_ttl SET 'ttl' = '6 hours'; + +SHOW CREATE TABLE test_ttl; + +ALTER TABLE test_ttl SET 'ttl' = 'immediate'; + +SHOW CREATE TABLE test_ttl; + +ALTER TABLE test_ttl SET 'ttl' = '0s'; + +SHOW CREATE TABLE test_ttl; + +ALTER TABLE test_ttl SET 'ttl' = 'forever'; + +SHOW CREATE TABLE test_ttl; + +SHOW CREATE DATABASE test_ttl_db; + +ALTER TABLE test_ttl UNSET 'ttl'; + +SHOW CREATE TABLE test_ttl; + +ALTER DATABASE test_ttl_db SET 'ttl' = 'forever'; + +SHOW CREATE TABLE test_ttl; + +SHOW CREATE DATABASE test_ttl_db; + +ALTER DATABASE test_ttl_db SET 'ttl' = '0s'; + +SHOW CREATE TABLE test_ttl; + +SHOW CREATE DATABASE test_ttl_db; + +ALTER DATABASE test_ttl_db SET 'ttl' = 'immediate'; + +SHOW CREATE TABLE test_ttl; + +SHOW CREATE DATABASE test_ttl_db; + +ALTER DATABASE test_ttl_db UNSET 'ttl'; + +SHOW CREATE TABLE test_ttl; + +SHOW CREATE DATABASE test_ttl_db; + +DROP TABLE test_ttl; + +USE public; + +DROP DATABASE test_ttl_db; \ No newline at end of file diff --git a/tests/cases/standalone/common/ttl/ttl_imme.result b/tests/cases/standalone/common/ttl/ttl_imme.result new file mode 100644 index 000000000000..a1bc767682e6 --- /dev/null +++ b/tests/cases/standalone/common/ttl/ttl_imme.result @@ -0,0 +1,57 @@ +CREATE TABLE test_ttl(ts TIMESTAMP TIME INDEX, val INT) WITH (ttl = 'immediate'); + +Affected Rows: 0 + +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | WITH( | +| | ttl = 'immediate' | +| | ) | ++----------+-----------------------------------------+ + +INSERT INTO test_ttl VALUES + (now(), 1); + +Affected Rows: 0 + +SELECT val from test_ttl; + +++ +++ + +-- SQLNESS SLEEP 2s +ADMIN flush_table('test_ttl'); + ++-------------------------------+ +| ADMIN flush_table('test_ttl') | ++-------------------------------+ +| 0 | ++-------------------------------+ + +ADMIN compact_table('test_ttl'); + ++---------------------------------+ +| ADMIN compact_table('test_ttl') | ++---------------------------------+ +| 0 | ++---------------------------------+ + +SELECT val from test_ttl; + +++ +++ + +DROP TABLE test_ttl; + +Affected Rows: 0 + diff --git a/tests/cases/standalone/common/ttl/ttl_imme.sql b/tests/cases/standalone/common/ttl/ttl_imme.sql new file mode 100644 index 000000000000..bb02e9a428a9 --- /dev/null +++ b/tests/cases/standalone/common/ttl/ttl_imme.sql @@ -0,0 +1,17 @@ +CREATE TABLE test_ttl(ts TIMESTAMP TIME INDEX, val INT) WITH (ttl = 'immediate'); + +SHOW CREATE TABLE test_ttl; + +INSERT INTO test_ttl VALUES + (now(), 1); + +SELECT val from test_ttl; + +-- SQLNESS SLEEP 2s +ADMIN flush_table('test_ttl'); + +ADMIN compact_table('test_ttl'); + +SELECT val from test_ttl; + +DROP TABLE test_ttl; From 655a1b58ddf7866a5200668f111a5666cc08aae7 Mon Sep 17 00:00:00 2001 From: discord9 Date: Wed, 4 Dec 2024 15:12:47 +0800 Subject: [PATCH 05/21] refactor: Option None means UNSET --- src/common/base/src/ttl.rs | 16 +---------- src/common/meta/src/ddl/alter_database.rs | 11 +++----- src/common/meta/src/key/schema_name.rs | 33 +++++++++++------------ src/metric-engine/src/engine/alter.rs | 2 +- src/mito2/src/compaction.rs | 18 ++++++++----- src/mito2/src/compaction/compactor.rs | 4 +-- src/mito2/src/compaction/window.rs | 2 +- src/mito2/src/engine/alter_test.rs | 4 +-- src/mito2/src/engine/create_test.rs | 2 +- src/mito2/src/engine/open_test.rs | 2 +- src/mito2/src/region/options.rs | 12 ++++----- src/operator/src/insert.rs | 2 +- src/query/src/sql/show_create_table.rs | 7 +++-- src/store-api/src/region_request.rs | 4 +-- src/table/src/requests.rs | 18 ++++++------- 15 files changed, 62 insertions(+), 75 deletions(-) diff --git a/src/common/base/src/ttl.rs b/src/common/base/src/ttl.rs index e4609bf145e9..d78edabc9f7c 100644 --- a/src/common/base/src/ttl.rs +++ b/src/common/base/src/ttl.rs @@ -19,7 +19,6 @@ use serde::de::Visitor; use serde::{Deserialize, Serialize}; /// Time To Live - #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default)] pub enum TimeToLive { /// immediately throw away on insert @@ -29,8 +28,6 @@ pub enum TimeToLive { /// Keep the data forever #[default] Forever, - // TODO(discord9): add a new variant - // that can't be overridden by database level ttl? call it ForceForever? } impl Serialize for TimeToLive { @@ -100,13 +97,11 @@ impl TimeToLive { } /// Print TimeToLive as string - /// - /// omit `Forever`` variant pub fn as_repr_opt(&self) -> Option { match self { TimeToLive::Immediate => Some("immediate".to_string()), TimeToLive::Duration(d) => Some(humantime::format_duration(*d).to_string()), - TimeToLive::Forever => None, + TimeToLive::Forever => Some("forever".to_string()), } } @@ -138,15 +133,6 @@ impl From for TimeToLive { } } -impl From> for TimeToLive { - fn from(duration: Option) -> Self { - match duration { - Some(d) => TimeToLive::from(d), - None => TimeToLive::Forever, - } - } -} - impl From for TimeToLive { fn from(duration: humantime::Duration) -> Self { Self::from(*duration) diff --git a/src/common/meta/src/ddl/alter_database.rs b/src/common/meta/src/ddl/alter_database.rs index e2cd760e6190..68f0f5428e08 100644 --- a/src/common/meta/src/ddl/alter_database.rs +++ b/src/common/meta/src/ddl/alter_database.rs @@ -13,7 +13,6 @@ // limitations under the License. use async_trait::async_trait; -use common_base::TimeToLive; use common_procedure::error::{FromJsonSnafu, Result as ProcedureResult, ToJsonSnafu}; use common_procedure::{Context as ProcedureContext, LockKey, Procedure, Status}; use common_telemetry::tracing::info; @@ -47,7 +46,7 @@ fn build_new_schema_value( for option in options.0.iter() { match option { SetDatabaseOption::Ttl(ttl) => { - value.ttl = *ttl; + value.ttl = Some(*ttl); } } } @@ -55,7 +54,7 @@ fn build_new_schema_value( AlterDatabaseKind::UnsetDatabaseOptions(keys) => { for key in keys.0.iter() { match key { - UnsetDatabaseOption::Ttl => value.ttl = TimeToLive::default(), + UnsetDatabaseOption::Ttl => value.ttl = None, } } } @@ -217,8 +216,6 @@ impl AlterDatabaseData { mod tests { use std::time::Duration; - use common_base::TimeToLive; - use crate::ddl::alter_database::build_new_schema_value; use crate::key::schema_name::SchemaNameValue; use crate::rpc::ddl::{ @@ -234,7 +231,7 @@ mod tests { let current_schema_value = SchemaNameValue::default(); let new_schema_value = build_new_schema_value(current_schema_value.clone(), &set_ttl).unwrap(); - assert_eq!(new_schema_value.ttl, Duration::from_secs(10).into()); + assert_eq!(new_schema_value.ttl, Some(Duration::from_secs(10).into())); let unset_ttl_alter_kind = AlterDatabaseKind::UnsetDatabaseOptions(UnsetDatabaseOptions(vec![ @@ -242,6 +239,6 @@ mod tests { ])); let new_schema_value = build_new_schema_value(current_schema_value, &unset_ttl_alter_kind).unwrap(); - assert_eq!(new_schema_value.ttl, TimeToLive::default()); + assert_eq!(new_schema_value.ttl, None); } } diff --git a/src/common/meta/src/key/schema_name.rs b/src/common/meta/src/key/schema_name.rs index c21d320478e1..7832458929ab 100644 --- a/src/common/meta/src/key/schema_name.rs +++ b/src/common/meta/src/key/schema_name.rs @@ -57,12 +57,12 @@ impl Default for SchemaNameKey<'_> { #[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)] pub struct SchemaNameValue { #[serde(default)] - pub ttl: TimeToLive, + pub ttl: Option, } impl Display for SchemaNameValue { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - if let Some(ttl) = self.ttl.as_repr_opt() { + if let Some(ttl) = self.ttl.and_then(|i| i.as_repr_opt()) { write!(f, "ttl='{}'", ttl)?; } @@ -86,8 +86,7 @@ impl TryFrom<&HashMap> for SchemaNameValue { }) }) .transpose()? - .map(|ttl| ttl.into()) - .unwrap_or_default(); + .map(|ttl| ttl.into()); Ok(Self { ttl }) } } @@ -95,7 +94,7 @@ impl TryFrom<&HashMap> for SchemaNameValue { impl From for HashMap { fn from(value: SchemaNameValue) -> Self { let mut opts = HashMap::new(); - if let Some(ttl) = value.ttl.as_repr_opt() { + if let Some(ttl) = value.ttl.and_then(|ttl| ttl.as_repr_opt()) { opts.insert(OPT_KEY_TTL.to_string(), ttl); } opts @@ -316,23 +315,21 @@ mod tests { #[test] fn test_display_schema_value() { - let schema_value = SchemaNameValue { - ttl: TimeToLive::default(), - }; + let schema_value = SchemaNameValue { ttl: None }; assert_eq!("", schema_value.to_string()); let schema_value = SchemaNameValue { - ttl: Duration::from_secs(9).into(), + ttl: Some(Duration::from_secs(9).into()), }; assert_eq!("ttl='9s'", schema_value.to_string()); let schema_value = SchemaNameValue { - ttl: Duration::from_secs(0).into(), + ttl: Some(Duration::from_secs(0).into()), }; - assert_eq!("", schema_value.to_string()); + assert_eq!("ttl='forever'", schema_value.to_string()); let schema_value = SchemaNameValue { - ttl: TimeToLive::Immediate, + ttl: Some(TimeToLive::Immediate), }; assert_eq!("ttl='immediate'", schema_value.to_string()); } @@ -347,7 +344,7 @@ mod tests { assert_eq!(key, parsed); let value = SchemaNameValue { - ttl: Duration::from_secs(10).into(), + ttl: Some(Duration::from_secs(10).into()), }; let mut opts: HashMap = HashMap::new(); opts.insert("ttl".to_string(), "10s".to_string()); @@ -361,7 +358,7 @@ mod tests { assert_eq!(Some(value), parsed); let imme = SchemaNameValue { - ttl: TimeToLive::Immediate, + ttl: Some(TimeToLive::Immediate), }; let parsed = SchemaNameValue::try_from_raw_value( serde_json::json!({"ttl": "immediate"}) @@ -372,7 +369,7 @@ mod tests { assert_eq!(Some(imme), parsed); let forever = SchemaNameValue { - ttl: TimeToLive::default(), + ttl: Some(TimeToLive::default()), }; let parsed = SchemaNameValue::try_from_raw_value( serde_json::json!({"ttl": "forever"}).to_string().as_bytes(), @@ -408,7 +405,7 @@ mod tests { let current_schema_value = manager.get(schema_key).await.unwrap().unwrap(); let new_schema_value = SchemaNameValue { - ttl: Duration::from_secs(10).into(), + ttl: Some(Duration::from_secs(10).into()), }; manager .update(schema_key, ¤t_schema_value, &new_schema_value) @@ -422,10 +419,10 @@ mod tests { .unwrap(); let new_schema_value = SchemaNameValue { - ttl: Duration::from_secs(40).into(), + ttl: Some(Duration::from_secs(40).into()), }; let incorrect_schema_value = SchemaNameValue { - ttl: Duration::from_secs(20).into(), + ttl: Some(Duration::from_secs(20).into()), } .try_as_raw_value() .unwrap(); diff --git a/src/metric-engine/src/engine/alter.rs b/src/metric-engine/src/engine/alter.rs index 159b0068a0ab..b05177cf4a17 100644 --- a/src/metric-engine/src/engine/alter.rs +++ b/src/metric-engine/src/engine/alter.rs @@ -207,7 +207,7 @@ mod test { let alter_region_option_request = RegionAlterRequest { schema_version: 0, kind: AlterKind::SetRegionOptions { - options: vec![SetRegionOption::TTL(Duration::from_secs(500).into())], + options: vec![SetRegionOption::TTL(Some(Duration::from_secs(500).into()))], }, }; let result = engine_inner diff --git a/src/mito2/src/compaction.rs b/src/mito2/src/compaction.rs index af61342e2932..554652393dd0 100644 --- a/src/mito2/src/compaction.rs +++ b/src/mito2/src/compaction.rs @@ -292,7 +292,7 @@ impl CompactionScheduler { access_layer: access_layer.clone(), manifest_ctx: manifest_ctx.clone(), file_purger: None, - ttl, + ttl: Some(ttl), }; let picker_output = { @@ -437,11 +437,11 @@ impl PendingCompaction { /// Finds TTL of table by first examine table options then database options. async fn find_ttl( table_id: TableId, - table_ttl: TimeToLive, + table_ttl: Option, schema_metadata_manager: &SchemaMetadataManagerRef, ) -> Result { - // If table TTL is not forever, we use it. - if !table_ttl.is_forever() { + // If table TTL is set, we use it. + if let Some(table_ttl) = table_ttl { return Ok(table_ttl); } @@ -449,7 +449,7 @@ async fn find_ttl( .get_schema_options_by_table_id(table_id) .await .context(GetSchemaMetadataSnafu)? - .map(|options| options.ttl) + .and_then(|options| options.ttl) .unwrap_or_default(); Ok(ttl) } @@ -656,8 +656,12 @@ fn ts_to_lit(ts: Timestamp, ts_col_unit: TimeUnit) -> Result { } /// Finds all expired SSTs across levels. -fn get_expired_ssts(levels: &[LevelMeta], ttl: TimeToLive, now: Timestamp) -> Vec { - let Some(ttl) = ttl.get_duration() else { +fn get_expired_ssts( + levels: &[LevelMeta], + ttl: Option, + now: Timestamp, +) -> Vec { + let Some(ttl) = ttl.and_then(|t| t.get_duration()) else { return vec![]; }; diff --git a/src/mito2/src/compaction/compactor.rs b/src/mito2/src/compaction/compactor.rs index 0fca2c754833..27668846f420 100644 --- a/src/mito2/src/compaction/compactor.rs +++ b/src/mito2/src/compaction/compactor.rs @@ -64,7 +64,7 @@ pub struct CompactionRegion { pub(crate) manifest_ctx: Arc, pub(crate) current_version: VersionRef, pub(crate) file_purger: Option>, - pub(crate) ttl: TimeToLive, + pub(crate) ttl: Option, } /// OpenCompactionRegionRequest represents the request to open a compaction region. @@ -194,7 +194,7 @@ pub async fn open_compaction_region( manifest_ctx, current_version, file_purger: Some(file_purger), - ttl, + ttl: Some(ttl), }) } diff --git a/src/mito2/src/compaction/window.rs b/src/mito2/src/compaction/window.rs index 507130052956..f16b8e4c95d3 100644 --- a/src/mito2/src/compaction/window.rs +++ b/src/mito2/src/compaction/window.rs @@ -253,7 +253,7 @@ mod tests { truncated_entry_id: None, compaction_time_window: None, options: RegionOptions { - ttl: ttl.into(), + ttl: ttl.map(|t| t.into()), compaction: Default::default(), storage: None, append_mode: false, diff --git a/src/mito2/src/engine/alter_test.rs b/src/mito2/src/engine/alter_test.rs index 2841d3e2ed05..ba781f0a8d39 100644 --- a/src/mito2/src/engine/alter_test.rs +++ b/src/mito2/src/engine/alter_test.rs @@ -604,7 +604,7 @@ async fn test_alter_region_ttl_options() { let alter_ttl_request = RegionAlterRequest { schema_version: 0, kind: AlterKind::SetRegionOptions { - options: vec![SetRegionOption::TTL(Duration::from_secs(500).into())], + options: vec![SetRegionOption::TTL(Some(Duration::from_secs(500).into()))], }, }; let alter_job = tokio::spawn(async move { @@ -618,7 +618,7 @@ async fn test_alter_region_ttl_options() { let check_ttl = |engine: &MitoEngine, expected: &Duration| { let current_ttl = engine.get_region(region_id).unwrap().version().options.ttl; - assert_eq!(current_ttl, (*expected).into()); + assert_eq!(current_ttl, Some((*expected).into())); }; // Verify the ttl. check_ttl(&engine, &Duration::from_secs(500)); diff --git a/src/mito2/src/engine/create_test.rs b/src/mito2/src/engine/create_test.rs index b3409a276409..48b04dc86d91 100644 --- a/src/mito2/src/engine/create_test.rs +++ b/src/mito2/src/engine/create_test.rs @@ -166,7 +166,7 @@ async fn test_engine_create_with_options() { let region = engine.get_region(region_id).unwrap(); assert_eq!( region.version().options.ttl, - Duration::from_secs(3600 * 24 * 10).into() + Some(Duration::from_secs(3600 * 24 * 10).into()) ); } diff --git a/src/mito2/src/engine/open_test.rs b/src/mito2/src/engine/open_test.rs index 1964d012beab..6752bbd04b12 100644 --- a/src/mito2/src/engine/open_test.rs +++ b/src/mito2/src/engine/open_test.rs @@ -181,7 +181,7 @@ async fn test_engine_region_open_with_options() { let region = engine.get_region(region_id).unwrap(); assert_eq!( region.version().options.ttl, - Duration::from_secs(3600 * 24 * 4).into() + Some(Duration::from_secs(3600 * 24 * 4).into()) ); } diff --git a/src/mito2/src/region/options.rs b/src/mito2/src/region/options.rs index 390774c50f84..3e9eb62c0629 100644 --- a/src/mito2/src/region/options.rs +++ b/src/mito2/src/region/options.rs @@ -56,7 +56,7 @@ pub enum MergeMode { #[serde(default)] pub struct RegionOptions { /// Region SST files TTL. - pub ttl: TimeToLive, + pub ttl: Option, /// Compaction options. pub compaction: CompactionOptions, /// Custom storage. Uses default storage if it is `None`. @@ -252,7 +252,7 @@ impl Default for TwcsOptions { #[serde(default)] struct RegionOptionsWithoutEnum { /// Region SST files TTL. - ttl: TimeToLive, + ttl: Option, storage: Option, #[serde_as(as = "DisplayFromStr")] append_mode: bool, @@ -457,7 +457,7 @@ mod tests { let map = make_map(&[("ttl", "7d")]); let options = RegionOptions::try_from(&map).unwrap(); let expect = RegionOptions { - ttl: Duration::from_secs(3600 * 24 * 7).into(), + ttl: Some(Duration::from_secs(3600 * 24 * 7).into()), ..Default::default() }; assert_eq!(expect, options); @@ -620,7 +620,7 @@ mod tests { ]); let options = RegionOptions::try_from(&map).unwrap(); let expect = RegionOptions { - ttl: Duration::from_secs(3600 * 24 * 7).into(), + ttl: Some(Duration::from_secs(3600 * 24 * 7).into()), compaction: CompactionOptions::Twcs(TwcsOptions { max_active_window_runs: 8, max_active_window_files: 11, @@ -653,7 +653,7 @@ mod tests { #[test] fn test_region_options_serde() { let options = RegionOptions { - ttl: Duration::from_secs(3600 * 24 * 7).into(), + ttl: Some(Duration::from_secs(3600 * 24 * 7).into()), compaction: CompactionOptions::Twcs(TwcsOptions { max_active_window_runs: 8, max_active_window_files: usize::MAX, @@ -721,7 +721,7 @@ mod tests { }"#; let got: RegionOptions = serde_json::from_str(region_options_json_str).unwrap(); let options = RegionOptions { - ttl: Duration::from_secs(3600 * 24 * 7).into(), + ttl: Some(Duration::from_secs(3600 * 24 * 7).into()), compaction: CompactionOptions::Twcs(TwcsOptions { max_active_window_runs: 8, max_active_window_files: 11, diff --git a/src/operator/src/insert.rs b/src/operator/src/insert.rs index 65ed5678e133..3d58cbd9efbc 100644 --- a/src/operator/src/insert.rs +++ b/src/operator/src/insert.rs @@ -271,7 +271,7 @@ impl Inserter { pub(crate) fn check_ttl_zero_table(ctx: &QueryContextRef, table_info: &TableInfo) { let ttl = table_info.meta.options.ttl; - if ttl.is_immediate() { + if ttl.map(|t| t.is_immediate()).unwrap_or(false) { ctx.add_ttl_zero_regions(table_info.region_ids().into_iter().map(|i| i.as_u64())); } } diff --git a/src/query/src/sql/show_create_table.rs b/src/query/src/sql/show_create_table.rs index 5004c89c9591..e871d95ac390 100644 --- a/src/query/src/sql/show_create_table.rs +++ b/src/query/src/sql/show_create_table.rs @@ -45,9 +45,12 @@ fn create_sql_options(table_meta: &TableMeta, schema_options: Option for ModifyColumnType { #[derive(Debug, Eq, PartialEq, Clone, Serialize, Deserialize)] pub enum SetRegionOption { - TTL(TimeToLive), + TTL(Option), // Modifying TwscOptions with values as (option name, new value). Twsc(String, String), } @@ -761,7 +761,7 @@ impl TryFrom<&PbOption> for SetRegionOption { let ttl = TimeToLive::from_humantime_or_str(value) .map_err(|_| InvalidSetRegionOptionRequestSnafu { key, value }.build())?; - Ok(Self::TTL(ttl)) + Ok(Self::TTL(Some(ttl))) } TWCS_MAX_ACTIVE_WINDOW_RUNS | TWCS_MAX_ACTIVE_WINDOW_FILES diff --git a/src/table/src/requests.rs b/src/table/src/requests.rs index 4c2433f8a017..9aeb28c88175 100644 --- a/src/table/src/requests.rs +++ b/src/table/src/requests.rs @@ -74,7 +74,7 @@ pub struct TableOptions { /// Memtable size of memtable. pub write_buffer_size: Option, /// Time-to-live of table. Expired data will be automatically purged. - pub ttl: TimeToLive, + pub ttl: Option, /// Extra options that may not applicable to all table engines. pub extra_options: HashMap, } @@ -115,7 +115,7 @@ impl TableOptions { } .build() })?; - options.ttl = ttl_value; + options.ttl = Some(ttl_value); } options.extra_options = HashMap::from_iter( @@ -134,7 +134,7 @@ impl fmt::Display for TableOptions { key_vals.push(format!("{}={}", WRITE_BUFFER_SIZE_KEY, size)); } - if let Some(ttl) = self.ttl.as_repr_opt() { + if let Some(ttl) = self.ttl.and_then(|ttl| ttl.as_repr_opt()) { key_vals.push(format!("{}={}", TTL_KEY, ttl)); } @@ -155,7 +155,7 @@ impl From<&TableOptions> for HashMap { write_buffer_size.to_string(), ); } - if let Some(ttl_str) = opts.ttl.as_repr_opt() { + if let Some(ttl_str) = opts.ttl.and_then(|ttl| ttl.as_repr_opt()) { let _ = res.insert(TTL_KEY.to_string(), ttl_str); } res.extend( @@ -340,7 +340,7 @@ mod tests { fn test_serialize_table_options() { let options = TableOptions { write_buffer_size: None, - ttl: Some(Duration::from_secs(1000)).into(), + ttl: Some(Duration::from_secs(1000).into()), extra_options: HashMap::new(), }; let serialized = serde_json::to_string(&options).unwrap(); @@ -352,7 +352,7 @@ mod tests { fn test_convert_hashmap_between_table_options() { let options = TableOptions { write_buffer_size: Some(ReadableSize::mb(128)), - ttl: Some(Duration::from_secs(1000)).into(), + ttl: Some(Duration::from_secs(1000).into()), extra_options: HashMap::new(), }; let serialized_map = HashMap::from(&options); @@ -370,7 +370,7 @@ mod tests { let options = TableOptions { write_buffer_size: Some(ReadableSize::mb(128)), - ttl: Some(Duration::from_secs(1000)).into(), + ttl: Some(Duration::from_secs(1000).into()), extra_options: HashMap::from([("a".to_string(), "A".to_string())]), }; let serialized_map = HashMap::from(&options); @@ -382,7 +382,7 @@ mod tests { fn test_table_options_to_string() { let options = TableOptions { write_buffer_size: Some(ReadableSize::mb(128)), - ttl: Some(Duration::from_secs(1000)).into(), + ttl: Some(Duration::from_secs(1000).into()), extra_options: HashMap::new(), }; @@ -393,7 +393,7 @@ mod tests { let options = TableOptions { write_buffer_size: Some(ReadableSize::mb(128)), - ttl: Some(Duration::from_secs(1000)).into(), + ttl: Some(Duration::from_secs(1000).into()), extra_options: HashMap::from([("a".to_string(), "A".to_string())]), }; From 449c1c0f820692da1bfecdaf206ed0c4bc8d7ed6 Mon Sep 17 00:00:00 2001 From: discord9 Date: Wed, 4 Dec 2024 15:32:17 +0800 Subject: [PATCH 06/21] tests: sqlness --- .../standalone/common/ttl/show_ttl.result | 56 +++++++++++++++++-- .../cases/standalone/common/ttl/show_ttl.sql | 8 +++ 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/tests/cases/standalone/common/ttl/show_ttl.result b/tests/cases/standalone/common/ttl/show_ttl.result index 650c49b2ad7a..1b73c87b3346 100644 --- a/tests/cases/standalone/common/ttl/show_ttl.result +++ b/tests/cases/standalone/common/ttl/show_ttl.result @@ -129,7 +129,7 @@ SHOW CREATE TABLE test_ttl; | | | | | ENGINE=mito | | | WITH( | -| | ttl = '1day' | +| | ttl = 'forever' | | | ) | +----------+-----------------------------------------+ @@ -150,7 +150,7 @@ SHOW CREATE TABLE test_ttl; | | | | | ENGINE=mito | | | WITH( | -| | ttl = '1day' | +| | ttl = 'forever' | | | ) | +----------+-----------------------------------------+ @@ -186,6 +186,17 @@ SHOW CREATE TABLE test_ttl; | | ) | +----------+-----------------------------------------+ +SHOW CREATE DATABASE test_ttl_db; + ++-------------+-------------------------------------------+ +| Database | Create Database | ++-------------+-------------------------------------------+ +| test_ttl_db | CREATE DATABASE IF NOT EXISTS test_ttl_db | +| | WITH( | +| | ttl = '1day' | +| | ) | ++-------------+-------------------------------------------+ + ALTER DATABASE test_ttl_db SET 'ttl' = 'forever'; Affected Rows: 0 @@ -202,7 +213,9 @@ SHOW CREATE TABLE test_ttl; | | ) | | | | | | ENGINE=mito | -| | | +| | WITH( | +| | ttl = 'forever' | +| | ) | +----------+-----------------------------------------+ SHOW CREATE DATABASE test_ttl_db; @@ -211,6 +224,9 @@ SHOW CREATE DATABASE test_ttl_db; | Database | Create Database | +-------------+-------------------------------------------+ | test_ttl_db | CREATE DATABASE IF NOT EXISTS test_ttl_db | +| | WITH( | +| | ttl = 'forever' | +| | ) | +-------------+-------------------------------------------+ ALTER DATABASE test_ttl_db SET 'ttl' = '0s'; @@ -229,7 +245,9 @@ SHOW CREATE TABLE test_ttl; | | ) | | | | | | ENGINE=mito | -| | | +| | WITH( | +| | ttl = 'forever' | +| | ) | +----------+-----------------------------------------+ SHOW CREATE DATABASE test_ttl_db; @@ -238,6 +256,9 @@ SHOW CREATE DATABASE test_ttl_db; | Database | Create Database | +-------------+-------------------------------------------+ | test_ttl_db | CREATE DATABASE IF NOT EXISTS test_ttl_db | +| | WITH( | +| | ttl = 'forever' | +| | ) | +-------------+-------------------------------------------+ ALTER DATABASE test_ttl_db SET 'ttl' = 'immediate'; @@ -299,6 +320,33 @@ SHOW CREATE DATABASE test_ttl_db; | test_ttl_db | CREATE DATABASE IF NOT EXISTS test_ttl_db | +-------------+-------------------------------------------+ +ALTER TABLE test_ttl UNSET 'ttl'; + +Affected Rows: 0 + +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | | ++----------+-----------------------------------------+ + +SHOW CREATE DATABASE test_ttl_db; + ++-------------+-------------------------------------------+ +| Database | Create Database | ++-------------+-------------------------------------------+ +| test_ttl_db | CREATE DATABASE IF NOT EXISTS test_ttl_db | ++-------------+-------------------------------------------+ + DROP TABLE test_ttl; Affected Rows: 0 diff --git a/tests/cases/standalone/common/ttl/show_ttl.sql b/tests/cases/standalone/common/ttl/show_ttl.sql index 83629b820fc4..42f38437e1df 100644 --- a/tests/cases/standalone/common/ttl/show_ttl.sql +++ b/tests/cases/standalone/common/ttl/show_ttl.sql @@ -36,6 +36,8 @@ ALTER TABLE test_ttl UNSET 'ttl'; SHOW CREATE TABLE test_ttl; +SHOW CREATE DATABASE test_ttl_db; + ALTER DATABASE test_ttl_db SET 'ttl' = 'forever'; SHOW CREATE TABLE test_ttl; @@ -60,6 +62,12 @@ SHOW CREATE TABLE test_ttl; SHOW CREATE DATABASE test_ttl_db; +ALTER TABLE test_ttl UNSET 'ttl'; + +SHOW CREATE TABLE test_ttl; + +SHOW CREATE DATABASE test_ttl_db; + DROP TABLE test_ttl; USE public; From 9d8b08daef5ccf87fe78edc9d40802d827224978 Mon Sep 17 00:00:00 2001 From: discord9 Date: Wed, 4 Dec 2024 16:17:26 +0800 Subject: [PATCH 07/21] fix: 10000 years --> forever --- src/common/base/src/ttl.rs | 7 +++++-- src/metric-engine/src/engine/create.rs | 4 ++-- tests/cases/standalone/common/alter/alter_database.result | 3 +++ .../standalone/common/alter/alter_table_options.result | 4 +++- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/common/base/src/ttl.rs b/src/common/base/src/ttl.rs index d78edabc9f7c..8330b26cf852 100644 --- a/src/common/base/src/ttl.rs +++ b/src/common/base/src/ttl.rs @@ -21,7 +21,7 @@ use serde::{Deserialize, Serialize}; /// Time To Live #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default)] pub enum TimeToLive { - /// immediately throw away on insert + /// Immediately throw away on insert Immediate, /// Duration to keep the data, this duration should be non-zero Duration(Duration), @@ -56,6 +56,7 @@ impl<'de> Deserialize<'de> for TimeToLive { formatter.write_str("a string of time, 'immediate', 'forever' or null") } + /// Correctly deserialize null in json fn visit_unit(self) -> Result { Ok(TimeToLive::Forever) } @@ -84,7 +85,9 @@ impl Display for TimeToLive { } impl TimeToLive { - /// Parse a string into TimeToLive + /// Parse a string that is either `immediate`, `forever`, or a duration to `TimeToLive` + /// + /// note that a empty string is treat as `forever` too pub fn from_humantime_or_str(s: &str) -> Result { match s { "immediate" => Ok(TimeToLive::Immediate), diff --git a/src/metric-engine/src/engine/create.rs b/src/metric-engine/src/engine/create.rs index d897640cc529..5cb8b8e2c7a2 100644 --- a/src/metric-engine/src/engine/create.rs +++ b/src/metric-engine/src/engine/create.rs @@ -540,7 +540,7 @@ pub(crate) fn region_options_for_metadata_region( mut original: HashMap, ) -> HashMap { original.remove(APPEND_MODE_KEY); - original.insert(TTL_KEY.to_string(), "10000 years".to_string()); + original.insert(TTL_KEY.to_string(), "forever".to_string()); original } @@ -731,7 +731,7 @@ mod test { ); assert_eq!( metadata_region_request.options.get("ttl").unwrap(), - "10000 years" + "forever" ); } } diff --git a/tests/cases/standalone/common/alter/alter_database.result b/tests/cases/standalone/common/alter/alter_database.result index a98d48323659..8ff458989e4c 100644 --- a/tests/cases/standalone/common/alter/alter_database.result +++ b/tests/cases/standalone/common/alter/alter_database.result @@ -62,6 +62,9 @@ SHOW CREATE DATABASE alter_database; | Database | Create Database | +----------------+----------------------------------------------+ | alter_database | CREATE DATABASE IF NOT EXISTS alter_database | +| | WITH( | +| | ttl = 'forever' | +| | ) | +----------------+----------------------------------------------+ ALTER DATABASE alter_database SET 'ttl'='😁'; diff --git a/tests/cases/standalone/common/alter/alter_table_options.result b/tests/cases/standalone/common/alter/alter_table_options.result index 8fa08eefea6b..b38a99d8465e 100644 --- a/tests/cases/standalone/common/alter/alter_table_options.result +++ b/tests/cases/standalone/common/alter/alter_table_options.result @@ -103,7 +103,9 @@ SHOW CREATE TABLE ato; | | ) | | | | | | ENGINE=mito | -| | | +| | WITH( | +| | ttl = 'forever' | +| | ) | +-------+------------------------------------+ ALTER TABLE ato SET 'ttl'='1s'; From ca6998320fad04c64e9db10eb1cbec93c3df4353 Mon Sep 17 00:00:00 2001 From: discord9 Date: Wed, 4 Dec 2024 19:38:04 +0800 Subject: [PATCH 08/21] chore: minor refactor from reviews --- src/common/base/src/lib.rs | 2 +- src/common/base/src/ttl.rs | 102 ++++++------------ src/common/meta/src/key/schema_name.rs | 19 +++- src/common/meta/src/rpc/ddl.rs | 6 +- src/mito2/src/compaction.rs | 14 +-- src/mito2/src/compaction/compactor.rs | 6 +- src/mito2/src/region/options.rs | 6 +- src/operator/src/insert.rs | 18 +++- .../src/req_convert/insert/stmt_to_region.rs | 4 +- src/session/src/context.rs | 10 +- src/store-api/src/region_request.rs | 6 +- src/table/src/requests.rs | 6 +- 12 files changed, 85 insertions(+), 114 deletions(-) diff --git a/src/common/base/src/lib.rs b/src/common/base/src/lib.rs index e81f819fd233..fa7b5c452162 100644 --- a/src/common/base/src/lib.rs +++ b/src/common/base/src/lib.rs @@ -25,4 +25,4 @@ pub type AffectedRows = usize; pub use bit_vec::BitVec; pub use plugins::Plugins; -pub use ttl::TimeToLive; +pub use ttl::Ttl; diff --git a/src/common/base/src/ttl.rs b/src/common/base/src/ttl.rs index 8330b26cf852..6049e4a1b39f 100644 --- a/src/common/base/src/ttl.rs +++ b/src/common/base/src/ttl.rs @@ -15,86 +15,43 @@ use std::fmt::Display; use std::time::Duration; -use serde::de::Visitor; use serde::{Deserialize, Serialize}; /// Time To Live -#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default)] -pub enum TimeToLive { +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum Ttl { /// Immediately throw away on insert Immediate, - /// Duration to keep the data, this duration should be non-zero - Duration(Duration), /// Keep the data forever #[default] Forever, + /// Duration to keep the data, this duration should be non-zero + #[serde(untagged, with = "humantime_serde")] + Duration(Duration), } -impl Serialize for TimeToLive { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - match self { - Self::Immediate => serializer.serialize_str("immediate"), - Self::Duration(d) => humantime_serde::serialize(d, serializer), - Self::Forever => serializer.serialize_str("forever"), - } - } -} - -impl<'de> Deserialize<'de> for TimeToLive { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - struct StrVisitor; - impl Visitor<'_> for StrVisitor { - type Value = TimeToLive; - - fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { - formatter.write_str("a string of time, 'immediate', 'forever' or null") - } - - /// Correctly deserialize null in json - fn visit_unit(self) -> Result { - Ok(TimeToLive::Forever) - } - - fn visit_str(self, value: &str) -> Result - where - E: serde::de::Error, - { - TimeToLive::from_humantime_or_str(value).map_err(serde::de::Error::custom) - } - } - // deser a string or null - let any = deserializer.deserialize_any(StrVisitor)?; - Ok(any) - } -} - -impl Display for TimeToLive { +impl Display for Ttl { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - TimeToLive::Immediate => write!(f, "immediate"), - TimeToLive::Duration(d) => write!(f, "{}", humantime::Duration::from(*d)), - TimeToLive::Forever => write!(f, "forever"), + Ttl::Immediate => write!(f, "immediate"), + Ttl::Duration(d) => write!(f, "{}", humantime::Duration::from(*d)), + Ttl::Forever => write!(f, "forever"), } } } -impl TimeToLive { +impl Ttl { /// Parse a string that is either `immediate`, `forever`, or a duration to `TimeToLive` /// /// note that a empty string is treat as `forever` too pub fn from_humantime_or_str(s: &str) -> Result { match s { - "immediate" => Ok(TimeToLive::Immediate), - "forever" | "" => Ok(TimeToLive::Forever), + "immediate" => Ok(Ttl::Immediate), + "forever" | "" => Ok(Ttl::Forever), _ => { let d = humantime::parse_duration(s).map_err(|e| e.to_string())?; - Ok(TimeToLive::from(d)) + Ok(Ttl::from(d)) } } } @@ -102,41 +59,41 @@ impl TimeToLive { /// Print TimeToLive as string pub fn as_repr_opt(&self) -> Option { match self { - TimeToLive::Immediate => Some("immediate".to_string()), - TimeToLive::Duration(d) => Some(humantime::format_duration(*d).to_string()), - TimeToLive::Forever => Some("forever".to_string()), + Ttl::Immediate => Some("immediate".to_string()), + Ttl::Duration(d) => Some(humantime::format_duration(*d).to_string()), + Ttl::Forever => Some("forever".to_string()), } } pub fn is_immediate(&self) -> bool { - matches!(self, TimeToLive::Immediate) + matches!(self, Ttl::Immediate) } /// Is the default value, which is `Forever` pub fn is_forever(&self) -> bool { - matches!(self, TimeToLive::Forever) + matches!(self, Ttl::Forever) } pub fn get_duration(&self) -> Option { match self { - TimeToLive::Duration(d) => Some(*d), + Ttl::Duration(d) => Some(*d), _ => None, } } } -impl From for TimeToLive { +impl From for Ttl { fn from(duration: Duration) -> Self { if duration.is_zero() { // compatibility with old code, and inline with cassandra's behavior when ttl set to 0 - TimeToLive::Forever + Ttl::Forever } else { - TimeToLive::Duration(duration) + Ttl::Duration(duration) } } } -impl From for TimeToLive { +impl From for Ttl { fn from(duration: humantime::Duration) -> Self { Self::from(*duration) } @@ -149,22 +106,23 @@ mod test { #[test] fn test_serde() { let cases = vec![ - ("\"immediate\"", TimeToLive::Immediate), - ("\"forever\"", TimeToLive::Forever), + ("\"immediate\"", Ttl::Immediate), + ("\"forever\"", Ttl::Forever), ("\"10d\"", Duration::from_secs(86400 * 10).into()), ( "\"10000 years\"", humantime::parse_duration("10000 years").unwrap().into(), ), - ("null", TimeToLive::Forever), ]; for (s, expected) in cases { let serialized = serde_json::to_string(&expected).unwrap(); - let deserialized: TimeToLive = serde_json::from_str(&serialized).unwrap(); + let deserialized: Ttl = serde_json::from_str(&serialized).unwrap(); assert_eq!(deserialized, expected); - let deserialized: TimeToLive = serde_json::from_str(s).unwrap(); + let deserialized: Ttl = serde_json::from_str(s).unwrap_or_else(|err| { + panic!("Actual serialized: {}, s=`{s}`, err: {:?}", serialized, err) + }); assert_eq!(deserialized, expected); } } diff --git a/src/common/meta/src/key/schema_name.rs b/src/common/meta/src/key/schema_name.rs index 7832458929ab..5d2360caa6fa 100644 --- a/src/common/meta/src/key/schema_name.rs +++ b/src/common/meta/src/key/schema_name.rs @@ -16,7 +16,7 @@ use std::collections::HashMap; use std::fmt::Display; use std::sync::Arc; -use common_base::TimeToLive; +use common_base::Ttl; use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; use futures::stream::BoxStream; use humantime_serde::re::humantime; @@ -57,7 +57,7 @@ impl Default for SchemaNameKey<'_> { #[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)] pub struct SchemaNameValue { #[serde(default)] - pub ttl: Option, + pub ttl: Option, } impl Display for SchemaNameValue { @@ -329,7 +329,7 @@ mod tests { assert_eq!("ttl='forever'", schema_value.to_string()); let schema_value = SchemaNameValue { - ttl: Some(TimeToLive::Immediate), + ttl: Some(Ttl::Immediate), }; assert_eq!("ttl='immediate'", schema_value.to_string()); } @@ -358,7 +358,7 @@ mod tests { assert_eq!(Some(value), parsed); let imme = SchemaNameValue { - ttl: Some(TimeToLive::Immediate), + ttl: Some(Ttl::Immediate), }; let parsed = SchemaNameValue::try_from_raw_value( serde_json::json!({"ttl": "immediate"}) @@ -369,7 +369,7 @@ mod tests { assert_eq!(Some(imme), parsed); let forever = SchemaNameValue { - ttl: Some(TimeToLive::default()), + ttl: Some(Ttl::default()), }; let parsed = SchemaNameValue::try_from_raw_value( serde_json::json!({"ttl": "forever"}).to_string().as_bytes(), @@ -433,5 +433,14 @@ mod tests { .update(schema_key, &incorrect_schema_value, &new_schema_value) .await .unwrap_err(); + + let new_schema_value = SchemaNameValue { ttl: None }; + manager + .update(schema_key, ¤t_schema_value, &new_schema_value) + .await + .unwrap(); + + let current_schema_value = manager.get(schema_key).await.unwrap().unwrap(); + assert_eq!(new_schema_value, *current_schema_value); } } diff --git a/src/common/meta/src/rpc/ddl.rs b/src/common/meta/src/rpc/ddl.rs index 03828a632e25..39145990686d 100644 --- a/src/common/meta/src/rpc/ddl.rs +++ b/src/common/meta/src/rpc/ddl.rs @@ -35,7 +35,7 @@ use api::v1::{ }; use base64::engine::general_purpose; use base64::Engine as _; -use common_base::TimeToLive; +use common_base::Ttl; use prost::Message; use serde::{Deserialize, Serialize}; use serde_with::{serde_as, DefaultOnNull}; @@ -1008,7 +1008,7 @@ impl TryFrom for SetDatabaseOption { fn try_from(PbOption { key, value }: PbOption) -> Result { match key.to_ascii_lowercase().as_str() { TTL_KEY => { - let ttl = TimeToLive::from_humantime_or_str(&value) + let ttl = Ttl::from_humantime_or_str(&value) .map_err(|_| InvalidSetDatabaseOptionSnafu { key, value }.build())?; Ok(SetDatabaseOption::Ttl(ttl)) @@ -1020,7 +1020,7 @@ impl TryFrom for SetDatabaseOption { #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] pub enum SetDatabaseOption { - Ttl(TimeToLive), + Ttl(Ttl), } #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] diff --git a/src/mito2/src/compaction.rs b/src/mito2/src/compaction.rs index 554652393dd0..18fe925f37bf 100644 --- a/src/mito2/src/compaction.rs +++ b/src/mito2/src/compaction.rs @@ -27,7 +27,7 @@ use std::sync::Arc; use std::time::Instant; use api::v1::region::compact_request; -use common_base::{Plugins, TimeToLive}; +use common_base::{Plugins, Ttl}; use common_meta::key::SchemaMetadataManagerRef; use common_telemetry::{debug, error, info, warn}; use common_time::range::TimestampRange; @@ -273,7 +273,7 @@ impl CompactionScheduler { .await .unwrap_or_else(|e| { warn!(e; "Failed to get ttl for region: {}", region_id); - TimeToLive::default() + Ttl::default() }); debug!( @@ -437,9 +437,9 @@ impl PendingCompaction { /// Finds TTL of table by first examine table options then database options. async fn find_ttl( table_id: TableId, - table_ttl: Option, + table_ttl: Option, schema_metadata_manager: &SchemaMetadataManagerRef, -) -> Result { +) -> Result { // If table TTL is set, we use it. if let Some(table_ttl) = table_ttl { return Ok(table_ttl); @@ -656,11 +656,7 @@ fn ts_to_lit(ts: Timestamp, ts_col_unit: TimeUnit) -> Result { } /// Finds all expired SSTs across levels. -fn get_expired_ssts( - levels: &[LevelMeta], - ttl: Option, - now: Timestamp, -) -> Vec { +fn get_expired_ssts(levels: &[LevelMeta], ttl: Option, now: Timestamp) -> Vec { let Some(ttl) = ttl.and_then(|t| t.get_duration()) else { return vec![]; }; diff --git a/src/mito2/src/compaction/compactor.rs b/src/mito2/src/compaction/compactor.rs index 27668846f420..0b97b0e0ede4 100644 --- a/src/mito2/src/compaction/compactor.rs +++ b/src/mito2/src/compaction/compactor.rs @@ -16,7 +16,7 @@ use std::sync::Arc; use std::time::Duration; use api::v1::region::compact_request; -use common_base::TimeToLive; +use common_base::Ttl; use common_meta::key::SchemaMetadataManagerRef; use common_telemetry::{info, warn}; use object_store::manager::ObjectStoreManagerRef; @@ -64,7 +64,7 @@ pub struct CompactionRegion { pub(crate) manifest_ctx: Arc, pub(crate) current_version: VersionRef, pub(crate) file_purger: Option>, - pub(crate) ttl: Option, + pub(crate) ttl: Option, } /// OpenCompactionRegionRequest represents the request to open a compaction region. @@ -181,7 +181,7 @@ pub async fn open_compaction_region( .await .unwrap_or_else(|e| { warn!(e; "Failed to get ttl for region: {}", region_metadata.region_id); - TimeToLive::default() + Ttl::default() }); Ok(CompactionRegion { region_id: req.region_id, diff --git a/src/mito2/src/region/options.rs b/src/mito2/src/region/options.rs index 3e9eb62c0629..74cd961c2dfb 100644 --- a/src/mito2/src/region/options.rs +++ b/src/mito2/src/region/options.rs @@ -20,7 +20,7 @@ use std::collections::HashMap; use std::time::Duration; use common_base::readable_size::ReadableSize; -use common_base::TimeToLive; +use common_base::Ttl; use common_wal::options::{WalOptions, WAL_OPTIONS_KEY}; use serde::de::Error as _; use serde::{Deserialize, Deserializer, Serialize}; @@ -56,7 +56,7 @@ pub enum MergeMode { #[serde(default)] pub struct RegionOptions { /// Region SST files TTL. - pub ttl: Option, + pub ttl: Option, /// Compaction options. pub compaction: CompactionOptions, /// Custom storage. Uses default storage if it is `None`. @@ -252,7 +252,7 @@ impl Default for TwcsOptions { #[serde(default)] struct RegionOptionsWithoutEnum { /// Region SST files TTL. - ttl: Option, + ttl: Option, storage: Option, #[serde_as(as = "DisplayFromStr")] append_mode: bool, diff --git a/src/operator/src/insert.rs b/src/operator/src/insert.rs index 3d58cbd9efbc..1ae31e5645ae 100644 --- a/src/operator/src/insert.rs +++ b/src/operator/src/insert.rs @@ -92,6 +92,14 @@ impl AutoCreateTableType { } } +#[derive(Clone)] +pub struct InsertRequestsWithImmeTtl { + /// Requests with normal ttl. + pub normal_requests: InsertRequests, + /// Requests with ttl=immediate. + pub imme_requests: InsertRequests, +} + impl Inserter { pub fn new( catalog_manager: CatalogManagerRef, @@ -244,7 +252,7 @@ impl Inserter { table_name: common_catalog::format_full_table_name(catalog, schema, table_name), })?; let table_info = table.table_info(); - check_ttl_zero_table(&ctx, &table_info); + add_ttl_imme_region(&ctx, &table_info); let inserts = TableToRegion::new(&table_info, &self.partition_manager) .convert(request) @@ -268,11 +276,11 @@ impl Inserter { } /// Check and add regions with ttl=0 to context -pub(crate) fn check_ttl_zero_table(ctx: &QueryContextRef, table_info: &TableInfo) { +pub(crate) fn add_ttl_imme_region(ctx: &QueryContextRef, table_info: &TableInfo) { let ttl = table_info.meta.options.ttl; if ttl.map(|t| t.is_immediate()).unwrap_or(false) { - ctx.add_ttl_zero_regions(table_info.region_ids().into_iter().map(|i| i.as_u64())); + ctx.add_ttl_imme_regions(table_info.region_ids().into_iter().map(|i| i.as_u64())); } } @@ -510,7 +518,7 @@ impl Inserter { ), })?; let table_info = table.table_info(); - check_ttl_zero_table(ctx, &table_info); + add_ttl_imme_region(ctx, &table_info); table_name_to_ids.insert(table_info.name.clone(), table_info.table_id()); } return Ok(table_name_to_ids); @@ -583,7 +591,7 @@ impl Inserter { continue; }; let table_info = table.table_info(); - check_ttl_zero_table(ctx, &table_info); + add_ttl_imme_region(ctx, &table_info); } Ok(table_name_to_ids) diff --git a/src/operator/src/req_convert/insert/stmt_to_region.rs b/src/operator/src/req_convert/insert/stmt_to_region.rs index 028e6e025e06..d11126d022b9 100644 --- a/src/operator/src/req_convert/insert/stmt_to_region.rs +++ b/src/operator/src/req_convert/insert/stmt_to_region.rs @@ -32,7 +32,7 @@ use crate::error::{ ColumnNotFoundSnafu, InvalidSqlSnafu, MissingInsertBodySnafu, ParseSqlSnafu, Result, SchemaReadOnlySnafu, TableNotFoundSnafu, }; -use crate::insert::check_ttl_zero_table; +use crate::insert::add_ttl_imme_region; use crate::req_convert::common::partitioner::Partitioner; use crate::req_convert::insert::semantic_type; @@ -66,7 +66,7 @@ impl<'a> StatementToRegion<'a> { let table = self.get_table(&catalog, &schema, &table_name).await?; let table_schema = table.schema(); let table_info = table.table_info(); - check_ttl_zero_table(query_ctx, &table_info); + add_ttl_imme_region(query_ctx, &table_info); ensure!( !common_catalog::consts::is_readonly_schema(&schema), diff --git a/src/session/src/context.rs b/src/session/src/context.rs index 6171479f4d23..385638873841 100644 --- a/src/session/src/context.rs +++ b/src/session/src/context.rs @@ -60,8 +60,8 @@ pub struct QueryContext { #[derive(Debug, Builder, Clone, Default)] pub struct QueryContextMutableFields { warning: Option, - /// regions with ttl=0 - ttl_zero_regions: HashSet, + /// regions with ttl=immediate + ttl_imme_regions: HashSet, } impl Display for QueryContext { @@ -287,11 +287,11 @@ impl QueryContext { self.mutable_query_context_data.write().unwrap().warning = Some(msg); } - pub fn add_ttl_zero_regions(&self, region_ids: impl Iterator) { + pub fn add_ttl_imme_regions(&self, region_ids: impl Iterator) { self.mutable_query_context_data .write() .unwrap() - .ttl_zero_regions + .ttl_imme_regions .extend(region_ids); } @@ -299,7 +299,7 @@ impl QueryContext { self.mutable_query_context_data .read() .unwrap() - .ttl_zero_regions + .ttl_imme_regions .clone() } diff --git a/src/store-api/src/region_request.rs b/src/store-api/src/region_request.rs index de2d21773d66..61f825bff241 100644 --- a/src/store-api/src/region_request.rs +++ b/src/store-api/src/region_request.rs @@ -25,7 +25,7 @@ use api::v1::region::{ }; use api::v1::{self, Analyzer, Option as PbOption, Rows, SemanticType}; pub use common_base::AffectedRows; -use common_base::TimeToLive; +use common_base::Ttl; use datatypes::data_type::ConcreteDataType; use datatypes::schema::FulltextOptions; use serde::{Deserialize, Serialize}; @@ -746,7 +746,7 @@ impl From for ModifyColumnType { #[derive(Debug, Eq, PartialEq, Clone, Serialize, Deserialize)] pub enum SetRegionOption { - TTL(Option), + TTL(Option), // Modifying TwscOptions with values as (option name, new value). Twsc(String, String), } @@ -758,7 +758,7 @@ impl TryFrom<&PbOption> for SetRegionOption { let PbOption { key, value } = value; match key.as_str() { TTL_KEY => { - let ttl = TimeToLive::from_humantime_or_str(value) + let ttl = Ttl::from_humantime_or_str(value) .map_err(|_| InvalidSetRegionOptionRequestSnafu { key, value }.build())?; Ok(Self::TTL(Some(ttl))) diff --git a/src/table/src/requests.rs b/src/table/src/requests.rs index 9aeb28c88175..a252ec1ecfe3 100644 --- a/src/table/src/requests.rs +++ b/src/table/src/requests.rs @@ -19,7 +19,7 @@ use std::fmt; use std::str::FromStr; use common_base::readable_size::ReadableSize; -use common_base::TimeToLive; +use common_base::Ttl; use common_datasource::object_store::s3::is_supported_in_s3; use common_query::AddColumnLocation; use common_time::range::TimestampRange; @@ -74,7 +74,7 @@ pub struct TableOptions { /// Memtable size of memtable. pub write_buffer_size: Option, /// Time-to-live of table. Expired data will be automatically purged. - pub ttl: Option, + pub ttl: Option, /// Extra options that may not applicable to all table engines. pub extra_options: HashMap, } @@ -108,7 +108,7 @@ impl TableOptions { } if let Some(ttl) = kvs.get(TTL_KEY) { - let ttl_value = TimeToLive::from_humantime_or_str(ttl).map_err(|_| { + let ttl_value = Ttl::from_humantime_or_str(ttl).map_err(|_| { ParseTableOptionSnafu { key: TTL_KEY, value: ttl, From 5f185f1fb698c054dabe1c380932fbad52eb1043 Mon Sep 17 00:00:00 2001 From: discord9 Date: Wed, 4 Dec 2024 20:03:43 +0800 Subject: [PATCH 09/21] chore: rename back TimeToLive --- src/common/base/src/lib.rs | 2 +- src/common/base/src/ttl.rs | 46 +++++++++++++------------- src/common/meta/src/key/schema_name.rs | 10 +++--- src/common/meta/src/rpc/ddl.rs | 6 ++-- src/metric-engine/src/engine/alter.rs | 2 +- src/mito2/src/compaction.rs | 14 +++++--- src/mito2/src/compaction/compactor.rs | 6 ++-- src/mito2/src/engine/alter_test.rs | 2 +- src/mito2/src/region/options.rs | 6 ++-- src/mito2/src/worker/handle_alter.rs | 2 +- src/store-api/src/region_request.rs | 10 +++--- src/table/src/metadata.rs | 2 +- src/table/src/requests.rs | 6 ++-- 13 files changed, 59 insertions(+), 55 deletions(-) diff --git a/src/common/base/src/lib.rs b/src/common/base/src/lib.rs index fa7b5c452162..e81f819fd233 100644 --- a/src/common/base/src/lib.rs +++ b/src/common/base/src/lib.rs @@ -25,4 +25,4 @@ pub type AffectedRows = usize; pub use bit_vec::BitVec; pub use plugins::Plugins; -pub use ttl::Ttl; +pub use ttl::TimeToLive; diff --git a/src/common/base/src/ttl.rs b/src/common/base/src/ttl.rs index 6049e4a1b39f..6eff65418ec5 100644 --- a/src/common/base/src/ttl.rs +++ b/src/common/base/src/ttl.rs @@ -20,7 +20,7 @@ use serde::{Deserialize, Serialize}; /// Time To Live #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] -pub enum Ttl { +pub enum TimeToLive { /// Immediately throw away on insert Immediate, /// Keep the data forever @@ -31,27 +31,27 @@ pub enum Ttl { Duration(Duration), } -impl Display for Ttl { +impl Display for TimeToLive { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Ttl::Immediate => write!(f, "immediate"), - Ttl::Duration(d) => write!(f, "{}", humantime::Duration::from(*d)), - Ttl::Forever => write!(f, "forever"), + TimeToLive::Immediate => write!(f, "immediate"), + TimeToLive::Duration(d) => write!(f, "{}", humantime::Duration::from(*d)), + TimeToLive::Forever => write!(f, "forever"), } } } -impl Ttl { +impl TimeToLive { /// Parse a string that is either `immediate`, `forever`, or a duration to `TimeToLive` /// /// note that a empty string is treat as `forever` too pub fn from_humantime_or_str(s: &str) -> Result { match s { - "immediate" => Ok(Ttl::Immediate), - "forever" | "" => Ok(Ttl::Forever), + "immediate" => Ok(TimeToLive::Immediate), + "forever" | "" => Ok(TimeToLive::Forever), _ => { let d = humantime::parse_duration(s).map_err(|e| e.to_string())?; - Ok(Ttl::from(d)) + Ok(TimeToLive::from(d)) } } } @@ -59,41 +59,41 @@ impl Ttl { /// Print TimeToLive as string pub fn as_repr_opt(&self) -> Option { match self { - Ttl::Immediate => Some("immediate".to_string()), - Ttl::Duration(d) => Some(humantime::format_duration(*d).to_string()), - Ttl::Forever => Some("forever".to_string()), + TimeToLive::Immediate => Some("immediate".to_string()), + TimeToLive::Duration(d) => Some(humantime::format_duration(*d).to_string()), + TimeToLive::Forever => Some("forever".to_string()), } } pub fn is_immediate(&self) -> bool { - matches!(self, Ttl::Immediate) + matches!(self, TimeToLive::Immediate) } /// Is the default value, which is `Forever` pub fn is_forever(&self) -> bool { - matches!(self, Ttl::Forever) + matches!(self, TimeToLive::Forever) } pub fn get_duration(&self) -> Option { match self { - Ttl::Duration(d) => Some(*d), + TimeToLive::Duration(d) => Some(*d), _ => None, } } } -impl From for Ttl { +impl From for TimeToLive { fn from(duration: Duration) -> Self { if duration.is_zero() { // compatibility with old code, and inline with cassandra's behavior when ttl set to 0 - Ttl::Forever + TimeToLive::Forever } else { - Ttl::Duration(duration) + TimeToLive::Duration(duration) } } } -impl From for Ttl { +impl From for TimeToLive { fn from(duration: humantime::Duration) -> Self { Self::from(*duration) } @@ -106,8 +106,8 @@ mod test { #[test] fn test_serde() { let cases = vec![ - ("\"immediate\"", Ttl::Immediate), - ("\"forever\"", Ttl::Forever), + ("\"immediate\"", TimeToLive::Immediate), + ("\"forever\"", TimeToLive::Forever), ("\"10d\"", Duration::from_secs(86400 * 10).into()), ( "\"10000 years\"", @@ -117,10 +117,10 @@ mod test { for (s, expected) in cases { let serialized = serde_json::to_string(&expected).unwrap(); - let deserialized: Ttl = serde_json::from_str(&serialized).unwrap(); + let deserialized: TimeToLive = serde_json::from_str(&serialized).unwrap(); assert_eq!(deserialized, expected); - let deserialized: Ttl = serde_json::from_str(s).unwrap_or_else(|err| { + let deserialized: TimeToLive = serde_json::from_str(s).unwrap_or_else(|err| { panic!("Actual serialized: {}, s=`{s}`, err: {:?}", serialized, err) }); assert_eq!(deserialized, expected); diff --git a/src/common/meta/src/key/schema_name.rs b/src/common/meta/src/key/schema_name.rs index 5d2360caa6fa..edcd3f914ce0 100644 --- a/src/common/meta/src/key/schema_name.rs +++ b/src/common/meta/src/key/schema_name.rs @@ -16,7 +16,7 @@ use std::collections::HashMap; use std::fmt::Display; use std::sync::Arc; -use common_base::Ttl; +use common_base::TimeToLive; use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; use futures::stream::BoxStream; use humantime_serde::re::humantime; @@ -57,7 +57,7 @@ impl Default for SchemaNameKey<'_> { #[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)] pub struct SchemaNameValue { #[serde(default)] - pub ttl: Option, + pub ttl: Option, } impl Display for SchemaNameValue { @@ -329,7 +329,7 @@ mod tests { assert_eq!("ttl='forever'", schema_value.to_string()); let schema_value = SchemaNameValue { - ttl: Some(Ttl::Immediate), + ttl: Some(TimeToLive::Immediate), }; assert_eq!("ttl='immediate'", schema_value.to_string()); } @@ -358,7 +358,7 @@ mod tests { assert_eq!(Some(value), parsed); let imme = SchemaNameValue { - ttl: Some(Ttl::Immediate), + ttl: Some(TimeToLive::Immediate), }; let parsed = SchemaNameValue::try_from_raw_value( serde_json::json!({"ttl": "immediate"}) @@ -369,7 +369,7 @@ mod tests { assert_eq!(Some(imme), parsed); let forever = SchemaNameValue { - ttl: Some(Ttl::default()), + ttl: Some(TimeToLive::default()), }; let parsed = SchemaNameValue::try_from_raw_value( serde_json::json!({"ttl": "forever"}).to_string().as_bytes(), diff --git a/src/common/meta/src/rpc/ddl.rs b/src/common/meta/src/rpc/ddl.rs index 39145990686d..03828a632e25 100644 --- a/src/common/meta/src/rpc/ddl.rs +++ b/src/common/meta/src/rpc/ddl.rs @@ -35,7 +35,7 @@ use api::v1::{ }; use base64::engine::general_purpose; use base64::Engine as _; -use common_base::Ttl; +use common_base::TimeToLive; use prost::Message; use serde::{Deserialize, Serialize}; use serde_with::{serde_as, DefaultOnNull}; @@ -1008,7 +1008,7 @@ impl TryFrom for SetDatabaseOption { fn try_from(PbOption { key, value }: PbOption) -> Result { match key.to_ascii_lowercase().as_str() { TTL_KEY => { - let ttl = Ttl::from_humantime_or_str(&value) + let ttl = TimeToLive::from_humantime_or_str(&value) .map_err(|_| InvalidSetDatabaseOptionSnafu { key, value }.build())?; Ok(SetDatabaseOption::Ttl(ttl)) @@ -1020,7 +1020,7 @@ impl TryFrom for SetDatabaseOption { #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] pub enum SetDatabaseOption { - Ttl(Ttl), + Ttl(TimeToLive), } #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] diff --git a/src/metric-engine/src/engine/alter.rs b/src/metric-engine/src/engine/alter.rs index b05177cf4a17..0a961d498833 100644 --- a/src/metric-engine/src/engine/alter.rs +++ b/src/metric-engine/src/engine/alter.rs @@ -207,7 +207,7 @@ mod test { let alter_region_option_request = RegionAlterRequest { schema_version: 0, kind: AlterKind::SetRegionOptions { - options: vec![SetRegionOption::TTL(Some(Duration::from_secs(500).into()))], + options: vec![SetRegionOption::Ttl(Some(Duration::from_secs(500).into()))], }, }; let result = engine_inner diff --git a/src/mito2/src/compaction.rs b/src/mito2/src/compaction.rs index 18fe925f37bf..554652393dd0 100644 --- a/src/mito2/src/compaction.rs +++ b/src/mito2/src/compaction.rs @@ -27,7 +27,7 @@ use std::sync::Arc; use std::time::Instant; use api::v1::region::compact_request; -use common_base::{Plugins, Ttl}; +use common_base::{Plugins, TimeToLive}; use common_meta::key::SchemaMetadataManagerRef; use common_telemetry::{debug, error, info, warn}; use common_time::range::TimestampRange; @@ -273,7 +273,7 @@ impl CompactionScheduler { .await .unwrap_or_else(|e| { warn!(e; "Failed to get ttl for region: {}", region_id); - Ttl::default() + TimeToLive::default() }); debug!( @@ -437,9 +437,9 @@ impl PendingCompaction { /// Finds TTL of table by first examine table options then database options. async fn find_ttl( table_id: TableId, - table_ttl: Option, + table_ttl: Option, schema_metadata_manager: &SchemaMetadataManagerRef, -) -> Result { +) -> Result { // If table TTL is set, we use it. if let Some(table_ttl) = table_ttl { return Ok(table_ttl); @@ -656,7 +656,11 @@ fn ts_to_lit(ts: Timestamp, ts_col_unit: TimeUnit) -> Result { } /// Finds all expired SSTs across levels. -fn get_expired_ssts(levels: &[LevelMeta], ttl: Option, now: Timestamp) -> Vec { +fn get_expired_ssts( + levels: &[LevelMeta], + ttl: Option, + now: Timestamp, +) -> Vec { let Some(ttl) = ttl.and_then(|t| t.get_duration()) else { return vec![]; }; diff --git a/src/mito2/src/compaction/compactor.rs b/src/mito2/src/compaction/compactor.rs index 0b97b0e0ede4..27668846f420 100644 --- a/src/mito2/src/compaction/compactor.rs +++ b/src/mito2/src/compaction/compactor.rs @@ -16,7 +16,7 @@ use std::sync::Arc; use std::time::Duration; use api::v1::region::compact_request; -use common_base::Ttl; +use common_base::TimeToLive; use common_meta::key::SchemaMetadataManagerRef; use common_telemetry::{info, warn}; use object_store::manager::ObjectStoreManagerRef; @@ -64,7 +64,7 @@ pub struct CompactionRegion { pub(crate) manifest_ctx: Arc, pub(crate) current_version: VersionRef, pub(crate) file_purger: Option>, - pub(crate) ttl: Option, + pub(crate) ttl: Option, } /// OpenCompactionRegionRequest represents the request to open a compaction region. @@ -181,7 +181,7 @@ pub async fn open_compaction_region( .await .unwrap_or_else(|e| { warn!(e; "Failed to get ttl for region: {}", region_metadata.region_id); - Ttl::default() + TimeToLive::default() }); Ok(CompactionRegion { region_id: req.region_id, diff --git a/src/mito2/src/engine/alter_test.rs b/src/mito2/src/engine/alter_test.rs index ba781f0a8d39..b774dd8a05cf 100644 --- a/src/mito2/src/engine/alter_test.rs +++ b/src/mito2/src/engine/alter_test.rs @@ -604,7 +604,7 @@ async fn test_alter_region_ttl_options() { let alter_ttl_request = RegionAlterRequest { schema_version: 0, kind: AlterKind::SetRegionOptions { - options: vec![SetRegionOption::TTL(Some(Duration::from_secs(500).into()))], + options: vec![SetRegionOption::Ttl(Some(Duration::from_secs(500).into()))], }, }; let alter_job = tokio::spawn(async move { diff --git a/src/mito2/src/region/options.rs b/src/mito2/src/region/options.rs index 74cd961c2dfb..3e9eb62c0629 100644 --- a/src/mito2/src/region/options.rs +++ b/src/mito2/src/region/options.rs @@ -20,7 +20,7 @@ use std::collections::HashMap; use std::time::Duration; use common_base::readable_size::ReadableSize; -use common_base::Ttl; +use common_base::TimeToLive; use common_wal::options::{WalOptions, WAL_OPTIONS_KEY}; use serde::de::Error as _; use serde::{Deserialize, Deserializer, Serialize}; @@ -56,7 +56,7 @@ pub enum MergeMode { #[serde(default)] pub struct RegionOptions { /// Region SST files TTL. - pub ttl: Option, + pub ttl: Option, /// Compaction options. pub compaction: CompactionOptions, /// Custom storage. Uses default storage if it is `None`. @@ -252,7 +252,7 @@ impl Default for TwcsOptions { #[serde(default)] struct RegionOptionsWithoutEnum { /// Region SST files TTL. - ttl: Option, + ttl: Option, storage: Option, #[serde_as(as = "DisplayFromStr")] append_mode: bool, diff --git a/src/mito2/src/worker/handle_alter.rs b/src/mito2/src/worker/handle_alter.rs index c5c45f2d440d..10d87e2940c2 100644 --- a/src/mito2/src/worker/handle_alter.rs +++ b/src/mito2/src/worker/handle_alter.rs @@ -184,7 +184,7 @@ impl RegionWorkerLoop { let mut current_options = version.options.clone(); for option in options { match option { - SetRegionOption::TTL(new_ttl) => { + SetRegionOption::Ttl(new_ttl) => { info!( "Update region ttl: {}, previous: {:?} new: {:?}", region.region_id, current_options.ttl, new_ttl diff --git a/src/store-api/src/region_request.rs b/src/store-api/src/region_request.rs index 61f825bff241..d91d431ffbb5 100644 --- a/src/store-api/src/region_request.rs +++ b/src/store-api/src/region_request.rs @@ -25,7 +25,7 @@ use api::v1::region::{ }; use api::v1::{self, Analyzer, Option as PbOption, Rows, SemanticType}; pub use common_base::AffectedRows; -use common_base::Ttl; +use common_base::TimeToLive; use datatypes::data_type::ConcreteDataType; use datatypes::schema::FulltextOptions; use serde::{Deserialize, Serialize}; @@ -746,7 +746,7 @@ impl From for ModifyColumnType { #[derive(Debug, Eq, PartialEq, Clone, Serialize, Deserialize)] pub enum SetRegionOption { - TTL(Option), + Ttl(Option), // Modifying TwscOptions with values as (option name, new value). Twsc(String, String), } @@ -758,10 +758,10 @@ impl TryFrom<&PbOption> for SetRegionOption { let PbOption { key, value } = value; match key.as_str() { TTL_KEY => { - let ttl = Ttl::from_humantime_or_str(value) + let ttl = TimeToLive::from_humantime_or_str(value) .map_err(|_| InvalidSetRegionOptionRequestSnafu { key, value }.build())?; - Ok(Self::TTL(Some(ttl))) + Ok(Self::Ttl(Some(ttl))) } TWCS_MAX_ACTIVE_WINDOW_RUNS | TWCS_MAX_ACTIVE_WINDOW_FILES @@ -795,7 +795,7 @@ impl From<&UnsetRegionOption> for SetRegionOption { UnsetRegionOption::TwcsTimeWindow => { SetRegionOption::Twsc(unset_option.to_string(), String::new()) } - UnsetRegionOption::Ttl => SetRegionOption::TTL(Default::default()), + UnsetRegionOption::Ttl => SetRegionOption::Ttl(Default::default()), } } } diff --git a/src/table/src/metadata.rs b/src/table/src/metadata.rs index 36873c9d737f..a3936f5ac036 100644 --- a/src/table/src/metadata.rs +++ b/src/table/src/metadata.rs @@ -224,7 +224,7 @@ impl TableMeta { for request in requests { match request { - SetRegionOption::TTL(new_ttl) => { + SetRegionOption::Ttl(new_ttl) => { new_options.ttl = *new_ttl; } SetRegionOption::Twsc(key, value) => { diff --git a/src/table/src/requests.rs b/src/table/src/requests.rs index a252ec1ecfe3..9aeb28c88175 100644 --- a/src/table/src/requests.rs +++ b/src/table/src/requests.rs @@ -19,7 +19,7 @@ use std::fmt; use std::str::FromStr; use common_base::readable_size::ReadableSize; -use common_base::Ttl; +use common_base::TimeToLive; use common_datasource::object_store::s3::is_supported_in_s3; use common_query::AddColumnLocation; use common_time::range::TimestampRange; @@ -74,7 +74,7 @@ pub struct TableOptions { /// Memtable size of memtable. pub write_buffer_size: Option, /// Time-to-live of table. Expired data will be automatically purged. - pub ttl: Option, + pub ttl: Option, /// Extra options that may not applicable to all table engines. pub extra_options: HashMap, } @@ -108,7 +108,7 @@ impl TableOptions { } if let Some(ttl) = kvs.get(TTL_KEY) { - let ttl_value = Ttl::from_humantime_or_str(ttl).map_err(|_| { + let ttl_value = TimeToLive::from_humantime_or_str(ttl).map_err(|_| { ParseTableOptionSnafu { key: TTL_KEY, value: ttl, From bbbfc1f36e321a0ebb169a67fc423365ba81afcb Mon Sep 17 00:00:00 2001 From: discord9 Date: Wed, 4 Dec 2024 20:37:42 +0800 Subject: [PATCH 10/21] refactor: split imme request from normal requests --- Cargo.lock | 1 + src/common/meta/src/key/schema_name.rs | 1 + src/operator/Cargo.toml | 1 + src/operator/src/insert.rs | 83 +++++++++++-------- .../src/req_convert/insert/row_to_region.rs | 26 ++++-- .../src/req_convert/insert/stmt_to_region.rs | 18 +++- .../src/req_convert/insert/table_to_region.rs | 18 +++- src/session/src/context.rs | 19 ----- 8 files changed, 101 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0590893e4dca..e98a422a59db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7593,6 +7593,7 @@ dependencies = [ name = "operator" version = "0.11.0" dependencies = [ + "ahash 0.8.11", "api", "async-stream", "async-trait", diff --git a/src/common/meta/src/key/schema_name.rs b/src/common/meta/src/key/schema_name.rs index edcd3f914ce0..972e9208670f 100644 --- a/src/common/meta/src/key/schema_name.rs +++ b/src/common/meta/src/key/schema_name.rs @@ -434,6 +434,7 @@ mod tests { .await .unwrap_err(); + let current_schema_value = manager.get(schema_key).await.unwrap().unwrap(); let new_schema_value = SchemaNameValue { ttl: None }; manager .update(schema_key, ¤t_schema_value, &new_schema_value) diff --git a/src/operator/Cargo.toml b/src/operator/Cargo.toml index d20034155f1b..cd26458fad68 100644 --- a/src/operator/Cargo.toml +++ b/src/operator/Cargo.toml @@ -11,6 +11,7 @@ testing = [] workspace = true [dependencies] +ahash.workspace = true api.workspace = true async-stream.workspace = true async-trait = "0.1" diff --git a/src/operator/src/insert.rs b/src/operator/src/insert.rs index 1ae31e5645ae..0f64171e93d5 100644 --- a/src/operator/src/insert.rs +++ b/src/operator/src/insert.rs @@ -12,11 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::HashMap; use std::sync::Arc; +use ahash::{HashMap, HashMapExt, HashSet, HashSetExt}; use api::v1::alter_table_expr::Kind; -use api::v1::region::{InsertRequests as RegionInsertRequests, RegionRequestHeader}; +use api::v1::region::{ + InsertRequest as RegionInsertRequest, InsertRequests as RegionInsertRequests, + RegionRequestHeader, +}; use api::v1::{ AlterTableExpr, ColumnDataType, ColumnSchema, CreateTableExpr, InsertRequests, RowInsertRequest, RowInsertRequests, SemanticType, @@ -93,11 +96,11 @@ impl AutoCreateTableType { } #[derive(Clone)] -pub struct InsertRequestsWithImmeTtl { +pub struct ImmeInsertRequests { /// Requests with normal ttl. - pub normal_requests: InsertRequests, + pub normal_requests: RegionInsertRequests, /// Requests with ttl=immediate. - pub imme_requests: InsertRequests, + pub imme_requests: RegionInsertRequests, } impl Inserter { @@ -192,12 +195,16 @@ impl Inserter { }); validate_column_count_match(&requests)?; - let table_name_to_ids = self + let (table_name_to_ids, imme_table_ids) = self .create_or_alter_tables_on_demand(&requests, &ctx, create_type, statement_executor) .await?; - let inserts = RowToRegion::new(table_name_to_ids, self.partition_manager.as_ref()) - .convert(requests) - .await?; + let inserts = RowToRegion::new( + table_name_to_ids, + imme_table_ids, + self.partition_manager.as_ref(), + ) + .convert(requests) + .await?; self.do_request(inserts, &ctx).await } @@ -224,7 +231,7 @@ impl Inserter { .await?; // check and create logical tables - let table_name_to_ids = self + let (table_name_to_ids, imme_table_ids) = self .create_or_alter_tables_on_demand( &requests, &ctx, @@ -232,7 +239,7 @@ impl Inserter { statement_executor, ) .await?; - let inserts = RowToRegion::new(table_name_to_ids, &self.partition_manager) + let inserts = RowToRegion::new(table_name_to_ids, imme_table_ids, &self.partition_manager) .convert(requests) .await?; @@ -252,7 +259,6 @@ impl Inserter { table_name: common_catalog::format_full_table_name(catalog, schema, table_name), })?; let table_info = table.table_info(); - add_ttl_imme_region(&ctx, &table_info); let inserts = TableToRegion::new(&table_info, &self.partition_manager) .convert(request) @@ -275,19 +281,15 @@ impl Inserter { } } -/// Check and add regions with ttl=0 to context -pub(crate) fn add_ttl_imme_region(ctx: &QueryContextRef, table_info: &TableInfo) { +pub(crate) fn is_ttl_imme_table(table_info: &TableInfo) -> bool { let ttl = table_info.meta.options.ttl; - - if ttl.map(|t| t.is_immediate()).unwrap_or(false) { - ctx.add_ttl_imme_regions(table_info.region_ids().into_iter().map(|i| i.as_u64())); - } + ttl.map(|t| t.is_immediate()).unwrap_or(false) } impl Inserter { async fn do_request( &self, - requests: RegionInsertRequests, + requests: ImmeInsertRequests, ctx: &QueryContextRef, ) -> Result { let write_cost = write_meter!( @@ -302,8 +304,21 @@ impl Inserter { ..Default::default() }); + let ImmeInsertRequests { + normal_requests, + imme_requests, + } = requests; + // Mirror requests for source table to flownode - match self.mirror_flow_node_requests(&requests).await { + match self + .mirror_flow_node_requests( + normal_requests + .requests + .iter() + .chain(imme_requests.requests.iter()), + ) + .await + { Ok(flow_requests) => { let node_manager = self.node_manager.clone(); let flow_tasks = flow_requests.into_iter().map(|(peer, inserts)| { @@ -339,7 +354,7 @@ impl Inserter { } let write_tasks = self - .group_requests_by_peer(requests, ctx) + .group_requests_by_peer(normal_requests) .await? .into_iter() .map(|(peer, inserts)| { @@ -371,12 +386,12 @@ impl Inserter { /// Mirror requests for source table to flownode async fn mirror_flow_node_requests( &self, - requests: &RegionInsertRequests, + requests: impl IntoIterator, ) -> Result> { // store partial source table requests used by flow node(only store what's used) let mut src_table_reqs: HashMap, RegionInsertRequests)>> = HashMap::new(); - for req in &requests.requests { + for req in requests { let table_id = RegionId::from_u64(req.region_id).table_id(); match src_table_reqs.get_mut(&table_id) { Some(Some((_peers, reqs))) => reqs.requests.push(req.clone()), @@ -437,17 +452,11 @@ impl Inserter { async fn group_requests_by_peer( &self, requests: RegionInsertRequests, - ctx: &QueryContextRef, ) -> Result> { // group by region ids first to reduce repeatedly call `find_region_leader` // TODO(discord9): determine if a addition clone is worth it let mut requests_per_region: HashMap = HashMap::new(); - let ttl_zero_regions = ctx.ttl_zero_regions(); for req in requests.requests { - // filter out ttl zero regions - if ttl_zero_regions.contains(&req.region_id) { - continue; - } let region_id = RegionId::from_u64(req.region_id); requests_per_region .entry(region_id) @@ -486,7 +495,7 @@ impl Inserter { ctx: &QueryContextRef, auto_create_table_type: AutoCreateTableType, statement_executor: &StatementExecutor, - ) -> Result> { + ) -> Result<(HashMap, HashSet)> { let _timer = crate::metrics::CREATE_ALTER_ON_DEMAND .with_label_values(&[auto_create_table_type.as_str()]) .start_timer(); @@ -507,6 +516,7 @@ impl Inserter { })? .unwrap_or(true); if !auto_create_table_hint { + let mut imme_table_ids = HashSet::new(); for req in &requests.inserts { let table = self .get_table(catalog, &schema, &req.table_name) @@ -518,10 +528,12 @@ impl Inserter { ), })?; let table_info = table.table_info(); - add_ttl_imme_region(ctx, &table_info); + if is_ttl_imme_table(&table_info) { + imme_table_ids.insert(table_info.table_id()); + } table_name_to_ids.insert(table_info.name.clone(), table_info.table_id()); } - return Ok(table_name_to_ids); + return Ok((table_name_to_ids, imme_table_ids)); } let mut create_tables = vec![]; @@ -583,6 +595,7 @@ impl Inserter { } } + let mut imme_table_ids = HashSet::new(); // add ttl zero regions to context for table_name in table_name_to_ids.keys() { let table = if let Some(table) = self.get_table(catalog, &schema, table_name).await? { @@ -591,10 +604,12 @@ impl Inserter { continue; }; let table_info = table.table_info(); - add_ttl_imme_region(ctx, &table_info); + if is_ttl_imme_table(&table_info) { + imme_table_ids.insert(table_info.table_id()); + } } - Ok(table_name_to_ids) + Ok((table_name_to_ids, imme_table_ids)) } async fn create_physical_table_on_demand( diff --git a/src/operator/src/req_convert/insert/row_to_region.rs b/src/operator/src/req_convert/insert/row_to_region.rs index a33a1329026d..25c4ea7dbe89 100644 --- a/src/operator/src/req_convert/insert/row_to_region.rs +++ b/src/operator/src/req_convert/insert/row_to_region.rs @@ -12,8 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::HashMap; - +use ahash::{HashMap, HashSet}; use api::v1::region::InsertRequests as RegionInsertRequests; use api::v1::RowInsertRequests; use partition::manager::PartitionRuleManager; @@ -21,37 +20,50 @@ use snafu::OptionExt; use table::metadata::TableId; use crate::error::{Result, TableNotFoundSnafu}; +use crate::insert::ImmeInsertRequests; use crate::req_convert::common::partitioner::Partitioner; pub struct RowToRegion<'a> { table_name_to_ids: HashMap, + imme_table_ids: HashSet, partition_manager: &'a PartitionRuleManager, } impl<'a> RowToRegion<'a> { pub fn new( table_name_to_ids: HashMap, + imme_table_ids: HashSet, partition_manager: &'a PartitionRuleManager, ) -> Self { Self { table_name_to_ids, + imme_table_ids, partition_manager, } } - pub async fn convert(&self, requests: RowInsertRequests) -> Result { + pub async fn convert(&self, requests: RowInsertRequests) -> Result { let mut region_request = Vec::with_capacity(requests.inserts.len()); + let mut imme_request = Vec::with_capacity(requests.inserts.len()); for request in requests.inserts { let table_id = self.get_table_id(&request.table_name)?; let requests = Partitioner::new(self.partition_manager) .partition_insert_requests(table_id, request.rows.unwrap_or_default()) .await?; - - region_request.extend(requests); + if self.imme_table_ids.contains(&table_id) { + imme_request.extend(requests); + } else { + region_request.extend(requests); + } } - Ok(RegionInsertRequests { - requests: region_request, + Ok(ImmeInsertRequests { + normal_requests: RegionInsertRequests { + requests: region_request, + }, + imme_requests: RegionInsertRequests { + requests: imme_request, + }, }) } diff --git a/src/operator/src/req_convert/insert/stmt_to_region.rs b/src/operator/src/req_convert/insert/stmt_to_region.rs index d11126d022b9..b8fa64a867d3 100644 --- a/src/operator/src/req_convert/insert/stmt_to_region.rs +++ b/src/operator/src/req_convert/insert/stmt_to_region.rs @@ -32,7 +32,7 @@ use crate::error::{ ColumnNotFoundSnafu, InvalidSqlSnafu, MissingInsertBodySnafu, ParseSqlSnafu, Result, SchemaReadOnlySnafu, TableNotFoundSnafu, }; -use crate::insert::add_ttl_imme_region; +use crate::insert::{is_ttl_imme_table, ImmeInsertRequests}; use crate::req_convert::common::partitioner::Partitioner; use crate::req_convert::insert::semantic_type; @@ -61,12 +61,11 @@ impl<'a> StatementToRegion<'a> { &self, stmt: &Insert, query_ctx: &QueryContextRef, - ) -> Result { + ) -> Result { let (catalog, schema, table_name) = self.get_full_name(stmt.table_name())?; let table = self.get_table(&catalog, &schema, &table_name).await?; let table_schema = table.schema(); let table_info = table.table_info(); - add_ttl_imme_region(query_ctx, &table_info); ensure!( !common_catalog::consts::is_readonly_schema(&schema), @@ -136,7 +135,18 @@ impl<'a> StatementToRegion<'a> { let requests = Partitioner::new(self.partition_manager) .partition_insert_requests(table_info.table_id(), Rows { schema, rows }) .await?; - Ok(RegionInsertRequests { requests }) + let requests = RegionInsertRequests { requests }; + Ok(if is_ttl_imme_table(&table_info) { + ImmeInsertRequests { + normal_requests: Default::default(), + imme_requests: requests, + } + } else { + ImmeInsertRequests { + normal_requests: requests, + imme_requests: Default::default(), + } + }) } async fn get_table(&self, catalog: &str, schema: &str, table: &str) -> Result { diff --git a/src/operator/src/req_convert/insert/table_to_region.rs b/src/operator/src/req_convert/insert/table_to_region.rs index 729355cf0159..b5dd8512aac3 100644 --- a/src/operator/src/req_convert/insert/table_to_region.rs +++ b/src/operator/src/req_convert/insert/table_to_region.rs @@ -19,6 +19,7 @@ use table::metadata::TableInfo; use table::requests::InsertRequest as TableInsertRequest; use crate::error::Result; +use crate::insert::{is_ttl_imme_table, ImmeInsertRequests}; use crate::req_convert::common::partitioner::Partitioner; use crate::req_convert::common::{column_schema, row_count}; @@ -35,7 +36,7 @@ impl<'a> TableToRegion<'a> { } } - pub async fn convert(&self, request: TableInsertRequest) -> Result { + pub async fn convert(&self, request: TableInsertRequest) -> Result { let row_count = row_count(&request.columns_values)?; let schema = column_schema(self.table_info, &request.columns_values)?; let rows = api::helper::vectors_to_rows(request.columns_values.values(), row_count); @@ -44,7 +45,19 @@ impl<'a> TableToRegion<'a> { let requests = Partitioner::new(self.partition_manager) .partition_insert_requests(self.table_info.table_id(), rows) .await?; - Ok(RegionInsertRequests { requests }) + + let requests = RegionInsertRequests { requests }; + if is_ttl_imme_table(&self.table_info) { + Ok(ImmeInsertRequests { + normal_requests: Default::default(), + imme_requests: requests, + }) + } else { + Ok(ImmeInsertRequests { + normal_requests: requests, + imme_requests: Default::default(), + }) + } } } @@ -112,6 +125,7 @@ mod tests { let region_requests = converter.convert(table_request).await.unwrap(); let mut region_id_to_region_requests = region_requests + .normal_requests .requests .into_iter() .map(|r| (r.region_id, r)) diff --git a/src/session/src/context.rs b/src/session/src/context.rs index 385638873841..4e681253c100 100644 --- a/src/session/src/context.rs +++ b/src/session/src/context.rs @@ -18,7 +18,6 @@ use std::net::SocketAddr; use std::sync::{Arc, RwLock}; use std::time::Duration; -use ahash::HashSet; use api::v1::region::RegionRequestHeader; use arc_swap::ArcSwap; use auth::UserInfoRef; @@ -60,8 +59,6 @@ pub struct QueryContext { #[derive(Debug, Builder, Clone, Default)] pub struct QueryContextMutableFields { warning: Option, - /// regions with ttl=immediate - ttl_imme_regions: HashSet, } impl Display for QueryContext { @@ -287,22 +284,6 @@ impl QueryContext { self.mutable_query_context_data.write().unwrap().warning = Some(msg); } - pub fn add_ttl_imme_regions(&self, region_ids: impl Iterator) { - self.mutable_query_context_data - .write() - .unwrap() - .ttl_imme_regions - .extend(region_ids); - } - - pub fn ttl_zero_regions(&self) -> HashSet { - self.mutable_query_context_data - .read() - .unwrap() - .ttl_imme_regions - .clone() - } - pub fn query_timeout(&self) -> Option { self.mutable_session_data.read().unwrap().query_timeout } From f023d36c73c448de661889d224becde5dfd7ab17 Mon Sep 17 00:00:00 2001 From: discord9 Date: Wed, 4 Dec 2024 21:43:27 +0800 Subject: [PATCH 11/21] fix: use correct lifetime --- src/operator/src/insert.rs | 6 +++--- src/operator/src/req_convert/insert/table_to_region.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/operator/src/insert.rs b/src/operator/src/insert.rs index 0f64171e93d5..549f3a855e95 100644 --- a/src/operator/src/insert.rs +++ b/src/operator/src/insert.rs @@ -384,9 +384,9 @@ impl Inserter { } /// Mirror requests for source table to flownode - async fn mirror_flow_node_requests( - &self, - requests: impl IntoIterator, + async fn mirror_flow_node_requests<'it, 'zelf: 'it>( + &'zelf self, + requests: impl Iterator, ) -> Result> { // store partial source table requests used by flow node(only store what's used) let mut src_table_reqs: HashMap, RegionInsertRequests)>> = diff --git a/src/operator/src/req_convert/insert/table_to_region.rs b/src/operator/src/req_convert/insert/table_to_region.rs index b5dd8512aac3..ef30646b7025 100644 --- a/src/operator/src/req_convert/insert/table_to_region.rs +++ b/src/operator/src/req_convert/insert/table_to_region.rs @@ -47,7 +47,7 @@ impl<'a> TableToRegion<'a> { .await?; let requests = RegionInsertRequests { requests }; - if is_ttl_imme_table(&self.table_info) { + if is_ttl_imme_table(self.table_info) { Ok(ImmeInsertRequests { normal_requests: Default::default(), imme_requests: requests, From 27c46af82d7441b53711c1c0b095ced536f760ac Mon Sep 17 00:00:00 2001 From: discord9 Date: Thu, 5 Dec 2024 12:38:21 +0800 Subject: [PATCH 12/21] refactor: rename immediate to instant --- src/common/base/src/ttl.rs | 12 ++++++------ src/common/meta/src/key/schema_name.rs | 10 ++++------ src/operator/src/insert.rs | 6 +++--- src/operator/src/req_convert/insert/row_to_region.rs | 2 +- .../src/req_convert/insert/stmt_to_region.rs | 4 ++-- .../src/req_convert/insert/table_to_region.rs | 4 ++-- tests/cases/standalone/common/ttl/show_ttl.result | 10 +++++----- tests/cases/standalone/common/ttl/show_ttl.sql | 4 ++-- tests/cases/standalone/common/ttl/ttl_imme.result | 4 ++-- tests/cases/standalone/common/ttl/ttl_imme.sql | 2 +- 10 files changed, 28 insertions(+), 30 deletions(-) diff --git a/src/common/base/src/ttl.rs b/src/common/base/src/ttl.rs index 6eff65418ec5..7b9365eb84e1 100644 --- a/src/common/base/src/ttl.rs +++ b/src/common/base/src/ttl.rs @@ -22,7 +22,7 @@ use serde::{Deserialize, Serialize}; #[serde(rename_all = "snake_case")] pub enum TimeToLive { /// Immediately throw away on insert - Immediate, + Instant, /// Keep the data forever #[default] Forever, @@ -34,7 +34,7 @@ pub enum TimeToLive { impl Display for TimeToLive { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - TimeToLive::Immediate => write!(f, "immediate"), + TimeToLive::Instant => write!(f, "instant"), TimeToLive::Duration(d) => write!(f, "{}", humantime::Duration::from(*d)), TimeToLive::Forever => write!(f, "forever"), } @@ -47,7 +47,7 @@ impl TimeToLive { /// note that a empty string is treat as `forever` too pub fn from_humantime_or_str(s: &str) -> Result { match s { - "immediate" => Ok(TimeToLive::Immediate), + "instant" => Ok(TimeToLive::Instant), "forever" | "" => Ok(TimeToLive::Forever), _ => { let d = humantime::parse_duration(s).map_err(|e| e.to_string())?; @@ -59,14 +59,14 @@ impl TimeToLive { /// Print TimeToLive as string pub fn as_repr_opt(&self) -> Option { match self { - TimeToLive::Immediate => Some("immediate".to_string()), + TimeToLive::Instant => Some("instant".to_string()), TimeToLive::Duration(d) => Some(humantime::format_duration(*d).to_string()), TimeToLive::Forever => Some("forever".to_string()), } } pub fn is_immediate(&self) -> bool { - matches!(self, TimeToLive::Immediate) + matches!(self, TimeToLive::Instant) } /// Is the default value, which is `Forever` @@ -106,7 +106,7 @@ mod test { #[test] fn test_serde() { let cases = vec![ - ("\"immediate\"", TimeToLive::Immediate), + ("\"instant\"", TimeToLive::Instant), ("\"forever\"", TimeToLive::Forever), ("\"10d\"", Duration::from_secs(86400 * 10).into()), ( diff --git a/src/common/meta/src/key/schema_name.rs b/src/common/meta/src/key/schema_name.rs index 972e9208670f..c76b70ef0e8e 100644 --- a/src/common/meta/src/key/schema_name.rs +++ b/src/common/meta/src/key/schema_name.rs @@ -329,9 +329,9 @@ mod tests { assert_eq!("ttl='forever'", schema_value.to_string()); let schema_value = SchemaNameValue { - ttl: Some(TimeToLive::Immediate), + ttl: Some(TimeToLive::Instant), }; - assert_eq!("ttl='immediate'", schema_value.to_string()); + assert_eq!("ttl='instant'", schema_value.to_string()); } #[test] @@ -358,12 +358,10 @@ mod tests { assert_eq!(Some(value), parsed); let imme = SchemaNameValue { - ttl: Some(TimeToLive::Immediate), + ttl: Some(TimeToLive::Instant), }; let parsed = SchemaNameValue::try_from_raw_value( - serde_json::json!({"ttl": "immediate"}) - .to_string() - .as_bytes(), + serde_json::json!({"ttl": "instant"}).to_string().as_bytes(), ) .unwrap(); assert_eq!(Some(imme), parsed); diff --git a/src/operator/src/insert.rs b/src/operator/src/insert.rs index 549f3a855e95..11f3327b1596 100644 --- a/src/operator/src/insert.rs +++ b/src/operator/src/insert.rs @@ -99,8 +99,8 @@ impl AutoCreateTableType { pub struct ImmeInsertRequests { /// Requests with normal ttl. pub normal_requests: RegionInsertRequests, - /// Requests with ttl=immediate. - pub imme_requests: RegionInsertRequests, + /// Requests with ttl=instant. + pub instant_requests: RegionInsertRequests, } impl Inserter { @@ -306,7 +306,7 @@ impl Inserter { let ImmeInsertRequests { normal_requests, - imme_requests, + instant_requests: imme_requests, } = requests; // Mirror requests for source table to flownode diff --git a/src/operator/src/req_convert/insert/row_to_region.rs b/src/operator/src/req_convert/insert/row_to_region.rs index 25c4ea7dbe89..9f5c7ff986b8 100644 --- a/src/operator/src/req_convert/insert/row_to_region.rs +++ b/src/operator/src/req_convert/insert/row_to_region.rs @@ -61,7 +61,7 @@ impl<'a> RowToRegion<'a> { normal_requests: RegionInsertRequests { requests: region_request, }, - imme_requests: RegionInsertRequests { + instant_requests: RegionInsertRequests { requests: imme_request, }, }) diff --git a/src/operator/src/req_convert/insert/stmt_to_region.rs b/src/operator/src/req_convert/insert/stmt_to_region.rs index b8fa64a867d3..8fd47f9393c9 100644 --- a/src/operator/src/req_convert/insert/stmt_to_region.rs +++ b/src/operator/src/req_convert/insert/stmt_to_region.rs @@ -139,12 +139,12 @@ impl<'a> StatementToRegion<'a> { Ok(if is_ttl_imme_table(&table_info) { ImmeInsertRequests { normal_requests: Default::default(), - imme_requests: requests, + instant_requests: requests, } } else { ImmeInsertRequests { normal_requests: requests, - imme_requests: Default::default(), + instant_requests: Default::default(), } }) } diff --git a/src/operator/src/req_convert/insert/table_to_region.rs b/src/operator/src/req_convert/insert/table_to_region.rs index ef30646b7025..984aa7410368 100644 --- a/src/operator/src/req_convert/insert/table_to_region.rs +++ b/src/operator/src/req_convert/insert/table_to_region.rs @@ -50,12 +50,12 @@ impl<'a> TableToRegion<'a> { if is_ttl_imme_table(self.table_info) { Ok(ImmeInsertRequests { normal_requests: Default::default(), - imme_requests: requests, + instant_requests: requests, }) } else { Ok(ImmeInsertRequests { normal_requests: requests, - imme_requests: Default::default(), + instant_requests: Default::default(), }) } } diff --git a/tests/cases/standalone/common/ttl/show_ttl.result b/tests/cases/standalone/common/ttl/show_ttl.result index 1b73c87b3346..8dd446f61dcb 100644 --- a/tests/cases/standalone/common/ttl/show_ttl.result +++ b/tests/cases/standalone/common/ttl/show_ttl.result @@ -91,7 +91,7 @@ SHOW CREATE TABLE test_ttl; | | ) | +----------+-----------------------------------------+ -ALTER TABLE test_ttl SET 'ttl' = 'immediate'; +ALTER TABLE test_ttl SET 'ttl' = 'instant'; Affected Rows: 0 @@ -108,7 +108,7 @@ SHOW CREATE TABLE test_ttl; | | | | | ENGINE=mito | | | WITH( | -| | ttl = 'immediate' | +| | ttl = 'instant' | | | ) | +----------+-----------------------------------------+ @@ -261,7 +261,7 @@ SHOW CREATE DATABASE test_ttl_db; | | ) | +-------------+-------------------------------------------+ -ALTER DATABASE test_ttl_db SET 'ttl' = 'immediate'; +ALTER DATABASE test_ttl_db SET 'ttl' = 'instant'; Affected Rows: 0 @@ -278,7 +278,7 @@ SHOW CREATE TABLE test_ttl; | | | | | ENGINE=mito | | | WITH( | -| | ttl = 'immediate' | +| | ttl = 'instant' | | | ) | +----------+-----------------------------------------+ @@ -289,7 +289,7 @@ SHOW CREATE DATABASE test_ttl_db; +-------------+-------------------------------------------+ | test_ttl_db | CREATE DATABASE IF NOT EXISTS test_ttl_db | | | WITH( | -| | ttl = 'immediate' | +| | ttl = 'instant' | | | ) | +-------------+-------------------------------------------+ diff --git a/tests/cases/standalone/common/ttl/show_ttl.sql b/tests/cases/standalone/common/ttl/show_ttl.sql index 42f38437e1df..0c521ef106cf 100644 --- a/tests/cases/standalone/common/ttl/show_ttl.sql +++ b/tests/cases/standalone/common/ttl/show_ttl.sql @@ -18,7 +18,7 @@ ALTER TABLE test_ttl SET 'ttl' = '6 hours'; SHOW CREATE TABLE test_ttl; -ALTER TABLE test_ttl SET 'ttl' = 'immediate'; +ALTER TABLE test_ttl SET 'ttl' = 'instant'; SHOW CREATE TABLE test_ttl; @@ -50,7 +50,7 @@ SHOW CREATE TABLE test_ttl; SHOW CREATE DATABASE test_ttl_db; -ALTER DATABASE test_ttl_db SET 'ttl' = 'immediate'; +ALTER DATABASE test_ttl_db SET 'ttl' = 'instant'; SHOW CREATE TABLE test_ttl; diff --git a/tests/cases/standalone/common/ttl/ttl_imme.result b/tests/cases/standalone/common/ttl/ttl_imme.result index a1bc767682e6..cef2644295d2 100644 --- a/tests/cases/standalone/common/ttl/ttl_imme.result +++ b/tests/cases/standalone/common/ttl/ttl_imme.result @@ -1,4 +1,4 @@ -CREATE TABLE test_ttl(ts TIMESTAMP TIME INDEX, val INT) WITH (ttl = 'immediate'); +CREATE TABLE test_ttl(ts TIMESTAMP TIME INDEX, val INT) WITH (ttl = 'instant'); Affected Rows: 0 @@ -15,7 +15,7 @@ SHOW CREATE TABLE test_ttl; | | | | | ENGINE=mito | | | WITH( | -| | ttl = 'immediate' | +| | ttl = 'instant' | | | ) | +----------+-----------------------------------------+ diff --git a/tests/cases/standalone/common/ttl/ttl_imme.sql b/tests/cases/standalone/common/ttl/ttl_imme.sql index bb02e9a428a9..d59c145b5cda 100644 --- a/tests/cases/standalone/common/ttl/ttl_imme.sql +++ b/tests/cases/standalone/common/ttl/ttl_imme.sql @@ -1,4 +1,4 @@ -CREATE TABLE test_ttl(ts TIMESTAMP TIME INDEX, val INT) WITH (ttl = 'immediate'); +CREATE TABLE test_ttl(ts TIMESTAMP TIME INDEX, val INT) WITH (ttl = 'instant'); SHOW CREATE TABLE test_ttl; From cc917617d2bbf1ccc5daf5459631542091b901f2 Mon Sep 17 00:00:00 2001 From: discord9 Date: Thu, 5 Dec 2024 14:43:51 +0800 Subject: [PATCH 13/21] tests: flow sink table default ttl --- .../standalone/common/flow/flow_basic.result | 62 ++++++++++++++++--- .../standalone/common/flow/flow_basic.sql | 8 ++- 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/tests/cases/standalone/common/flow/flow_basic.result b/tests/cases/standalone/common/flow/flow_basic.result index cc9b4e038b0f..237cd0493495 100644 --- a/tests/cases/standalone/common/flow/flow_basic.result +++ b/tests/cases/standalone/common/flow/flow_basic.result @@ -227,6 +227,23 @@ ADMIN FLUSH_FLOW('test_distinct_basic'); | FLOW_FLUSHED | +-----------------------------------------+ +SHOW CREATE TABLE out_distinct_basic; + ++--------------------+---------------------------------------------------+ +| Table | Create Table | ++--------------------+---------------------------------------------------+ +| out_distinct_basic | CREATE TABLE IF NOT EXISTS "out_distinct_basic" ( | +| | "dis" INT NULL, | +| | "update_at" TIMESTAMP(3) NULL, | +| | "__ts_placeholder" TIMESTAMP(3) NOT NULL, | +| | TIME INDEX ("__ts_placeholder"), | +| | PRIMARY KEY ("dis") | +| | ) | +| | | +| | ENGINE=mito | +| | | ++--------------------+---------------------------------------------------+ + SELECT dis FROM @@ -321,16 +338,6 @@ where Affected Rows: 0 -SHOW CREATE FLOW filter_numbers_basic; - -+----------------------+----------------------------------------------------------------------------------------------------------------------------------------------+ -| Flow | Create Flow | -+----------------------+----------------------------------------------------------------------------------------------------------------------------------------------+ -| filter_numbers_basic | CREATE FLOW IF NOT EXISTS filter_numbers_basic | -| | SINK TO out_num_cnt_basic | -| | AS SELECT INTERVAL '1 day 1 second', INTERVAL '1 month 1 day 1 second', INTERVAL '1 year 1 month' FROM numbers_input_basic WHERE number > 10 | -+----------------------+----------------------------------------------------------------------------------------------------------------------------------------------+ - drop flow filter_numbers_basic; Affected Rows: 0 @@ -478,6 +485,23 @@ ADMIN FLUSH_FLOW('calc_ngx_country'); | FLOW_FLUSHED | +--------------------------------------+ +SHOW CREATE TABLE ngx_country; + ++-------------+---------------------------------------------+ +| Table | Create Table | ++-------------+---------------------------------------------+ +| ngx_country | CREATE TABLE IF NOT EXISTS "ngx_country" ( | +| | "ngx_access_log.country" STRING NULL, | +| | "update_at" TIMESTAMP(3) NULL, | +| | "__ts_placeholder" TIMESTAMP(3) NOT NULL, | +| | TIME INDEX ("__ts_placeholder"), | +| | PRIMARY KEY ("ngx_access_log.country") | +| | ) | +| | | +| | ENGINE=mito | +| | | ++-------------+---------------------------------------------+ + SELECT "ngx_access_log.country" FROM @@ -594,6 +618,24 @@ ADMIN FLUSH_FLOW('calc_ngx_country'); | FLOW_FLUSHED | +--------------------------------------+ +SHOW CREATE TABLE ngx_country; + ++-------------+---------------------------------------------------------+ +| Table | Create Table | ++-------------+---------------------------------------------------------+ +| ngx_country | CREATE TABLE IF NOT EXISTS "ngx_country" ( | +| | "ngx_access_log.country" STRING NULL, | +| | "time_window" TIMESTAMP(3) NULL, | +| | "update_at" TIMESTAMP(3) NULL, | +| | "__ts_placeholder" TIMESTAMP(3) NOT NULL, | +| | TIME INDEX ("__ts_placeholder"), | +| | PRIMARY KEY ("ngx_access_log.country", "time_window") | +| | ) | +| | | +| | ENGINE=mito | +| | | ++-------------+---------------------------------------------------------+ + SELECT "ngx_access_log.country", time_window diff --git a/tests/cases/standalone/common/flow/flow_basic.sql b/tests/cases/standalone/common/flow/flow_basic.sql index 70d7b14157c2..5accb8ab5836 100644 --- a/tests/cases/standalone/common/flow/flow_basic.sql +++ b/tests/cases/standalone/common/flow/flow_basic.sql @@ -128,6 +128,8 @@ VALUES -- SQLNESS REPLACE (ADMIN\sFLUSH_FLOW\('\w+'\)\s+\|\n\+-+\+\n\|\s+)[0-9]+\s+\| $1 FLOW_FLUSHED | ADMIN FLUSH_FLOW('test_distinct_basic'); +SHOW CREATE TABLE out_distinct_basic; + SELECT dis FROM @@ -180,8 +182,6 @@ FROM where number > 10; -SHOW CREATE FLOW filter_numbers_basic; - drop flow filter_numbers_basic; drop table out_num_cnt_basic; @@ -270,6 +270,8 @@ VALUES -- SQLNESS REPLACE (ADMIN\sFLUSH_FLOW\('\w+'\)\s+\|\n\+-+\+\n\|\s+)[0-9]+\s+\| $1 FLOW_FLUSHED | ADMIN FLUSH_FLOW('calc_ngx_country'); +SHOW CREATE TABLE ngx_country; + SELECT "ngx_access_log.country" FROM @@ -333,6 +335,8 @@ VALUES -- SQLNESS REPLACE (ADMIN\sFLUSH_FLOW\('\w+'\)\s+\|\n\+-+\+\n\|\s+)[0-9]+\s+\| $1 FLOW_FLUSHED | ADMIN FLUSH_FLOW('calc_ngx_country'); +SHOW CREATE TABLE ngx_country; + SELECT "ngx_access_log.country", time_window From 9dd4a0acfd2417be9be4987844b3934c256d4fda Mon Sep 17 00:00:00 2001 From: discord9 Date: Thu, 5 Dec 2024 17:35:39 +0800 Subject: [PATCH 14/21] refactor: per review --- Cargo.lock | 1 + src/common/base/src/lib.rs | 2 +- src/common/base/src/ttl.rs | 29 ++++---- src/common/meta/src/key/schema_name.rs | 12 ++-- src/metric-engine/Cargo.toml | 1 + src/metric-engine/src/engine/create.rs | 3 +- src/operator/src/insert.rs | 67 +++++++++---------- .../src/req_convert/insert/row_to_region.rs | 20 +++--- .../src/req_convert/insert/stmt_to_region.rs | 10 +-- .../src/req_convert/insert/table_to_region.rs | 10 +-- src/query/src/sql/show_create_table.rs | 4 +- src/table/src/metadata.rs | 9 +++ src/table/src/requests.rs | 4 +- .../standalone/common/ttl/ttl_imme.result | 57 ---------------- 14 files changed, 93 insertions(+), 136 deletions(-) delete mode 100644 tests/cases/standalone/common/ttl/ttl_imme.result diff --git a/Cargo.lock b/Cargo.lock index e98a422a59db..37a90122cf84 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6529,6 +6529,7 @@ dependencies = [ "aquamarine", "async-trait", "base64 0.21.7", + "common-base", "common-error", "common-macro", "common-query", diff --git a/src/common/base/src/lib.rs b/src/common/base/src/lib.rs index e81f819fd233..fb631b620f7c 100644 --- a/src/common/base/src/lib.rs +++ b/src/common/base/src/lib.rs @@ -25,4 +25,4 @@ pub type AffectedRows = usize; pub use bit_vec::BitVec; pub use plugins::Plugins; -pub use ttl::TimeToLive; +pub use ttl::{TimeToLive, FOREVER, INSTANT}; diff --git a/src/common/base/src/ttl.rs b/src/common/base/src/ttl.rs index 7b9365eb84e1..a3c78212a90b 100644 --- a/src/common/base/src/ttl.rs +++ b/src/common/base/src/ttl.rs @@ -17,11 +17,14 @@ use std::time::Duration; use serde::{Deserialize, Serialize}; +pub const INSTANT: &str = "instant"; +pub const FOREVER: &str = "forever"; + /// Time To Live #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum TimeToLive { - /// Immediately throw away on insert + /// Instantly discard upon insert Instant, /// Keep the data forever #[default] @@ -34,21 +37,21 @@ pub enum TimeToLive { impl Display for TimeToLive { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - TimeToLive::Instant => write!(f, "instant"), + TimeToLive::Instant => write!(f, "{}", INSTANT), TimeToLive::Duration(d) => write!(f, "{}", humantime::Duration::from(*d)), - TimeToLive::Forever => write!(f, "forever"), + TimeToLive::Forever => write!(f, "{}", FOREVER), } } } impl TimeToLive { - /// Parse a string that is either `immediate`, `forever`, or a duration to `TimeToLive` + /// Parse a string that is either `instant`, `forever`, or a duration to `TimeToLive` /// - /// note that a empty string is treat as `forever` too + /// note that an empty string or a zero duration(a duration that spans no time) is treat as `forever` too pub fn from_humantime_or_str(s: &str) -> Result { - match s { - "instant" => Ok(TimeToLive::Instant), - "forever" | "" => Ok(TimeToLive::Forever), + match s.to_lowercase().as_ref() { + INSTANT => Ok(TimeToLive::Instant), + FOREVER | "" => Ok(TimeToLive::Forever), _ => { let d = humantime::parse_duration(s).map_err(|e| e.to_string())?; Ok(TimeToLive::from(d)) @@ -57,15 +60,16 @@ impl TimeToLive { } /// Print TimeToLive as string - pub fn as_repr_opt(&self) -> Option { + pub fn to_string_opt(&self) -> Option { match self { - TimeToLive::Instant => Some("instant".to_string()), + TimeToLive::Instant => Some(INSTANT.to_string()), TimeToLive::Duration(d) => Some(humantime::format_duration(*d).to_string()), - TimeToLive::Forever => Some("forever".to_string()), + TimeToLive::Forever => Some(FOREVER.to_string()), } } - pub fn is_immediate(&self) -> bool { + /// is instant variant + pub fn is_instant(&self) -> bool { matches!(self, TimeToLive::Instant) } @@ -74,6 +78,7 @@ impl TimeToLive { matches!(self, TimeToLive::Forever) } + /// Get duration if it is a duration variant, otherwise return None pub fn get_duration(&self) -> Option { match self { TimeToLive::Duration(d) => Some(*d), diff --git a/src/common/meta/src/key/schema_name.rs b/src/common/meta/src/key/schema_name.rs index c76b70ef0e8e..295d3d0a1f9a 100644 --- a/src/common/meta/src/key/schema_name.rs +++ b/src/common/meta/src/key/schema_name.rs @@ -62,7 +62,7 @@ pub struct SchemaNameValue { impl Display for SchemaNameValue { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - if let Some(ttl) = self.ttl.and_then(|i| i.as_repr_opt()) { + if let Some(ttl) = self.ttl.and_then(|i| i.to_string_opt()) { write!(f, "ttl='{}'", ttl)?; } @@ -94,7 +94,7 @@ impl TryFrom<&HashMap> for SchemaNameValue { impl From for HashMap { fn from(value: SchemaNameValue) -> Self { let mut opts = HashMap::new(); - if let Some(ttl) = value.ttl.and_then(|ttl| ttl.as_repr_opt()) { + if let Some(ttl) = value.ttl.and_then(|ttl| ttl.to_string_opt()) { opts.insert(OPT_KEY_TTL.to_string(), ttl); } opts @@ -310,6 +310,8 @@ impl<'a> From<&'a SchemaName> for SchemaNameKey<'a> { mod tests { use std::time::Duration; + use common_base::INSTANT; + use super::*; use crate::kv_backend::memory::MemoryKvBackend; @@ -357,14 +359,14 @@ mod tests { .unwrap(); assert_eq!(Some(value), parsed); - let imme = SchemaNameValue { + let instant = SchemaNameValue { ttl: Some(TimeToLive::Instant), }; let parsed = SchemaNameValue::try_from_raw_value( - serde_json::json!({"ttl": "instant"}).to_string().as_bytes(), + serde_json::json!({"ttl": INSTANT}).to_string().as_bytes(), ) .unwrap(); - assert_eq!(Some(imme), parsed); + assert_eq!(Some(instant), parsed); let forever = SchemaNameValue { ttl: Some(TimeToLive::default()), diff --git a/src/metric-engine/Cargo.toml b/src/metric-engine/Cargo.toml index 13aa59fe8b30..85aa371594e8 100644 --- a/src/metric-engine/Cargo.toml +++ b/src/metric-engine/Cargo.toml @@ -12,6 +12,7 @@ api.workspace = true aquamarine.workspace = true async-trait.workspace = true base64.workspace = true +common-base.workspace = true common-error.workspace = true common-macro.workspace = true common-query.workspace = true diff --git a/src/metric-engine/src/engine/create.rs b/src/metric-engine/src/engine/create.rs index 5cb8b8e2c7a2..0f3e430766b7 100644 --- a/src/metric-engine/src/engine/create.rs +++ b/src/metric-engine/src/engine/create.rs @@ -15,6 +15,7 @@ use std::collections::{HashMap, HashSet}; use api::v1::SemanticType; +use common_base::FOREVER; use common_error::ext::BoxedError; use common_telemetry::{info, warn}; use common_time::Timestamp; @@ -540,7 +541,7 @@ pub(crate) fn region_options_for_metadata_region( mut original: HashMap, ) -> HashMap { original.remove(APPEND_MODE_KEY); - original.insert(TTL_KEY.to_string(), "forever".to_string()); + original.insert(TTL_KEY.to_string(), FOREVER.to_string()); original } diff --git a/src/operator/src/insert.rs b/src/operator/src/insert.rs index 11f3327b1596..205d3dcf7a0a 100644 --- a/src/operator/src/insert.rs +++ b/src/operator/src/insert.rs @@ -47,7 +47,6 @@ use store_api::metric_engine_consts::{ }; use store_api::mito_engine_options::{APPEND_MODE_KEY, MERGE_MODE_KEY}; use store_api::storage::{RegionId, TableId}; -use table::metadata::TableInfo; use table::requests::{InsertRequest as TableInsertRequest, AUTO_CREATE_TABLE_KEY, TTL_KEY}; use table::table_reference::TableReference; use table::TableRef; @@ -95,8 +94,14 @@ impl AutoCreateTableType { } } +/// Split insert requests into normal and instant requests. +/// +/// Where instant requests are requests with ttl=instant, +/// and normal requests are requests with ttl set to other values. +/// +/// This is used to split requests for different processing. #[derive(Clone)] -pub struct ImmeInsertRequests { +pub struct InstantInsertRequests { /// Requests with normal ttl. pub normal_requests: RegionInsertRequests, /// Requests with ttl=instant. @@ -195,12 +200,12 @@ impl Inserter { }); validate_column_count_match(&requests)?; - let (table_name_to_ids, imme_table_ids) = self + let (table_name_to_ids, instant_table_ids) = self .create_or_alter_tables_on_demand(&requests, &ctx, create_type, statement_executor) .await?; let inserts = RowToRegion::new( table_name_to_ids, - imme_table_ids, + instant_table_ids, self.partition_manager.as_ref(), ) .convert(requests) @@ -231,7 +236,7 @@ impl Inserter { .await?; // check and create logical tables - let (table_name_to_ids, imme_table_ids) = self + let (table_name_to_ids, instant_table_ids) = self .create_or_alter_tables_on_demand( &requests, &ctx, @@ -239,9 +244,13 @@ impl Inserter { statement_executor, ) .await?; - let inserts = RowToRegion::new(table_name_to_ids, imme_table_ids, &self.partition_manager) - .convert(requests) - .await?; + let inserts = RowToRegion::new( + table_name_to_ids, + instant_table_ids, + &self.partition_manager, + ) + .convert(requests) + .await?; self.do_request(inserts, &ctx).await } @@ -281,15 +290,10 @@ impl Inserter { } } -pub(crate) fn is_ttl_imme_table(table_info: &TableInfo) -> bool { - let ttl = table_info.meta.options.ttl; - ttl.map(|t| t.is_immediate()).unwrap_or(false) -} - impl Inserter { async fn do_request( &self, - requests: ImmeInsertRequests, + requests: InstantInsertRequests, ctx: &QueryContextRef, ) -> Result { let write_cost = write_meter!( @@ -304,9 +308,9 @@ impl Inserter { ..Default::default() }); - let ImmeInsertRequests { + let InstantInsertRequests { normal_requests, - instant_requests: imme_requests, + instant_requests, } = requests; // Mirror requests for source table to flownode @@ -315,7 +319,7 @@ impl Inserter { normal_requests .requests .iter() - .chain(imme_requests.requests.iter()), + .chain(instant_requests.requests.iter()), ) .await { @@ -516,7 +520,7 @@ impl Inserter { })? .unwrap_or(true); if !auto_create_table_hint { - let mut imme_table_ids = HashSet::new(); + let mut instant_table_ids = HashSet::new(); for req in &requests.inserts { let table = self .get_table(catalog, &schema, &req.table_name) @@ -528,20 +532,25 @@ impl Inserter { ), })?; let table_info = table.table_info(); - if is_ttl_imme_table(&table_info) { - imme_table_ids.insert(table_info.table_id()); + if table_info.is_ttl_instant_table() { + instant_table_ids.insert(table_info.table_id()); } table_name_to_ids.insert(table_info.name.clone(), table_info.table_id()); } - return Ok((table_name_to_ids, imme_table_ids)); + return Ok((table_name_to_ids, instant_table_ids)); } let mut create_tables = vec![]; let mut alter_tables = vec![]; + let mut instant_table_ids = HashSet::new(); + for req in &requests.inserts { match self.get_table(catalog, &schema, &req.table_name).await? { Some(table) => { let table_info = table.table_info(); + if table_info.is_ttl_instant_table() { + instant_table_ids.insert(table_info.table_id()); + } table_name_to_ids.insert(table_info.name.clone(), table_info.table_id()); if let Some(alter_expr) = self.get_alter_table_expr_on_demand(req, &table, ctx)? @@ -595,21 +604,7 @@ impl Inserter { } } - let mut imme_table_ids = HashSet::new(); - // add ttl zero regions to context - for table_name in table_name_to_ids.keys() { - let table = if let Some(table) = self.get_table(catalog, &schema, table_name).await? { - table - } else { - continue; - }; - let table_info = table.table_info(); - if is_ttl_imme_table(&table_info) { - imme_table_ids.insert(table_info.table_id()); - } - } - - Ok((table_name_to_ids, imme_table_ids)) + Ok((table_name_to_ids, instant_table_ids)) } async fn create_physical_table_on_demand( diff --git a/src/operator/src/req_convert/insert/row_to_region.rs b/src/operator/src/req_convert/insert/row_to_region.rs index 9f5c7ff986b8..e1aa09d0cd10 100644 --- a/src/operator/src/req_convert/insert/row_to_region.rs +++ b/src/operator/src/req_convert/insert/row_to_region.rs @@ -20,49 +20,49 @@ use snafu::OptionExt; use table::metadata::TableId; use crate::error::{Result, TableNotFoundSnafu}; -use crate::insert::ImmeInsertRequests; +use crate::insert::InstantInsertRequests; use crate::req_convert::common::partitioner::Partitioner; pub struct RowToRegion<'a> { table_name_to_ids: HashMap, - imme_table_ids: HashSet, + instant_table_ids: HashSet, partition_manager: &'a PartitionRuleManager, } impl<'a> RowToRegion<'a> { pub fn new( table_name_to_ids: HashMap, - imme_table_ids: HashSet, + instant_table_ids: HashSet, partition_manager: &'a PartitionRuleManager, ) -> Self { Self { table_name_to_ids, - imme_table_ids, + instant_table_ids, partition_manager, } } - pub async fn convert(&self, requests: RowInsertRequests) -> Result { + pub async fn convert(&self, requests: RowInsertRequests) -> Result { let mut region_request = Vec::with_capacity(requests.inserts.len()); - let mut imme_request = Vec::with_capacity(requests.inserts.len()); + let mut instant_request = Vec::with_capacity(requests.inserts.len()); for request in requests.inserts { let table_id = self.get_table_id(&request.table_name)?; let requests = Partitioner::new(self.partition_manager) .partition_insert_requests(table_id, request.rows.unwrap_or_default()) .await?; - if self.imme_table_ids.contains(&table_id) { - imme_request.extend(requests); + if self.instant_table_ids.contains(&table_id) { + instant_request.extend(requests); } else { region_request.extend(requests); } } - Ok(ImmeInsertRequests { + Ok(InstantInsertRequests { normal_requests: RegionInsertRequests { requests: region_request, }, instant_requests: RegionInsertRequests { - requests: imme_request, + requests: instant_request, }, }) } diff --git a/src/operator/src/req_convert/insert/stmt_to_region.rs b/src/operator/src/req_convert/insert/stmt_to_region.rs index 8fd47f9393c9..b15aa43e1506 100644 --- a/src/operator/src/req_convert/insert/stmt_to_region.rs +++ b/src/operator/src/req_convert/insert/stmt_to_region.rs @@ -32,7 +32,7 @@ use crate::error::{ ColumnNotFoundSnafu, InvalidSqlSnafu, MissingInsertBodySnafu, ParseSqlSnafu, Result, SchemaReadOnlySnafu, TableNotFoundSnafu, }; -use crate::insert::{is_ttl_imme_table, ImmeInsertRequests}; +use crate::insert::InstantInsertRequests; use crate::req_convert::common::partitioner::Partitioner; use crate::req_convert::insert::semantic_type; @@ -61,7 +61,7 @@ impl<'a> StatementToRegion<'a> { &self, stmt: &Insert, query_ctx: &QueryContextRef, - ) -> Result { + ) -> Result { let (catalog, schema, table_name) = self.get_full_name(stmt.table_name())?; let table = self.get_table(&catalog, &schema, &table_name).await?; let table_schema = table.schema(); @@ -136,13 +136,13 @@ impl<'a> StatementToRegion<'a> { .partition_insert_requests(table_info.table_id(), Rows { schema, rows }) .await?; let requests = RegionInsertRequests { requests }; - Ok(if is_ttl_imme_table(&table_info) { - ImmeInsertRequests { + Ok(if table_info.is_ttl_instant_table() { + InstantInsertRequests { normal_requests: Default::default(), instant_requests: requests, } } else { - ImmeInsertRequests { + InstantInsertRequests { normal_requests: requests, instant_requests: Default::default(), } diff --git a/src/operator/src/req_convert/insert/table_to_region.rs b/src/operator/src/req_convert/insert/table_to_region.rs index 984aa7410368..d00f6992f0a2 100644 --- a/src/operator/src/req_convert/insert/table_to_region.rs +++ b/src/operator/src/req_convert/insert/table_to_region.rs @@ -19,7 +19,7 @@ use table::metadata::TableInfo; use table::requests::InsertRequest as TableInsertRequest; use crate::error::Result; -use crate::insert::{is_ttl_imme_table, ImmeInsertRequests}; +use crate::insert::InstantInsertRequests; use crate::req_convert::common::partitioner::Partitioner; use crate::req_convert::common::{column_schema, row_count}; @@ -36,7 +36,7 @@ impl<'a> TableToRegion<'a> { } } - pub async fn convert(&self, request: TableInsertRequest) -> Result { + pub async fn convert(&self, request: TableInsertRequest) -> Result { let row_count = row_count(&request.columns_values)?; let schema = column_schema(self.table_info, &request.columns_values)?; let rows = api::helper::vectors_to_rows(request.columns_values.values(), row_count); @@ -47,13 +47,13 @@ impl<'a> TableToRegion<'a> { .await?; let requests = RegionInsertRequests { requests }; - if is_ttl_imme_table(self.table_info) { - Ok(ImmeInsertRequests { + if self.table_info.is_ttl_instant_table() { + Ok(InstantInsertRequests { normal_requests: Default::default(), instant_requests: requests, }) } else { - Ok(ImmeInsertRequests { + Ok(InstantInsertRequests { normal_requests: requests, instant_requests: Default::default(), }) diff --git a/src/query/src/sql/show_create_table.rs b/src/query/src/sql/show_create_table.rs index e871d95ac390..ea39dc1aa5f2 100644 --- a/src/query/src/sql/show_create_table.rs +++ b/src/query/src/sql/show_create_table.rs @@ -45,11 +45,11 @@ fn create_sql_options(table_meta: &TableMeta, schema_options: Option bool { + self.meta + .options + .ttl + .map(|t| t.is_instant()) + .unwrap_or(false) + } } impl TableInfoBuilder { diff --git a/src/table/src/requests.rs b/src/table/src/requests.rs index 9aeb28c88175..803ba2b4c977 100644 --- a/src/table/src/requests.rs +++ b/src/table/src/requests.rs @@ -134,7 +134,7 @@ impl fmt::Display for TableOptions { key_vals.push(format!("{}={}", WRITE_BUFFER_SIZE_KEY, size)); } - if let Some(ttl) = self.ttl.and_then(|ttl| ttl.as_repr_opt()) { + if let Some(ttl) = self.ttl.and_then(|ttl| ttl.to_string_opt()) { key_vals.push(format!("{}={}", TTL_KEY, ttl)); } @@ -155,7 +155,7 @@ impl From<&TableOptions> for HashMap { write_buffer_size.to_string(), ); } - if let Some(ttl_str) = opts.ttl.and_then(|ttl| ttl.as_repr_opt()) { + if let Some(ttl_str) = opts.ttl.and_then(|ttl| ttl.to_string_opt()) { let _ = res.insert(TTL_KEY.to_string(), ttl_str); } res.extend( diff --git a/tests/cases/standalone/common/ttl/ttl_imme.result b/tests/cases/standalone/common/ttl/ttl_imme.result deleted file mode 100644 index cef2644295d2..000000000000 --- a/tests/cases/standalone/common/ttl/ttl_imme.result +++ /dev/null @@ -1,57 +0,0 @@ -CREATE TABLE test_ttl(ts TIMESTAMP TIME INDEX, val INT) WITH (ttl = 'instant'); - -Affected Rows: 0 - -SHOW CREATE TABLE test_ttl; - -+----------+-----------------------------------------+ -| Table | Create Table | -+----------+-----------------------------------------+ -| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | -| | "ts" TIMESTAMP(3) NOT NULL, | -| | "val" INT NULL, | -| | TIME INDEX ("ts") | -| | ) | -| | | -| | ENGINE=mito | -| | WITH( | -| | ttl = 'instant' | -| | ) | -+----------+-----------------------------------------+ - -INSERT INTO test_ttl VALUES - (now(), 1); - -Affected Rows: 0 - -SELECT val from test_ttl; - -++ -++ - --- SQLNESS SLEEP 2s -ADMIN flush_table('test_ttl'); - -+-------------------------------+ -| ADMIN flush_table('test_ttl') | -+-------------------------------+ -| 0 | -+-------------------------------+ - -ADMIN compact_table('test_ttl'); - -+---------------------------------+ -| ADMIN compact_table('test_ttl') | -+---------------------------------+ -| 0 | -+---------------------------------+ - -SELECT val from test_ttl; - -++ -++ - -DROP TABLE test_ttl; - -Affected Rows: 0 - From 555dd12e81413a9e532b3b761649346c66060eef Mon Sep 17 00:00:00 2001 From: discord9 Date: Thu, 5 Dec 2024 17:35:53 +0800 Subject: [PATCH 15/21] tests: sqlness --- .../common/flow/flow_advance.result | 101 ++++++++++++++++++ .../standalone/common/flow/flow_advance.sql | 39 +++++++ .../cases/standalone/common/ttl/ttl_imme.sql | 17 --- .../standalone/common/ttl/ttl_instant.result | 80 ++++++++++++++ .../standalone/common/ttl/ttl_instant.sql | 38 +++++++ 5 files changed, 258 insertions(+), 17 deletions(-) create mode 100644 tests/cases/standalone/common/flow/flow_advance.result create mode 100644 tests/cases/standalone/common/flow/flow_advance.sql delete mode 100644 tests/cases/standalone/common/ttl/ttl_imme.sql create mode 100644 tests/cases/standalone/common/ttl/ttl_instant.result create mode 100644 tests/cases/standalone/common/ttl/ttl_instant.sql diff --git a/tests/cases/standalone/common/flow/flow_advance.result b/tests/cases/standalone/common/flow/flow_advance.result new file mode 100644 index 000000000000..38d14d6b3155 --- /dev/null +++ b/tests/cases/standalone/common/flow/flow_advance.result @@ -0,0 +1,101 @@ +-- test ttl = instant +CREATE TABLE distinct_basic ( + number INT, + ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY(number), + TIME INDEX(ts) +)WITH ('ttl' = 'instant'); + +Affected Rows: 0 + +CREATE FLOW test_distinct_basic SINK TO out_distinct_basic AS +SELECT + DISTINCT number as dis +FROM + distinct_basic; + +Affected Rows: 0 + +-- SQLNESS ARG restart=true +INSERT INTO + distinct_basic +VALUES + (20, "2021-07-01 00:00:00.200"), + (20, "2021-07-01 00:00:00.200"), + (22, "2021-07-01 00:00:00.600"); + +Affected Rows: 0 + +-- SQLNESS REPLACE (ADMIN\sFLUSH_FLOW\('\w+'\)\s+\|\n\+-+\+\n\|\s+)[0-9]+\s+\| $1 FLOW_FLUSHED | +ADMIN FLUSH_FLOW('test_distinct_basic'); + ++-----------------------------------------+ +| ADMIN FLUSH_FLOW('test_distinct_basic') | ++-----------------------------------------+ +| FLOW_FLUSHED | ++-----------------------------------------+ + +SHOW CREATE TABLE distinct_basic; + ++----------------+-----------------------------------------------------------+ +| Table | Create Table | ++----------------+-----------------------------------------------------------+ +| distinct_basic | CREATE TABLE IF NOT EXISTS "distinct_basic" ( | +| | "number" INT NULL, | +| | "ts" TIMESTAMP(3) NOT NULL DEFAULT current_timestamp(), | +| | TIME INDEX ("ts"), | +| | PRIMARY KEY ("number") | +| | ) | +| | | +| | ENGINE=mito | +| | WITH( | +| | ttl = 'instant' | +| | ) | ++----------------+-----------------------------------------------------------+ + +SHOW CREATE TABLE out_distinct_basic; + ++--------------------+---------------------------------------------------+ +| Table | Create Table | ++--------------------+---------------------------------------------------+ +| out_distinct_basic | CREATE TABLE IF NOT EXISTS "out_distinct_basic" ( | +| | "dis" INT NULL, | +| | "update_at" TIMESTAMP(3) NULL, | +| | "__ts_placeholder" TIMESTAMP(3) NOT NULL, | +| | TIME INDEX ("__ts_placeholder"), | +| | PRIMARY KEY ("dis") | +| | ) | +| | | +| | ENGINE=mito | +| | | ++--------------------+---------------------------------------------------+ + +SELECT + dis +FROM + out_distinct_basic; + ++-----+ +| dis | ++-----+ +| 20 | +| 22 | ++-----+ + +SELECT number FROM distinct_basic; + +++ +++ + +DROP FLOW test_distinct_basic; + +Affected Rows: 0 + +DROP TABLE distinct_basic; + +Affected Rows: 0 + +DROP TABLE out_distinct_basic; + +Affected Rows: 0 + diff --git a/tests/cases/standalone/common/flow/flow_advance.sql b/tests/cases/standalone/common/flow/flow_advance.sql new file mode 100644 index 000000000000..18dfea25db04 --- /dev/null +++ b/tests/cases/standalone/common/flow/flow_advance.sql @@ -0,0 +1,39 @@ +-- test ttl = instant +CREATE TABLE distinct_basic ( + number INT, + ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY(number), + TIME INDEX(ts) +)WITH ('ttl' = 'instant'); + +CREATE FLOW test_distinct_basic SINK TO out_distinct_basic AS +SELECT + DISTINCT number as dis +FROM + distinct_basic; + +-- SQLNESS ARG restart=true +INSERT INTO + distinct_basic +VALUES + (20, "2021-07-01 00:00:00.200"), + (20, "2021-07-01 00:00:00.200"), + (22, "2021-07-01 00:00:00.600"); + +-- SQLNESS REPLACE (ADMIN\sFLUSH_FLOW\('\w+'\)\s+\|\n\+-+\+\n\|\s+)[0-9]+\s+\| $1 FLOW_FLUSHED | +ADMIN FLUSH_FLOW('test_distinct_basic'); + +SHOW CREATE TABLE distinct_basic; + +SHOW CREATE TABLE out_distinct_basic; + +SELECT + dis +FROM + out_distinct_basic; + +SELECT number FROM distinct_basic; + +DROP FLOW test_distinct_basic; +DROP TABLE distinct_basic; +DROP TABLE out_distinct_basic; \ No newline at end of file diff --git a/tests/cases/standalone/common/ttl/ttl_imme.sql b/tests/cases/standalone/common/ttl/ttl_imme.sql deleted file mode 100644 index d59c145b5cda..000000000000 --- a/tests/cases/standalone/common/ttl/ttl_imme.sql +++ /dev/null @@ -1,17 +0,0 @@ -CREATE TABLE test_ttl(ts TIMESTAMP TIME INDEX, val INT) WITH (ttl = 'instant'); - -SHOW CREATE TABLE test_ttl; - -INSERT INTO test_ttl VALUES - (now(), 1); - -SELECT val from test_ttl; - --- SQLNESS SLEEP 2s -ADMIN flush_table('test_ttl'); - -ADMIN compact_table('test_ttl'); - -SELECT val from test_ttl; - -DROP TABLE test_ttl; diff --git a/tests/cases/standalone/common/ttl/ttl_instant.result b/tests/cases/standalone/common/ttl/ttl_instant.result new file mode 100644 index 000000000000..0fb1739e97f9 --- /dev/null +++ b/tests/cases/standalone/common/ttl/ttl_instant.result @@ -0,0 +1,80 @@ +CREATE TABLE test_ttl(ts TIMESTAMP TIME INDEX, val INT) WITH (ttl = 'instant'); + +Affected Rows: 0 + +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts") | +| | ) | +| | | +| | ENGINE=mito | +| | WITH( | +| | ttl = 'instant' | +| | ) | ++----------+-----------------------------------------+ + +INSERT INTO + test_ttl +VALUES + (now(), 1), + (now(), 2), + (now(), 3), + (now() + INTERVAL '1 SECOND', 4), + (now() - INTERVAL '1 SECOND', 5); + +Affected Rows: 0 + +SELECT + val +from + test_ttl; + +++ +++ + +INSERT INTO + test_ttl +VALUES + (now(), 1), + (now(), 2), + (now(), 3), + (now() + INTERVAL '1 SECOND', 4), + (now() - INTERVAL '1 SECOND', 5); + +Affected Rows: 0 + +-- SQLNESS SLEEP 2s +ADMIN flush_table('test_ttl'); + ++-------------------------------+ +| ADMIN flush_table('test_ttl') | ++-------------------------------+ +| 0 | ++-------------------------------+ + +ADMIN compact_table('test_ttl'); + ++---------------------------------+ +| ADMIN compact_table('test_ttl') | ++---------------------------------+ +| 0 | ++---------------------------------+ + +SELECT + val +from + test_ttl; + +++ +++ + +DROP TABLE test_ttl; + +Affected Rows: 0 + diff --git a/tests/cases/standalone/common/ttl/ttl_instant.sql b/tests/cases/standalone/common/ttl/ttl_instant.sql new file mode 100644 index 000000000000..8f503d01de40 --- /dev/null +++ b/tests/cases/standalone/common/ttl/ttl_instant.sql @@ -0,0 +1,38 @@ +CREATE TABLE test_ttl(ts TIMESTAMP TIME INDEX, val INT) WITH (ttl = 'instant'); + +SHOW CREATE TABLE test_ttl; + +INSERT INTO + test_ttl +VALUES + (now(), 1), + (now(), 2), + (now(), 3), + (now() + INTERVAL '1 SECOND', 4), + (now() - INTERVAL '1 SECOND', 5); + +SELECT + val +from + test_ttl; + +INSERT INTO + test_ttl +VALUES + (now(), 1), + (now(), 2), + (now(), 3), + (now() + INTERVAL '1 SECOND', 4), + (now() - INTERVAL '1 SECOND', 5); + +-- SQLNESS SLEEP 2s +ADMIN flush_table('test_ttl'); + +ADMIN compact_table('test_ttl'); + +SELECT + val +from + test_ttl; + +DROP TABLE test_ttl; \ No newline at end of file From ec321d6fe1ab8bdeadf52cd4ed3b6d8450f220db Mon Sep 17 00:00:00 2001 From: discord9 Date: Thu, 5 Dec 2024 18:45:37 +0800 Subject: [PATCH 16/21] fix: ttl alter to instant --- Cargo.lock | 2 ++ src/common/base/src/lib.rs | 2 -- src/common/meta/src/key/schema_name.rs | 4 ++-- src/common/meta/src/rpc/ddl.rs | 2 +- src/common/time/Cargo.toml | 2 ++ src/common/time/src/lib.rs | 2 ++ src/common/{base => time}/src/ttl.rs | 16 ++++++++++++++++ src/metric-engine/src/engine/create.rs | 3 +-- src/mito2/src/compaction.rs | 16 ++++------------ src/mito2/src/compaction/compactor.rs | 2 +- src/mito2/src/region/options.rs | 2 +- src/mito2/src/sst/version.rs | 13 ++++++++++--- src/store-api/src/region_request.rs | 2 +- src/table/src/requests.rs | 2 +- 14 files changed, 44 insertions(+), 26 deletions(-) rename src/common/{base => time}/src/ttl.rs (90%) diff --git a/Cargo.lock b/Cargo.lock index 37a90122cf84..d476f42179b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2394,6 +2394,8 @@ dependencies = [ "chrono-tz 0.8.6", "common-error", "common-macro", + "humantime", + "humantime-serde", "once_cell", "rand", "serde", diff --git a/src/common/base/src/lib.rs b/src/common/base/src/lib.rs index fb631b620f7c..62a801d9462d 100644 --- a/src/common/base/src/lib.rs +++ b/src/common/base/src/lib.rs @@ -19,10 +19,8 @@ pub mod range_read; #[allow(clippy::all)] pub mod readable_size; pub mod secrets; -pub mod ttl; pub type AffectedRows = usize; pub use bit_vec::BitVec; pub use plugins::Plugins; -pub use ttl::{TimeToLive, FOREVER, INSTANT}; diff --git a/src/common/meta/src/key/schema_name.rs b/src/common/meta/src/key/schema_name.rs index 295d3d0a1f9a..1e144b0a246b 100644 --- a/src/common/meta/src/key/schema_name.rs +++ b/src/common/meta/src/key/schema_name.rs @@ -16,8 +16,8 @@ use std::collections::HashMap; use std::fmt::Display; use std::sync::Arc; -use common_base::TimeToLive; use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; +use common_time::TimeToLive; use futures::stream::BoxStream; use humantime_serde::re::humantime; use serde::{Deserialize, Serialize}; @@ -310,7 +310,7 @@ impl<'a> From<&'a SchemaName> for SchemaNameKey<'a> { mod tests { use std::time::Duration; - use common_base::INSTANT; + use common_time::INSTANT; use super::*; use crate::kv_backend::memory::MemoryKvBackend; diff --git a/src/common/meta/src/rpc/ddl.rs b/src/common/meta/src/rpc/ddl.rs index 03828a632e25..228ca8607582 100644 --- a/src/common/meta/src/rpc/ddl.rs +++ b/src/common/meta/src/rpc/ddl.rs @@ -35,7 +35,7 @@ use api::v1::{ }; use base64::engine::general_purpose; use base64::Engine as _; -use common_base::TimeToLive; +use common_time::TimeToLive; use prost::Message; use serde::{Deserialize, Serialize}; use serde_with::{serde_as, DefaultOnNull}; diff --git a/src/common/time/Cargo.toml b/src/common/time/Cargo.toml index fdd06140f187..28ff40ff90c8 100644 --- a/src/common/time/Cargo.toml +++ b/src/common/time/Cargo.toml @@ -13,6 +13,8 @@ chrono.workspace = true chrono-tz = "0.8" common-error.workspace = true common-macro.workspace = true +humantime.workspace = true +humantime-serde.workspace = true once_cell.workspace = true serde = { version = "1.0", features = ["derive"] } serde_json.workspace = true diff --git a/src/common/time/src/lib.rs b/src/common/time/src/lib.rs index fa025bf661c2..8511c3520d59 100644 --- a/src/common/time/src/lib.rs +++ b/src/common/time/src/lib.rs @@ -22,6 +22,7 @@ pub mod time; pub mod timestamp; pub mod timestamp_millis; pub mod timezone; +pub mod ttl; pub mod util; pub use date::Date; @@ -32,3 +33,4 @@ pub use range::RangeMillis; pub use timestamp::Timestamp; pub use timestamp_millis::TimestampMillis; pub use timezone::Timezone; +pub use ttl::{TimeToLive, FOREVER, INSTANT}; diff --git a/src/common/base/src/ttl.rs b/src/common/time/src/ttl.rs similarity index 90% rename from src/common/base/src/ttl.rs rename to src/common/time/src/ttl.rs index a3c78212a90b..d4ad377fd1e3 100644 --- a/src/common/base/src/ttl.rs +++ b/src/common/time/src/ttl.rs @@ -17,6 +17,8 @@ use std::time::Duration; use serde::{Deserialize, Serialize}; +use crate::Timestamp; + pub const INSTANT: &str = "instant"; pub const FOREVER: &str = "forever"; @@ -59,6 +61,20 @@ impl TimeToLive { } } + /// Check if the TimeToLive is expired + /// with the given `created_at` and `now` timestamp + pub fn is_expired( + &self, + created_at: &Timestamp, + now: &Timestamp, + ) -> crate::error::Result { + Ok(match self { + TimeToLive::Instant => true, + TimeToLive::Forever => false, + TimeToLive::Duration(d) => now.sub_duration(*d)? > *created_at, + }) + } + /// Print TimeToLive as string pub fn to_string_opt(&self) -> Option { match self { diff --git a/src/metric-engine/src/engine/create.rs b/src/metric-engine/src/engine/create.rs index 0f3e430766b7..b87682d4599e 100644 --- a/src/metric-engine/src/engine/create.rs +++ b/src/metric-engine/src/engine/create.rs @@ -15,10 +15,9 @@ use std::collections::{HashMap, HashSet}; use api::v1::SemanticType; -use common_base::FOREVER; use common_error::ext::BoxedError; use common_telemetry::{info, warn}; -use common_time::Timestamp; +use common_time::{Timestamp, FOREVER}; use datatypes::data_type::ConcreteDataType; use datatypes::schema::ColumnSchema; use datatypes::value::Value; diff --git a/src/mito2/src/compaction.rs b/src/mito2/src/compaction.rs index 554652393dd0..2977558c3fba 100644 --- a/src/mito2/src/compaction.rs +++ b/src/mito2/src/compaction.rs @@ -27,12 +27,12 @@ use std::sync::Arc; use std::time::Instant; use api::v1::region::compact_request; -use common_base::{Plugins, TimeToLive}; +use common_base::Plugins; use common_meta::key::SchemaMetadataManagerRef; use common_telemetry::{debug, error, info, warn}; use common_time::range::TimestampRange; use common_time::timestamp::TimeUnit; -use common_time::Timestamp; +use common_time::{TimeToLive, Timestamp}; use datafusion_common::ScalarValue; use datafusion_expr::Expr; use serde::{Deserialize, Serialize}; @@ -661,21 +661,13 @@ fn get_expired_ssts( ttl: Option, now: Timestamp, ) -> Vec { - let Some(ttl) = ttl.and_then(|t| t.get_duration()) else { + let Some(ttl) = ttl else { return vec![]; }; - let expire_time = match now.sub_duration(ttl) { - Ok(expire_time) => expire_time, - Err(e) => { - error!(e; "Failed to calculate region TTL expire time"); - return vec![]; - } - }; - levels .iter() - .flat_map(|l| l.get_expired_files(&expire_time).into_iter()) + .flat_map(|l| l.get_expired_files(&now, &ttl).into_iter()) .collect() } diff --git a/src/mito2/src/compaction/compactor.rs b/src/mito2/src/compaction/compactor.rs index 27668846f420..792634b2e4a2 100644 --- a/src/mito2/src/compaction/compactor.rs +++ b/src/mito2/src/compaction/compactor.rs @@ -16,9 +16,9 @@ use std::sync::Arc; use std::time::Duration; use api::v1::region::compact_request; -use common_base::TimeToLive; use common_meta::key::SchemaMetadataManagerRef; use common_telemetry::{info, warn}; +use common_time::TimeToLive; use object_store::manager::ObjectStoreManagerRef; use serde::{Deserialize, Serialize}; use smallvec::SmallVec; diff --git a/src/mito2/src/region/options.rs b/src/mito2/src/region/options.rs index 3e9eb62c0629..4514137cc335 100644 --- a/src/mito2/src/region/options.rs +++ b/src/mito2/src/region/options.rs @@ -20,7 +20,7 @@ use std::collections::HashMap; use std::time::Duration; use common_base::readable_size::ReadableSize; -use common_base::TimeToLive; +use common_time::TimeToLive; use common_wal::options::{WalOptions, WAL_OPTIONS_KEY}; use serde::de::Error as _; use serde::{Deserialize, Deserializer, Serialize}; diff --git a/src/mito2/src/sst/version.rs b/src/mito2/src/sst/version.rs index c677a9541344..891ded08f6bd 100644 --- a/src/mito2/src/sst/version.rs +++ b/src/mito2/src/sst/version.rs @@ -17,7 +17,7 @@ use std::collections::HashMap; use std::fmt; use std::sync::Arc; -use common_time::Timestamp; +use common_time::{TimeToLive, Timestamp}; use crate::sst::file::{FileHandle, FileId, FileMeta, Level, MAX_LEVEL}; use crate::sst::file_purger::FilePurgerRef; @@ -160,12 +160,19 @@ impl LevelMeta { } /// Returns expired SSTs from current level. - pub fn get_expired_files(&self, expire_time: &Timestamp) -> Vec { + pub fn get_expired_files(&self, now: &Timestamp, ttl: &TimeToLive) -> Vec { self.files .values() .filter(|v| { let (_, end) = v.time_range(); - &end < expire_time + + match ttl.is_expired(&end, now) { + Ok(expired) => expired, + Err(e) => { + common_telemetry::error!(e; "Failed to calculate region TTL expire time"); + false + } + } }) .cloned() .collect() diff --git a/src/store-api/src/region_request.rs b/src/store-api/src/region_request.rs index d91d431ffbb5..ee9ebf483d98 100644 --- a/src/store-api/src/region_request.rs +++ b/src/store-api/src/region_request.rs @@ -25,7 +25,7 @@ use api::v1::region::{ }; use api::v1::{self, Analyzer, Option as PbOption, Rows, SemanticType}; pub use common_base::AffectedRows; -use common_base::TimeToLive; +use common_time::TimeToLive; use datatypes::data_type::ConcreteDataType; use datatypes::schema::FulltextOptions; use serde::{Deserialize, Serialize}; diff --git a/src/table/src/requests.rs b/src/table/src/requests.rs index 803ba2b4c977..66b405b7bc7e 100644 --- a/src/table/src/requests.rs +++ b/src/table/src/requests.rs @@ -19,10 +19,10 @@ use std::fmt; use std::str::FromStr; use common_base::readable_size::ReadableSize; -use common_base::TimeToLive; use common_datasource::object_store::s3::is_supported_in_s3; use common_query::AddColumnLocation; use common_time::range::TimestampRange; +use common_time::TimeToLive; use datatypes::data_type::ConcreteDataType; use datatypes::prelude::VectorRef; use datatypes::schema::{ColumnSchema, FulltextOptions}; From c5186dd7ea6cde2ee89deb52c93b154a53b9a1d5 Mon Sep 17 00:00:00 2001 From: discord9 Date: Thu, 5 Dec 2024 19:23:24 +0800 Subject: [PATCH 17/21] tests: sqlness --- ...advance.result => flow_advance_ttl.result} | 0 ...{flow_advance.sql => flow_advance_ttl.sql} | 0 .../standalone/common/ttl/ttl_instant.result | 254 +++++++++++++++++- .../standalone/common/ttl/ttl_instant.sql | 121 ++++++++- 4 files changed, 360 insertions(+), 15 deletions(-) rename tests/cases/standalone/common/flow/{flow_advance.result => flow_advance_ttl.result} (100%) rename tests/cases/standalone/common/flow/{flow_advance.sql => flow_advance_ttl.sql} (100%) diff --git a/tests/cases/standalone/common/flow/flow_advance.result b/tests/cases/standalone/common/flow/flow_advance_ttl.result similarity index 100% rename from tests/cases/standalone/common/flow/flow_advance.result rename to tests/cases/standalone/common/flow/flow_advance_ttl.result diff --git a/tests/cases/standalone/common/flow/flow_advance.sql b/tests/cases/standalone/common/flow/flow_advance_ttl.sql similarity index 100% rename from tests/cases/standalone/common/flow/flow_advance.sql rename to tests/cases/standalone/common/flow/flow_advance_ttl.sql diff --git a/tests/cases/standalone/common/ttl/ttl_instant.result b/tests/cases/standalone/common/ttl/ttl_instant.result index 0fb1739e97f9..e8d74f4c4bdf 100644 --- a/tests/cases/standalone/common/ttl/ttl_instant.result +++ b/tests/cases/standalone/common/ttl/ttl_instant.result @@ -1,4 +1,8 @@ -CREATE TABLE test_ttl(ts TIMESTAMP TIME INDEX, val INT) WITH (ttl = 'instant'); +CREATE TABLE test_ttl( + ts TIMESTAMP TIME INDEX, + val INT, + PRIMARY KEY (`val`) +) WITH (ttl = 'instant'); Affected Rows: 0 @@ -10,7 +14,8 @@ SHOW CREATE TABLE test_ttl; | test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | | | "ts" TIMESTAMP(3) NOT NULL, | | | "val" INT NULL, | -| | TIME INDEX ("ts") | +| | TIME INDEX ("ts"), | +| | PRIMARY KEY ("val") | | | ) | | | | | | ENGINE=mito | @@ -24,9 +29,7 @@ INSERT INTO VALUES (now(), 1), (now(), 2), - (now(), 3), - (now() + INTERVAL '1 SECOND', 4), - (now() - INTERVAL '1 SECOND', 5); + (now(), 3); Affected Rows: 0 @@ -38,17 +41,252 @@ from ++ ++ +-- SQLNESS SLEEP 2s +ADMIN flush_table('test_ttl'); + ++-------------------------------+ +| ADMIN flush_table('test_ttl') | ++-------------------------------+ +| 0 | ++-------------------------------+ + +ADMIN compact_table('test_ttl'); + ++---------------------------------+ +| ADMIN compact_table('test_ttl') | ++---------------------------------+ +| 0 | ++---------------------------------+ + +SELECT + val +from + test_ttl; + +++ +++ + +ALTER TABLE test_ttl UNSET 'ttl'; + +Affected Rows: 0 + INSERT INTO test_ttl VALUES (now(), 1), (now(), 2), - (now(), 3), - (now() + INTERVAL '1 SECOND', 4), - (now() - INTERVAL '1 SECOND', 5); + (now(), 3); + +Affected Rows: 3 + +SELECT + val +from + test_ttl; + ++-----+ +| val | ++-----+ +| 1 | +| 2 | +| 3 | ++-----+ + +DROP TABLE test_ttl; + +Affected Rows: 0 + +CREATE TABLE test_ttl( + ts TIMESTAMP TIME INDEX, + val INT, + PRIMARY KEY (`val`) +) WITH (ttl = '1s'); Affected Rows: 0 +SHOW CREATE TABLE test_ttl; + ++----------+-----------------------------------------+ +| Table | Create Table | ++----------+-----------------------------------------+ +| test_ttl | CREATE TABLE IF NOT EXISTS "test_ttl" ( | +| | "ts" TIMESTAMP(3) NOT NULL, | +| | "val" INT NULL, | +| | TIME INDEX ("ts"), | +| | PRIMARY KEY ("val") | +| | ) | +| | | +| | ENGINE=mito | +| | WITH( | +| | ttl = '1s' | +| | ) | ++----------+-----------------------------------------+ + +INSERT INTO + test_ttl +VALUES + (now(), 1), + (now(), 2), + (now(), 3); + +Affected Rows: 3 + +SELECT + val +from + test_ttl; + ++-----+ +| val | ++-----+ +| 1 | +| 2 | +| 3 | ++-----+ + +ADMIN flush_table('test_ttl'); + ++-------------------------------+ +| ADMIN flush_table('test_ttl') | ++-------------------------------+ +| 0 | ++-------------------------------+ + +ADMIN compact_table('test_ttl'); + ++---------------------------------+ +| ADMIN compact_table('test_ttl') | ++---------------------------------+ +| 0 | ++---------------------------------+ + +SELECT + val +from + test_ttl; + ++-----+ +| val | ++-----+ +| 1 | +| 2 | +| 3 | ++-----+ + +-- SQLNESS SLEEP 2s +ADMIN flush_table('test_ttl'); + ++-------------------------------+ +| ADMIN flush_table('test_ttl') | ++-------------------------------+ +| 0 | ++-------------------------------+ + +ADMIN compact_table('test_ttl'); + ++---------------------------------+ +| ADMIN compact_table('test_ttl') | ++---------------------------------+ +| 0 | ++---------------------------------+ + +SELECT + val +from + test_ttl; + +++ +++ + +ALTER TABLE + test_ttl +SET + ttl = '1d'; + +Affected Rows: 0 + +INSERT INTO + test_ttl +VALUES + (now(), 1), + (now(), 2), + (now(), 3); + +Affected Rows: 3 + +SELECT + val +from + test_ttl; + ++-----+ +| val | ++-----+ +| 1 | +| 2 | +| 3 | ++-----+ + +ALTER TABLE + test_ttl +SET + ttl = 'instant'; + +Affected Rows: 0 + +ADMIN flush_table('test_ttl'); + ++-------------------------------+ +| ADMIN flush_table('test_ttl') | ++-------------------------------+ +| 0 | ++-------------------------------+ + +ADMIN compact_table('test_ttl'); + ++---------------------------------+ +| ADMIN compact_table('test_ttl') | ++---------------------------------+ +| 0 | ++---------------------------------+ + +SELECT + val +from + test_ttl; + +++ +++ + +ALTER TABLE + test_ttl +SET + ttl = '1s'; + +Affected Rows: 0 + +INSERT INTO + test_ttl +VALUES + (now(), 1), + (now(), 2), + (now(), 3); + +Affected Rows: 3 + +SELECT + val +from + test_ttl; + ++-----+ +| val | ++-----+ +| 1 | +| 2 | +| 3 | ++-----+ + -- SQLNESS SLEEP 2s ADMIN flush_table('test_ttl'); diff --git a/tests/cases/standalone/common/ttl/ttl_instant.sql b/tests/cases/standalone/common/ttl/ttl_instant.sql index 8f503d01de40..2fc6de96fb65 100644 --- a/tests/cases/standalone/common/ttl/ttl_instant.sql +++ b/tests/cases/standalone/common/ttl/ttl_instant.sql @@ -1,4 +1,8 @@ -CREATE TABLE test_ttl(ts TIMESTAMP TIME INDEX, val INT) WITH (ttl = 'instant'); +CREATE TABLE test_ttl( + ts TIMESTAMP TIME INDEX, + val INT, + PRIMARY KEY (`val`) +) WITH (ttl = 'instant'); SHOW CREATE TABLE test_ttl; @@ -7,23 +11,126 @@ INSERT INTO VALUES (now(), 1), (now(), 2), - (now(), 3), - (now() + INTERVAL '1 SECOND', 4), - (now() - INTERVAL '1 SECOND', 5); + (now(), 3); SELECT val from test_ttl; +-- SQLNESS SLEEP 2s +ADMIN flush_table('test_ttl'); + +ADMIN compact_table('test_ttl'); + +SELECT + val +from + test_ttl; + +ALTER TABLE test_ttl UNSET 'ttl'; + +INSERT INTO + test_ttl +VALUES + (now(), 1), + (now(), 2), + (now(), 3); + +SELECT + val +from + test_ttl; + +DROP TABLE test_ttl; + +CREATE TABLE test_ttl( + ts TIMESTAMP TIME INDEX, + val INT, + PRIMARY KEY (`val`) +) WITH (ttl = '1s'); + +SHOW CREATE TABLE test_ttl; + +INSERT INTO + test_ttl +VALUES + (now(), 1), + (now(), 2), + (now(), 3); + +SELECT + val +from + test_ttl; + +ADMIN flush_table('test_ttl'); + +ADMIN compact_table('test_ttl'); + +SELECT + val +from + test_ttl; + +-- SQLNESS SLEEP 2s +ADMIN flush_table('test_ttl'); + +ADMIN compact_table('test_ttl'); + +SELECT + val +from + test_ttl; + +ALTER TABLE + test_ttl +SET + ttl = '1d'; + +INSERT INTO + test_ttl +VALUES + (now(), 1), + (now(), 2), + (now(), 3); + +SELECT + val +from + test_ttl; + +ALTER TABLE + test_ttl +SET + ttl = 'instant'; + +ADMIN flush_table('test_ttl'); + +ADMIN compact_table('test_ttl'); + + +SELECT + val +from + test_ttl; + +ALTER TABLE + test_ttl +SET + ttl = '1s'; + INSERT INTO test_ttl VALUES (now(), 1), (now(), 2), - (now(), 3), - (now() + INTERVAL '1 SECOND', 4), - (now() - INTERVAL '1 SECOND', 5); + (now(), 3); + +SELECT + val +from + test_ttl; -- SQLNESS SLEEP 2s ADMIN flush_table('test_ttl'); From 5399c3241b470334e70f4cc161dc0b3dfaa32928 Mon Sep 17 00:00:00 2001 From: discord9 Date: Fri, 6 Dec 2024 11:53:48 +0800 Subject: [PATCH 18/21] refactor: per review --- Cargo.lock | 4 -- src/common/base/Cargo.toml | 3 -- src/common/meta/src/key/schema_name.rs | 4 +- src/common/time/src/error.rs | 8 ++++ src/common/time/src/ttl.rs | 23 ++------- src/operator/src/insert.rs | 8 ++-- .../src/req_convert/insert/row_to_region.rs | 9 ++-- .../src/req_convert/insert/stmt_to_region.rs | 16 +++---- .../src/req_convert/insert/table_to_region.rs | 11 +++-- src/query/src/sql/show_create_table.rs | 4 +- src/session/Cargo.toml | 1 - src/table/src/requests.rs | 4 +- .../standalone/common/flow/flow_basic.result | 10 ++++ .../standalone/common/flow/flow_basic.sql | 2 + .../standalone/common/ttl/ttl_instant.result | 44 ++++++++++++----- .../standalone/common/ttl/ttl_instant.sql | 47 ++++++++++++++----- 16 files changed, 123 insertions(+), 75 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d476f42179b8..ccec5ef0243b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1899,12 +1899,9 @@ dependencies = [ "common-macro", "common-test-util", "futures", - "humantime", - "humantime-serde", "paste", "pin-project", "serde", - "serde_json", "snafu 0.8.5", "tokio", "toml 0.8.19", @@ -10915,7 +10912,6 @@ dependencies = [ name = "session" version = "0.11.0" dependencies = [ - "ahash 0.8.11", "api", "arc-swap", "auth", diff --git a/src/common/base/Cargo.toml b/src/common/base/Cargo.toml index 2631e6f56c1a..465599974dae 100644 --- a/src/common/base/Cargo.toml +++ b/src/common/base/Cargo.toml @@ -15,8 +15,6 @@ bytes.workspace = true common-error.workspace = true common-macro.workspace = true futures.workspace = true -humantime.workspace = true -humantime-serde.workspace = true paste = "1.0" pin-project.workspace = true serde = { version = "1.0", features = ["derive"] } @@ -26,5 +24,4 @@ zeroize = { version = "1.6", default-features = false, features = ["alloc"] } [dev-dependencies] common-test-util.workspace = true -serde_json.workspace = true toml.workspace = true diff --git a/src/common/meta/src/key/schema_name.rs b/src/common/meta/src/key/schema_name.rs index 1e144b0a246b..18c653ee01b3 100644 --- a/src/common/meta/src/key/schema_name.rs +++ b/src/common/meta/src/key/schema_name.rs @@ -62,7 +62,7 @@ pub struct SchemaNameValue { impl Display for SchemaNameValue { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - if let Some(ttl) = self.ttl.and_then(|i| i.to_string_opt()) { + if let Some(ttl) = self.ttl.map(|i| i.to_string()) { write!(f, "ttl='{}'", ttl)?; } @@ -94,7 +94,7 @@ impl TryFrom<&HashMap> for SchemaNameValue { impl From for HashMap { fn from(value: SchemaNameValue) -> Self { let mut opts = HashMap::new(); - if let Some(ttl) = value.ttl.and_then(|ttl| ttl.to_string_opt()) { + if let Some(ttl) = value.ttl.map(|ttl| ttl.to_string()) { opts.insert(OPT_KEY_TTL.to_string(), ttl); } opts diff --git a/src/common/time/src/error.rs b/src/common/time/src/error.rs index 45d94a782885..437521b43b2f 100644 --- a/src/common/time/src/error.rs +++ b/src/common/time/src/error.rs @@ -93,12 +93,20 @@ pub enum Error { #[snafu(implicit)] location: Location, }, + + #[snafu(display("Failed to parse duration: {raw:?}"))] + ParseDuration { + raw: humantime::DurationError, + #[snafu(implicit)] + location: Location, + }, } impl ErrorExt for Error { fn status_code(&self) -> StatusCode { match self { Error::ParseDateStr { .. } + | Error::ParseDuration { .. } | Error::ParseTimestamp { .. } | Error::InvalidTimezoneOffset { .. } | Error::Format { .. } diff --git a/src/common/time/src/ttl.rs b/src/common/time/src/ttl.rs index d4ad377fd1e3..94e9a97ba548 100644 --- a/src/common/time/src/ttl.rs +++ b/src/common/time/src/ttl.rs @@ -17,6 +17,7 @@ use std::time::Duration; use serde::{Deserialize, Serialize}; +use crate::error::{Error, ParseDurationSnafu}; use crate::Timestamp; pub const INSTANT: &str = "instant"; @@ -50,12 +51,13 @@ impl TimeToLive { /// Parse a string that is either `instant`, `forever`, or a duration to `TimeToLive` /// /// note that an empty string or a zero duration(a duration that spans no time) is treat as `forever` too - pub fn from_humantime_or_str(s: &str) -> Result { + pub fn from_humantime_or_str(s: &str) -> Result { match s.to_lowercase().as_ref() { INSTANT => Ok(TimeToLive::Instant), FOREVER | "" => Ok(TimeToLive::Forever), _ => { - let d = humantime::parse_duration(s).map_err(|e| e.to_string())?; + let d = humantime::parse_duration(s) + .map_err(|e| ParseDurationSnafu { raw: e }.build())?; Ok(TimeToLive::from(d)) } } @@ -75,15 +77,6 @@ impl TimeToLive { }) } - /// Print TimeToLive as string - pub fn to_string_opt(&self) -> Option { - match self { - TimeToLive::Instant => Some(INSTANT.to_string()), - TimeToLive::Duration(d) => Some(humantime::format_duration(*d).to_string()), - TimeToLive::Forever => Some(FOREVER.to_string()), - } - } - /// is instant variant pub fn is_instant(&self) -> bool { matches!(self, TimeToLive::Instant) @@ -93,14 +86,6 @@ impl TimeToLive { pub fn is_forever(&self) -> bool { matches!(self, TimeToLive::Forever) } - - /// Get duration if it is a duration variant, otherwise return None - pub fn get_duration(&self) -> Option { - match self { - TimeToLive::Duration(d) => Some(*d), - _ => None, - } - } } impl From for TimeToLive { diff --git a/src/operator/src/insert.rs b/src/operator/src/insert.rs index 205d3dcf7a0a..8c7e68aca33f 100644 --- a/src/operator/src/insert.rs +++ b/src/operator/src/insert.rs @@ -101,7 +101,7 @@ impl AutoCreateTableType { /// /// This is used to split requests for different processing. #[derive(Clone)] -pub struct InstantInsertRequests { +pub struct InstantOrNormalInsertRequests { /// Requests with normal ttl. pub normal_requests: RegionInsertRequests, /// Requests with ttl=instant. @@ -293,7 +293,7 @@ impl Inserter { impl Inserter { async fn do_request( &self, - requests: InstantInsertRequests, + requests: InstantOrNormalInsertRequests, ctx: &QueryContextRef, ) -> Result { let write_cost = write_meter!( @@ -308,7 +308,7 @@ impl Inserter { ..Default::default() }); - let InstantInsertRequests { + let InstantOrNormalInsertRequests { normal_requests, instant_requests, } = requests; @@ -589,6 +589,8 @@ impl Inserter { AutoCreateTableType::Physical | AutoCreateTableType::Log | AutoCreateTableType::LastNonNull => { + // note that auto create table shouldn't be ttl instant table + // for it's a very unexpected behavior and should be set by user explicitly for create_table in create_tables { let table = self .create_physical_table(create_table, ctx, statement_executor) diff --git a/src/operator/src/req_convert/insert/row_to_region.rs b/src/operator/src/req_convert/insert/row_to_region.rs index e1aa09d0cd10..1dadc4ee0eb0 100644 --- a/src/operator/src/req_convert/insert/row_to_region.rs +++ b/src/operator/src/req_convert/insert/row_to_region.rs @@ -20,7 +20,7 @@ use snafu::OptionExt; use table::metadata::TableId; use crate::error::{Result, TableNotFoundSnafu}; -use crate::insert::InstantInsertRequests; +use crate::insert::InstantOrNormalInsertRequests; use crate::req_convert::common::partitioner::Partitioner; pub struct RowToRegion<'a> { @@ -42,7 +42,10 @@ impl<'a> RowToRegion<'a> { } } - pub async fn convert(&self, requests: RowInsertRequests) -> Result { + pub async fn convert( + &self, + requests: RowInsertRequests, + ) -> Result { let mut region_request = Vec::with_capacity(requests.inserts.len()); let mut instant_request = Vec::with_capacity(requests.inserts.len()); for request in requests.inserts { @@ -57,7 +60,7 @@ impl<'a> RowToRegion<'a> { } } - Ok(InstantInsertRequests { + Ok(InstantOrNormalInsertRequests { normal_requests: RegionInsertRequests { requests: region_request, }, diff --git a/src/operator/src/req_convert/insert/stmt_to_region.rs b/src/operator/src/req_convert/insert/stmt_to_region.rs index b15aa43e1506..4370dc2079c6 100644 --- a/src/operator/src/req_convert/insert/stmt_to_region.rs +++ b/src/operator/src/req_convert/insert/stmt_to_region.rs @@ -32,7 +32,7 @@ use crate::error::{ ColumnNotFoundSnafu, InvalidSqlSnafu, MissingInsertBodySnafu, ParseSqlSnafu, Result, SchemaReadOnlySnafu, TableNotFoundSnafu, }; -use crate::insert::InstantInsertRequests; +use crate::insert::InstantOrNormalInsertRequests; use crate::req_convert::common::partitioner::Partitioner; use crate::req_convert::insert::semantic_type; @@ -61,7 +61,7 @@ impl<'a> StatementToRegion<'a> { &self, stmt: &Insert, query_ctx: &QueryContextRef, - ) -> Result { + ) -> Result { let (catalog, schema, table_name) = self.get_full_name(stmt.table_name())?; let table = self.get_table(&catalog, &schema, &table_name).await?; let table_schema = table.schema(); @@ -136,17 +136,17 @@ impl<'a> StatementToRegion<'a> { .partition_insert_requests(table_info.table_id(), Rows { schema, rows }) .await?; let requests = RegionInsertRequests { requests }; - Ok(if table_info.is_ttl_instant_table() { - InstantInsertRequests { + if table_info.is_ttl_instant_table() { + Ok(InstantOrNormalInsertRequests { normal_requests: Default::default(), instant_requests: requests, - } + }) } else { - InstantInsertRequests { + Ok(InstantOrNormalInsertRequests { normal_requests: requests, instant_requests: Default::default(), - } - }) + }) + } } async fn get_table(&self, catalog: &str, schema: &str, table: &str) -> Result { diff --git a/src/operator/src/req_convert/insert/table_to_region.rs b/src/operator/src/req_convert/insert/table_to_region.rs index d00f6992f0a2..2d6fd5af94e8 100644 --- a/src/operator/src/req_convert/insert/table_to_region.rs +++ b/src/operator/src/req_convert/insert/table_to_region.rs @@ -19,7 +19,7 @@ use table::metadata::TableInfo; use table::requests::InsertRequest as TableInsertRequest; use crate::error::Result; -use crate::insert::InstantInsertRequests; +use crate::insert::InstantOrNormalInsertRequests; use crate::req_convert::common::partitioner::Partitioner; use crate::req_convert::common::{column_schema, row_count}; @@ -36,7 +36,10 @@ impl<'a> TableToRegion<'a> { } } - pub async fn convert(&self, request: TableInsertRequest) -> Result { + pub async fn convert( + &self, + request: TableInsertRequest, + ) -> Result { let row_count = row_count(&request.columns_values)?; let schema = column_schema(self.table_info, &request.columns_values)?; let rows = api::helper::vectors_to_rows(request.columns_values.values(), row_count); @@ -48,12 +51,12 @@ impl<'a> TableToRegion<'a> { let requests = RegionInsertRequests { requests }; if self.table_info.is_ttl_instant_table() { - Ok(InstantInsertRequests { + Ok(InstantOrNormalInsertRequests { normal_requests: Default::default(), instant_requests: requests, }) } else { - Ok(InstantInsertRequests { + Ok(InstantOrNormalInsertRequests { normal_requests: requests, instant_requests: Default::default(), }) diff --git a/src/query/src/sql/show_create_table.rs b/src/query/src/sql/show_create_table.rs index ea39dc1aa5f2..17f1dcdd394b 100644 --- a/src/query/src/sql/show_create_table.rs +++ b/src/query/src/sql/show_create_table.rs @@ -45,11 +45,11 @@ fn create_sql_options(table_meta: &TableMeta, schema_options: Option for HashMap { write_buffer_size.to_string(), ); } - if let Some(ttl_str) = opts.ttl.and_then(|ttl| ttl.to_string_opt()) { + if let Some(ttl_str) = opts.ttl.map(|ttl| ttl.to_string()) { let _ = res.insert(TTL_KEY.to_string(), ttl_str); } res.extend( diff --git a/tests/cases/standalone/common/flow/flow_basic.result b/tests/cases/standalone/common/flow/flow_basic.result index 237cd0493495..8ee6a90c83bf 100644 --- a/tests/cases/standalone/common/flow/flow_basic.result +++ b/tests/cases/standalone/common/flow/flow_basic.result @@ -338,6 +338,16 @@ where Affected Rows: 0 +SHOW CREATE FLOW filter_numbers_basic; + ++----------------------+----------------------------------------------------------------------------------------------------------------------------------------------+ +| Flow | Create Flow | ++----------------------+----------------------------------------------------------------------------------------------------------------------------------------------+ +| filter_numbers_basic | CREATE FLOW IF NOT EXISTS filter_numbers_basic | +| | SINK TO out_num_cnt_basic | +| | AS SELECT INTERVAL '1 day 1 second', INTERVAL '1 month 1 day 1 second', INTERVAL '1 year 1 month' FROM numbers_input_basic WHERE number > 10 | ++----------------------+----------------------------------------------------------------------------------------------------------------------------------------------+ + drop flow filter_numbers_basic; Affected Rows: 0 diff --git a/tests/cases/standalone/common/flow/flow_basic.sql b/tests/cases/standalone/common/flow/flow_basic.sql index 5accb8ab5836..43a42de4dd5f 100644 --- a/tests/cases/standalone/common/flow/flow_basic.sql +++ b/tests/cases/standalone/common/flow/flow_basic.sql @@ -182,6 +182,8 @@ FROM where number > 10; +SHOW CREATE FLOW filter_numbers_basic; + drop flow filter_numbers_basic; drop table out_num_cnt_basic; diff --git a/tests/cases/standalone/common/ttl/ttl_instant.result b/tests/cases/standalone/common/ttl/ttl_instant.result index e8d74f4c4bdf..bfb023f482b7 100644 --- a/tests/cases/standalone/common/ttl/ttl_instant.result +++ b/tests/cases/standalone/common/ttl/ttl_instant.result @@ -36,7 +36,9 @@ Affected Rows: 0 SELECT val from - test_ttl; + test_ttl +ORDER BY + val; ++ ++ @@ -61,12 +63,15 @@ ADMIN compact_table('test_ttl'); SELECT val from - test_ttl; + test_ttl +ORDER BY + val; ++ ++ -ALTER TABLE test_ttl UNSET 'ttl'; +ALTER TABLE + test_ttl UNSET 'ttl'; Affected Rows: 0 @@ -82,7 +87,9 @@ Affected Rows: 3 SELECT val from - test_ttl; + test_ttl +ORDER BY + val; +-----+ | val | @@ -134,7 +141,9 @@ Affected Rows: 3 SELECT val from - test_ttl; + test_ttl +ORDER BY + val; +-----+ | val | @@ -163,7 +172,9 @@ ADMIN compact_table('test_ttl'); SELECT val from - test_ttl; + test_ttl +ORDER BY + val; +-----+ | val | @@ -193,7 +204,9 @@ ADMIN compact_table('test_ttl'); SELECT val from - test_ttl; + test_ttl +ORDER BY + val; ++ ++ @@ -217,7 +230,9 @@ Affected Rows: 3 SELECT val from - test_ttl; + test_ttl +ORDER BY + val; +-----+ | val | @@ -253,11 +268,14 @@ ADMIN compact_table('test_ttl'); SELECT val from - test_ttl; + test_ttl +ORDER BY + val; ++ ++ +-- to makesure alter back and forth from duration to/from instant wouldn't break anything ALTER TABLE test_ttl SET @@ -277,7 +295,9 @@ Affected Rows: 3 SELECT val from - test_ttl; + test_ttl +ORDER BY + val; +-----+ | val | @@ -307,7 +327,9 @@ ADMIN compact_table('test_ttl'); SELECT val from - test_ttl; + test_ttl +ORDER BY + val; ++ ++ diff --git a/tests/cases/standalone/common/ttl/ttl_instant.sql b/tests/cases/standalone/common/ttl/ttl_instant.sql index 2fc6de96fb65..c13516b93f43 100644 --- a/tests/cases/standalone/common/ttl/ttl_instant.sql +++ b/tests/cases/standalone/common/ttl/ttl_instant.sql @@ -16,7 +16,9 @@ VALUES SELECT val from - test_ttl; + test_ttl +ORDER BY + val; -- SQLNESS SLEEP 2s ADMIN flush_table('test_ttl'); @@ -26,9 +28,12 @@ ADMIN compact_table('test_ttl'); SELECT val from - test_ttl; + test_ttl +ORDER BY + val; -ALTER TABLE test_ttl UNSET 'ttl'; +ALTER TABLE + test_ttl UNSET 'ttl'; INSERT INTO test_ttl @@ -40,7 +45,9 @@ VALUES SELECT val from - test_ttl; + test_ttl +ORDER BY + val; DROP TABLE test_ttl; @@ -62,7 +69,9 @@ VALUES SELECT val from - test_ttl; + test_ttl +ORDER BY + val; ADMIN flush_table('test_ttl'); @@ -71,7 +80,9 @@ ADMIN compact_table('test_ttl'); SELECT val from - test_ttl; + test_ttl +ORDER BY + val; -- SQLNESS SLEEP 2s ADMIN flush_table('test_ttl'); @@ -81,7 +92,9 @@ ADMIN compact_table('test_ttl'); SELECT val from - test_ttl; + test_ttl +ORDER BY + val; ALTER TABLE test_ttl @@ -98,7 +111,9 @@ VALUES SELECT val from - test_ttl; + test_ttl +ORDER BY + val; ALTER TABLE test_ttl @@ -109,12 +124,14 @@ ADMIN flush_table('test_ttl'); ADMIN compact_table('test_ttl'); - SELECT val from - test_ttl; + test_ttl +ORDER BY + val; +-- to makesure alter back and forth from duration to/from instant wouldn't break anything ALTER TABLE test_ttl SET @@ -130,7 +147,9 @@ VALUES SELECT val from - test_ttl; + test_ttl +ORDER BY + val; -- SQLNESS SLEEP 2s ADMIN flush_table('test_ttl'); @@ -140,6 +159,8 @@ ADMIN compact_table('test_ttl'); SELECT val from - test_ttl; + test_ttl +ORDER BY + val; -DROP TABLE test_ttl; \ No newline at end of file +DROP TABLE test_ttl; From 1f7872d149c1e556557e339638d37dfe034988cc Mon Sep 17 00:00:00 2001 From: discord9 Date: Fri, 6 Dec 2024 14:39:43 +0800 Subject: [PATCH 19/21] chore: per review --- src/common/time/src/error.rs | 5 +++-- src/common/time/src/ttl.rs | 4 ++-- src/operator/src/insert.rs | 6 +++--- src/operator/src/req_convert/insert/row_to_region.rs | 6 +++--- src/operator/src/req_convert/insert/stmt_to_region.rs | 8 ++++---- src/operator/src/req_convert/insert/table_to_region.rs | 8 ++++---- tests/cases/standalone/common/ttl/ttl_instant.result | 2 +- tests/cases/standalone/common/ttl/ttl_instant.sql | 2 +- 8 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/common/time/src/error.rs b/src/common/time/src/error.rs index 437521b43b2f..abb6378c029f 100644 --- a/src/common/time/src/error.rs +++ b/src/common/time/src/error.rs @@ -94,9 +94,10 @@ pub enum Error { location: Location, }, - #[snafu(display("Failed to parse duration: {raw:?}"))] + #[snafu(display("Failed to parse duration"))] ParseDuration { - raw: humantime::DurationError, + #[snafu(source)] + error: humantime::DurationError, #[snafu(implicit)] location: Location, }, diff --git a/src/common/time/src/ttl.rs b/src/common/time/src/ttl.rs index 94e9a97ba548..45a6b61ce3e3 100644 --- a/src/common/time/src/ttl.rs +++ b/src/common/time/src/ttl.rs @@ -16,6 +16,7 @@ use std::fmt::Display; use std::time::Duration; use serde::{Deserialize, Serialize}; +use snafu::ResultExt; use crate::error::{Error, ParseDurationSnafu}; use crate::Timestamp; @@ -56,8 +57,7 @@ impl TimeToLive { INSTANT => Ok(TimeToLive::Instant), FOREVER | "" => Ok(TimeToLive::Forever), _ => { - let d = humantime::parse_duration(s) - .map_err(|e| ParseDurationSnafu { raw: e }.build())?; + let d = humantime::parse_duration(s).context(ParseDurationSnafu)?; Ok(TimeToLive::from(d)) } } diff --git a/src/operator/src/insert.rs b/src/operator/src/insert.rs index 8c7e68aca33f..ec01b329457f 100644 --- a/src/operator/src/insert.rs +++ b/src/operator/src/insert.rs @@ -101,7 +101,7 @@ impl AutoCreateTableType { /// /// This is used to split requests for different processing. #[derive(Clone)] -pub struct InstantOrNormalInsertRequests { +pub struct InstantAndNormalInsertRequests { /// Requests with normal ttl. pub normal_requests: RegionInsertRequests, /// Requests with ttl=instant. @@ -293,7 +293,7 @@ impl Inserter { impl Inserter { async fn do_request( &self, - requests: InstantOrNormalInsertRequests, + requests: InstantAndNormalInsertRequests, ctx: &QueryContextRef, ) -> Result { let write_cost = write_meter!( @@ -308,7 +308,7 @@ impl Inserter { ..Default::default() }); - let InstantOrNormalInsertRequests { + let InstantAndNormalInsertRequests { normal_requests, instant_requests, } = requests; diff --git a/src/operator/src/req_convert/insert/row_to_region.rs b/src/operator/src/req_convert/insert/row_to_region.rs index 1dadc4ee0eb0..125910ba455f 100644 --- a/src/operator/src/req_convert/insert/row_to_region.rs +++ b/src/operator/src/req_convert/insert/row_to_region.rs @@ -20,7 +20,7 @@ use snafu::OptionExt; use table::metadata::TableId; use crate::error::{Result, TableNotFoundSnafu}; -use crate::insert::InstantOrNormalInsertRequests; +use crate::insert::InstantAndNormalInsertRequests; use crate::req_convert::common::partitioner::Partitioner; pub struct RowToRegion<'a> { @@ -45,7 +45,7 @@ impl<'a> RowToRegion<'a> { pub async fn convert( &self, requests: RowInsertRequests, - ) -> Result { + ) -> Result { let mut region_request = Vec::with_capacity(requests.inserts.len()); let mut instant_request = Vec::with_capacity(requests.inserts.len()); for request in requests.inserts { @@ -60,7 +60,7 @@ impl<'a> RowToRegion<'a> { } } - Ok(InstantOrNormalInsertRequests { + Ok(InstantAndNormalInsertRequests { normal_requests: RegionInsertRequests { requests: region_request, }, diff --git a/src/operator/src/req_convert/insert/stmt_to_region.rs b/src/operator/src/req_convert/insert/stmt_to_region.rs index 4370dc2079c6..cd48b4fca54e 100644 --- a/src/operator/src/req_convert/insert/stmt_to_region.rs +++ b/src/operator/src/req_convert/insert/stmt_to_region.rs @@ -32,7 +32,7 @@ use crate::error::{ ColumnNotFoundSnafu, InvalidSqlSnafu, MissingInsertBodySnafu, ParseSqlSnafu, Result, SchemaReadOnlySnafu, TableNotFoundSnafu, }; -use crate::insert::InstantOrNormalInsertRequests; +use crate::insert::InstantAndNormalInsertRequests; use crate::req_convert::common::partitioner::Partitioner; use crate::req_convert::insert::semantic_type; @@ -61,7 +61,7 @@ impl<'a> StatementToRegion<'a> { &self, stmt: &Insert, query_ctx: &QueryContextRef, - ) -> Result { + ) -> Result { let (catalog, schema, table_name) = self.get_full_name(stmt.table_name())?; let table = self.get_table(&catalog, &schema, &table_name).await?; let table_schema = table.schema(); @@ -137,12 +137,12 @@ impl<'a> StatementToRegion<'a> { .await?; let requests = RegionInsertRequests { requests }; if table_info.is_ttl_instant_table() { - Ok(InstantOrNormalInsertRequests { + Ok(InstantAndNormalInsertRequests { normal_requests: Default::default(), instant_requests: requests, }) } else { - Ok(InstantOrNormalInsertRequests { + Ok(InstantAndNormalInsertRequests { normal_requests: requests, instant_requests: Default::default(), }) diff --git a/src/operator/src/req_convert/insert/table_to_region.rs b/src/operator/src/req_convert/insert/table_to_region.rs index 2d6fd5af94e8..ac79cce503ed 100644 --- a/src/operator/src/req_convert/insert/table_to_region.rs +++ b/src/operator/src/req_convert/insert/table_to_region.rs @@ -19,7 +19,7 @@ use table::metadata::TableInfo; use table::requests::InsertRequest as TableInsertRequest; use crate::error::Result; -use crate::insert::InstantOrNormalInsertRequests; +use crate::insert::InstantAndNormalInsertRequests; use crate::req_convert::common::partitioner::Partitioner; use crate::req_convert::common::{column_schema, row_count}; @@ -39,7 +39,7 @@ impl<'a> TableToRegion<'a> { pub async fn convert( &self, request: TableInsertRequest, - ) -> Result { + ) -> Result { let row_count = row_count(&request.columns_values)?; let schema = column_schema(self.table_info, &request.columns_values)?; let rows = api::helper::vectors_to_rows(request.columns_values.values(), row_count); @@ -51,12 +51,12 @@ impl<'a> TableToRegion<'a> { let requests = RegionInsertRequests { requests }; if self.table_info.is_ttl_instant_table() { - Ok(InstantOrNormalInsertRequests { + Ok(InstantAndNormalInsertRequests { normal_requests: Default::default(), instant_requests: requests, }) } else { - Ok(InstantOrNormalInsertRequests { + Ok(InstantAndNormalInsertRequests { normal_requests: requests, instant_requests: Default::default(), }) diff --git a/tests/cases/standalone/common/ttl/ttl_instant.result b/tests/cases/standalone/common/ttl/ttl_instant.result index bfb023f482b7..57f12e01ddc7 100644 --- a/tests/cases/standalone/common/ttl/ttl_instant.result +++ b/tests/cases/standalone/common/ttl/ttl_instant.result @@ -275,7 +275,7 @@ ORDER BY ++ ++ --- to makesure alter back and forth from duration to/from instant wouldn't break anything +-- to make sure alter back and forth from duration to/from instant wouldn't break anything ALTER TABLE test_ttl SET diff --git a/tests/cases/standalone/common/ttl/ttl_instant.sql b/tests/cases/standalone/common/ttl/ttl_instant.sql index c13516b93f43..b76128ccdf0b 100644 --- a/tests/cases/standalone/common/ttl/ttl_instant.sql +++ b/tests/cases/standalone/common/ttl/ttl_instant.sql @@ -131,7 +131,7 @@ from ORDER BY val; --- to makesure alter back and forth from duration to/from instant wouldn't break anything +-- to make sure alter back and forth from duration to/from instant wouldn't break anything ALTER TABLE test_ttl SET From 2a77e9c5e83abf5ecc4db77b03a9b300573447ab Mon Sep 17 00:00:00 2001 From: discord9 Date: Fri, 6 Dec 2024 15:54:19 +0800 Subject: [PATCH 20/21] feat: add db ttl type&forbid instant for db --- src/common/meta/src/key/schema_name.rs | 27 ++-- src/common/meta/src/rpc/ddl.rs | 6 +- src/common/time/src/error.rs | 7 ++ src/common/time/src/lib.rs | 2 +- src/common/time/src/ttl.rs | 115 +++++++++++++++++- src/mito2/src/compaction.rs | 3 +- .../standalone/common/ttl/show_ttl.result | 19 ++- .../cases/standalone/common/ttl/show_ttl.sql | 9 +- 8 files changed, 159 insertions(+), 29 deletions(-) diff --git a/src/common/meta/src/key/schema_name.rs b/src/common/meta/src/key/schema_name.rs index 18c653ee01b3..35413433a445 100644 --- a/src/common/meta/src/key/schema_name.rs +++ b/src/common/meta/src/key/schema_name.rs @@ -17,7 +17,7 @@ use std::fmt::Display; use std::sync::Arc; use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; -use common_time::TimeToLive; +use common_time::DatabaseTimeToLive; use futures::stream::BoxStream; use humantime_serde::re::humantime; use serde::{Deserialize, Serialize}; @@ -57,7 +57,7 @@ impl Default for SchemaNameKey<'_> { #[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)] pub struct SchemaNameValue { #[serde(default)] - pub ttl: Option, + pub ttl: Option, } impl Display for SchemaNameValue { @@ -310,8 +310,6 @@ impl<'a> From<&'a SchemaName> for SchemaNameKey<'a> { mod tests { use std::time::Duration; - use common_time::INSTANT; - use super::*; use crate::kv_backend::memory::MemoryKvBackend; @@ -329,11 +327,6 @@ mod tests { ttl: Some(Duration::from_secs(0).into()), }; assert_eq!("ttl='forever'", schema_value.to_string()); - - let schema_value = SchemaNameValue { - ttl: Some(TimeToLive::Instant), - }; - assert_eq!("ttl='instant'", schema_value.to_string()); } #[test] @@ -359,17 +352,8 @@ mod tests { .unwrap(); assert_eq!(Some(value), parsed); - let instant = SchemaNameValue { - ttl: Some(TimeToLive::Instant), - }; - let parsed = SchemaNameValue::try_from_raw_value( - serde_json::json!({"ttl": INSTANT}).to_string().as_bytes(), - ) - .unwrap(); - assert_eq!(Some(instant), parsed); - let forever = SchemaNameValue { - ttl: Some(TimeToLive::default()), + ttl: Some(Default::default()), }; let parsed = SchemaNameValue::try_from_raw_value( serde_json::json!({"ttl": "forever"}).to_string().as_bytes(), @@ -377,6 +361,11 @@ mod tests { .unwrap(); assert_eq!(Some(forever), parsed); + let instant_err = SchemaNameValue::try_from_raw_value( + serde_json::json!({"ttl": "instant"}).to_string().as_bytes(), + ); + assert!(instant_err.is_err()); + let none = SchemaNameValue::try_from_raw_value("null".as_bytes()).unwrap(); assert!(none.is_none()); diff --git a/src/common/meta/src/rpc/ddl.rs b/src/common/meta/src/rpc/ddl.rs index 228ca8607582..bec12796e791 100644 --- a/src/common/meta/src/rpc/ddl.rs +++ b/src/common/meta/src/rpc/ddl.rs @@ -35,7 +35,7 @@ use api::v1::{ }; use base64::engine::general_purpose; use base64::Engine as _; -use common_time::TimeToLive; +use common_time::DatabaseTimeToLive; use prost::Message; use serde::{Deserialize, Serialize}; use serde_with::{serde_as, DefaultOnNull}; @@ -1008,7 +1008,7 @@ impl TryFrom for SetDatabaseOption { fn try_from(PbOption { key, value }: PbOption) -> Result { match key.to_ascii_lowercase().as_str() { TTL_KEY => { - let ttl = TimeToLive::from_humantime_or_str(&value) + let ttl = DatabaseTimeToLive::from_humantime_or_str(&value) .map_err(|_| InvalidSetDatabaseOptionSnafu { key, value }.build())?; Ok(SetDatabaseOption::Ttl(ttl)) @@ -1020,7 +1020,7 @@ impl TryFrom for SetDatabaseOption { #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] pub enum SetDatabaseOption { - Ttl(TimeToLive), + Ttl(DatabaseTimeToLive), } #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] diff --git a/src/common/time/src/error.rs b/src/common/time/src/error.rs index abb6378c029f..0f6b5bdeb999 100644 --- a/src/common/time/src/error.rs +++ b/src/common/time/src/error.rs @@ -101,6 +101,12 @@ pub enum Error { #[snafu(implicit)] location: Location, }, + + #[snafu(display("Database's TTL can't be `instant`"))] + InvalidDatabaseTtl { + #[snafu(implicit)] + location: Location, + }, } impl ErrorExt for Error { @@ -108,6 +114,7 @@ impl ErrorExt for Error { match self { Error::ParseDateStr { .. } | Error::ParseDuration { .. } + | Error::InvalidDatabaseTtl { .. } | Error::ParseTimestamp { .. } | Error::InvalidTimezoneOffset { .. } | Error::Format { .. } diff --git a/src/common/time/src/lib.rs b/src/common/time/src/lib.rs index 8511c3520d59..feb19cf9a191 100644 --- a/src/common/time/src/lib.rs +++ b/src/common/time/src/lib.rs @@ -33,4 +33,4 @@ pub use range::RangeMillis; pub use timestamp::Timestamp; pub use timestamp_millis::TimestampMillis; pub use timezone::Timezone; -pub use ttl::{TimeToLive, FOREVER, INSTANT}; +pub use ttl::{DatabaseTimeToLive, TimeToLive, FOREVER, INSTANT}; diff --git a/src/common/time/src/ttl.rs b/src/common/time/src/ttl.rs index 45a6b61ce3e3..6e0f13ac6b39 100644 --- a/src/common/time/src/ttl.rs +++ b/src/common/time/src/ttl.rs @@ -18,12 +18,88 @@ use std::time::Duration; use serde::{Deserialize, Serialize}; use snafu::ResultExt; -use crate::error::{Error, ParseDurationSnafu}; +use crate::error::{Error, InvalidDatabaseTtlSnafu, ParseDurationSnafu}; use crate::Timestamp; pub const INSTANT: &str = "instant"; pub const FOREVER: &str = "forever"; +/// Time To Live for database, which can be `Forever`, or a `Duration`, but can't be `Instant`. +/// +/// unlike `TimeToLive` which can be `Instant`, `Forever`, or a `Duration` +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum DatabaseTimeToLive { + /// Keep the data forever + #[default] + Forever, + /// Duration to keep the data, this duration should be non-zero + #[serde(untagged, with = "humantime_serde")] + Duration(Duration), +} + +impl Display for DatabaseTimeToLive { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + DatabaseTimeToLive::Forever => write!(f, "{}", FOREVER), + DatabaseTimeToLive::Duration(d) => write!(f, "{}", humantime::Duration::from(*d)), + } + } +} + +impl DatabaseTimeToLive { + /// Parse a string that is either `forever`, or a duration to `TimeToLive` + /// + /// note that an empty string or a zero duration(a duration that spans no time) is treat as `forever` too + pub fn from_humantime_or_str(s: &str) -> Result { + let ttl = match s.to_lowercase().as_ref() { + INSTANT => InvalidDatabaseTtlSnafu.fail()?, + FOREVER | "" => Self::Forever, + _ => { + let d = humantime::parse_duration(s).context(ParseDurationSnafu)?; + Self::from(d) + } + }; + Ok(ttl) + } +} + +impl TryFrom for DatabaseTimeToLive { + type Error = Error; + fn try_from(value: TimeToLive) -> Result { + match value { + TimeToLive::Instant => InvalidDatabaseTtlSnafu.fail()?, + TimeToLive::Forever => Ok(Self::Forever), + TimeToLive::Duration(d) => Ok(Self::from(d)), + } + } +} + +impl From for TimeToLive { + fn from(value: DatabaseTimeToLive) -> Self { + match value { + DatabaseTimeToLive::Forever => TimeToLive::Forever, + DatabaseTimeToLive::Duration(d) => TimeToLive::from(d), + } + } +} + +impl From for DatabaseTimeToLive { + fn from(duration: Duration) -> Self { + if duration.is_zero() { + Self::Forever + } else { + Self::Duration(duration) + } + } +} + +impl From for DatabaseTimeToLive { + fn from(duration: humantime::Duration) -> Self { + Self::from(*duration) + } +} + /// Time To Live #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Default, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] @@ -109,6 +185,33 @@ impl From for TimeToLive { mod test { use super::*; + #[test] + fn test_db_ttl_table_ttl() { + let ttl = TimeToLive::from(Duration::from_secs(10)); + let db_ttl: DatabaseTimeToLive = ttl.try_into().unwrap(); + assert_eq!(db_ttl, DatabaseTimeToLive::from(Duration::from_secs(10))); + + // test 0 duration + let ttl = Duration::from_secs(0); + let db_ttl: DatabaseTimeToLive = ttl.into(); + assert_eq!(db_ttl, DatabaseTimeToLive::Forever); + + let ttl = DatabaseTimeToLive::from_humantime_or_str("10s").unwrap(); + let ttl: TimeToLive = ttl.into(); + assert_eq!(ttl, TimeToLive::from(Duration::from_secs(10))); + + let ttl = DatabaseTimeToLive::from_humantime_or_str("forever").unwrap(); + let ttl: TimeToLive = ttl.into(); + assert_eq!(ttl, TimeToLive::Forever); + + assert!(DatabaseTimeToLive::from_humantime_or_str("instant").is_err()); + + // test 0s + let ttl = DatabaseTimeToLive::from_humantime_or_str("0s").unwrap(); + let ttl: TimeToLive = ttl.into(); + assert_eq!(ttl, TimeToLive::Forever); + } + #[test] fn test_serde() { let cases = vec![ @@ -130,6 +233,16 @@ mod test { panic!("Actual serialized: {}, s=`{s}`, err: {:?}", serialized, err) }); assert_eq!(deserialized, expected); + + // test db ttl too + if s == "\"instant\"" { + assert!(serde_json::from_str::(s).is_err()); + continue; + } + + let db_ttl: DatabaseTimeToLive = serde_json::from_str(s).unwrap(); + let re_serialized = serde_json::to_string(&db_ttl).unwrap(); + assert_eq!(re_serialized, serialized); } } } diff --git a/src/mito2/src/compaction.rs b/src/mito2/src/compaction.rs index 2977558c3fba..31e1b0674f72 100644 --- a/src/mito2/src/compaction.rs +++ b/src/mito2/src/compaction.rs @@ -450,7 +450,8 @@ async fn find_ttl( .await .context(GetSchemaMetadataSnafu)? .and_then(|options| options.ttl) - .unwrap_or_default(); + .unwrap_or_default() + .into(); Ok(ttl) } diff --git a/tests/cases/standalone/common/ttl/show_ttl.result b/tests/cases/standalone/common/ttl/show_ttl.result index 8dd446f61dcb..d98c1b612bca 100644 --- a/tests/cases/standalone/common/ttl/show_ttl.result +++ b/tests/cases/standalone/common/ttl/show_ttl.result @@ -263,7 +263,7 @@ SHOW CREATE DATABASE test_ttl_db; ALTER DATABASE test_ttl_db SET 'ttl' = 'instant'; -Affected Rows: 0 +Error: 1004(InvalidArguments), Invalid set database option, key: ttl, value: instant SHOW CREATE TABLE test_ttl; @@ -278,7 +278,7 @@ SHOW CREATE TABLE test_ttl; | | | | | ENGINE=mito | | | WITH( | -| | ttl = 'instant' | +| | ttl = 'forever' | | | ) | +----------+-----------------------------------------+ @@ -289,7 +289,7 @@ SHOW CREATE DATABASE test_ttl_db; +-------------+-------------------------------------------+ | test_ttl_db | CREATE DATABASE IF NOT EXISTS test_ttl_db | | | WITH( | -| | ttl = 'instant' | +| | ttl = 'forever' | | | ) | +-------------+-------------------------------------------+ @@ -359,3 +359,16 @@ DROP DATABASE test_ttl_db; Affected Rows: 0 +-- test both set database to instant and alter ttl to instant for a database is forbidden +CREATE DATABASE test_ttl_db WITH (ttl = 'instant'); + +Error: 1002(Unexpected), Failed to parse value instant into key ttl + +CREATE DATABASE test_ttl_db_2 WITH (ttl = '1s'); + +Affected Rows: 1 + +ALTER DATABASE test_ttl_db_2 SET 'ttl' = 'instant'; + +Error: 1004(InvalidArguments), Invalid set database option, key: ttl, value: instant + diff --git a/tests/cases/standalone/common/ttl/show_ttl.sql b/tests/cases/standalone/common/ttl/show_ttl.sql index 0c521ef106cf..d226b96211d5 100644 --- a/tests/cases/standalone/common/ttl/show_ttl.sql +++ b/tests/cases/standalone/common/ttl/show_ttl.sql @@ -72,4 +72,11 @@ DROP TABLE test_ttl; USE public; -DROP DATABASE test_ttl_db; \ No newline at end of file +DROP DATABASE test_ttl_db; + +-- test both set database to instant and alter ttl to instant for a database is forbidden +CREATE DATABASE test_ttl_db WITH (ttl = 'instant'); + +CREATE DATABASE test_ttl_db_2 WITH (ttl = '1s'); + +ALTER DATABASE test_ttl_db_2 SET 'ttl' = 'instant'; From 020ed3a8aad202afa34baca07458dc00d745dda2 Mon Sep 17 00:00:00 2001 From: discord9 Date: Fri, 6 Dec 2024 16:03:27 +0800 Subject: [PATCH 21/21] tests: more unit test --- src/common/time/src/ttl.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/common/time/src/ttl.rs b/src/common/time/src/ttl.rs index 6e0f13ac6b39..0544cfb0d198 100644 --- a/src/common/time/src/ttl.rs +++ b/src/common/time/src/ttl.rs @@ -187,15 +187,33 @@ mod test { #[test] fn test_db_ttl_table_ttl() { + // test from ttl to db ttl let ttl = TimeToLive::from(Duration::from_secs(10)); let db_ttl: DatabaseTimeToLive = ttl.try_into().unwrap(); assert_eq!(db_ttl, DatabaseTimeToLive::from(Duration::from_secs(10))); + assert_eq!(TimeToLive::from(db_ttl), ttl); + + let ttl = TimeToLive::from(Duration::from_secs(0)); + let db_ttl: DatabaseTimeToLive = ttl.try_into().unwrap(); + assert_eq!(db_ttl, DatabaseTimeToLive::Forever); + assert_eq!(TimeToLive::from(db_ttl), ttl); + + let ttl = TimeToLive::Instant; + let err_instant = DatabaseTimeToLive::try_from(ttl); + assert!(err_instant.is_err()); // test 0 duration let ttl = Duration::from_secs(0); let db_ttl: DatabaseTimeToLive = ttl.into(); assert_eq!(db_ttl, DatabaseTimeToLive::Forever); + let ttl = Duration::from_secs(10); + let db_ttl: DatabaseTimeToLive = ttl.into(); + assert_eq!( + db_ttl, + DatabaseTimeToLive::Duration(Duration::from_secs(10)) + ); + let ttl = DatabaseTimeToLive::from_humantime_or_str("10s").unwrap(); let ttl: TimeToLive = ttl.into(); assert_eq!(ttl, TimeToLive::from(Duration::from_secs(10)));