From 4810c91a64d0151e8fbe26eb9146b14588e00dd4 Mon Sep 17 00:00:00 2001 From: Zhenchi Date: Mon, 19 Feb 2024 16:03:41 +0800 Subject: [PATCH] refactor(index): move option `segment_row_count` from WriteOptions to IndexOptions (#3307) Signed-off-by: Zhenchi --- src/mito2/src/access_layer.rs | 1 - src/mito2/src/cache/write_cache.rs | 1 - src/mito2/src/region/options.rs | 25 +++++++++++++++++++++++-- src/mito2/src/sst/index.rs | 9 +++------ src/mito2/src/sst/parquet.rs | 5 ----- 5 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/mito2/src/access_layer.rs b/src/mito2/src/access_layer.rs index 6c69e8459d7..ef8d2358a8d 100644 --- a/src/mito2/src/access_layer.rs +++ b/src/mito2/src/access_layer.rs @@ -140,7 +140,6 @@ impl AccessLayer { file_id, file_path: index_file_path, metadata: &request.metadata, - segment_row_count: write_opts.index_segment_row_count, row_group_size: write_opts.row_group_size, object_store: self.object_store.clone(), intermediate_manager: self.intermediate_manager.clone(), diff --git a/src/mito2/src/cache/write_cache.rs b/src/mito2/src/cache/write_cache.rs index edf89a47c68..c13e32e6415 100644 --- a/src/mito2/src/cache/write_cache.rs +++ b/src/mito2/src/cache/write_cache.rs @@ -114,7 +114,6 @@ impl WriteCache { file_id, file_path: self.file_cache.cache_file_path(puffin_key), metadata: &write_request.metadata, - segment_row_count: write_opts.index_segment_row_count, row_group_size: write_opts.row_group_size, object_store: self.file_cache.local_store(), intermediate_manager: self.intermediate_manager.clone(), diff --git a/src/mito2/src/region/options.rs b/src/mito2/src/region/options.rs index 5811d8b3798..d9205eea6c0 100644 --- a/src/mito2/src/region/options.rs +++ b/src/mito2/src/region/options.rs @@ -27,6 +27,8 @@ use store_api::storage::ColumnId; use crate::error::{Error, JsonOptionsSnafu, Result}; +const DEFAULT_INDEX_SEGMENT_ROW_COUNT: usize = 1024; + /// Options that affect the entire region. /// /// Users need to specify the options while creating/opening a region. @@ -171,13 +173,27 @@ pub struct IndexOptions { } /// Options for the inverted index. -#[derive(Debug, Clone, PartialEq, Eq, Default, Deserialize)] +#[serde_as] +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(default)] pub struct InvertedIndexOptions { /// The column ids that should be ignored when building the inverted index. /// The column ids are separated by commas. For example, "1,2,3". #[serde(deserialize_with = "deserialize_ignore_column_ids")] pub ignore_column_ids: Vec, + + /// The number of rows in a segment. + #[serde_as(as = "DisplayFromStr")] + pub segment_row_count: usize, +} + +impl Default for InvertedIndexOptions { + fn default() -> Self { + Self { + ignore_column_ids: Vec::new(), + segment_row_count: DEFAULT_INDEX_SEGMENT_ROW_COUNT, + } + } } fn deserialize_ignore_column_ids<'de, D>(deserializer: D) -> Result, D::Error> @@ -300,12 +316,16 @@ mod tests { #[test] fn test_with_index() { - let map = make_map(&[("index.inverted_index.ignore_column_ids", "1,2,3")]); + let map = make_map(&[ + ("index.inverted_index.ignore_column_ids", "1,2,3"), + ("index.inverted_index.segment_row_count", "512"), + ]); let options = RegionOptions::try_from(&map).unwrap(); let expect = RegionOptions { index_options: IndexOptions { inverted_index: InvertedIndexOptions { ignore_column_ids: vec![1, 2, 3], + segment_row_count: 512, }, }, ..Default::default() @@ -356,6 +376,7 @@ mod tests { index_options: IndexOptions { inverted_index: InvertedIndexOptions { ignore_column_ids: vec![1, 2, 3], + segment_row_count: 1024, }, }, }; diff --git a/src/mito2/src/sst/index.rs b/src/mito2/src/sst/index.rs index 34bca9f39dc..6a1b0a6e71b 100644 --- a/src/mito2/src/sst/index.rs +++ b/src/mito2/src/sst/index.rs @@ -130,7 +130,6 @@ pub(crate) struct IndexerBuilder<'a> { pub(crate) file_path: String, pub(crate) metadata: &'a RegionMetadataRef, pub(crate) row_group_size: usize, - pub(crate) segment_row_count: usize, pub(crate) object_store: ObjectStore, pub(crate) intermediate_manager: IntermediateManager, pub(crate) index_options: IndexOptions, @@ -156,7 +155,9 @@ impl<'a> IndexerBuilder<'a> { return Indexer::default(); } - let Some(mut segment_row_count) = NonZeroUsize::new(self.segment_row_count) else { + let Some(mut segment_row_count) = + NonZeroUsize::new(self.index_options.inverted_index.segment_row_count) + else { warn!( "Segment row count is 0, skip creating index, region_id: {}, file_id: {}", self.metadata.region_id, self.file_id, @@ -287,7 +288,6 @@ mod tests { file_id: FileId::random(), file_path: "test".to_string(), metadata: &metadata, - segment_row_count: 16, row_group_size: 1024, object_store: mock_object_store(), intermediate_manager: mock_intm_mgr(), @@ -308,7 +308,6 @@ mod tests { file_id: FileId::random(), file_path: "test".to_string(), metadata: &metadata, - segment_row_count: 16, row_group_size: 1024, object_store: mock_object_store(), intermediate_manager: mock_intm_mgr(), @@ -329,7 +328,6 @@ mod tests { file_id: FileId::random(), file_path: "test".to_string(), metadata: &metadata, - segment_row_count: 16, row_group_size: 1024, object_store: mock_object_store(), intermediate_manager: mock_intm_mgr(), @@ -350,7 +348,6 @@ mod tests { file_id: FileId::random(), file_path: "test".to_string(), metadata: &metadata, - segment_row_count: 0, row_group_size: 0, object_store: mock_object_store(), intermediate_manager: mock_intm_mgr(), diff --git a/src/mito2/src/sst/parquet.rs b/src/mito2/src/sst/parquet.rs index c88c29d8bb2..b0b72bde3bf 100644 --- a/src/mito2/src/sst/parquet.rs +++ b/src/mito2/src/sst/parquet.rs @@ -39,8 +39,6 @@ pub const PARQUET_METADATA_KEY: &str = "greptime:metadata"; pub(crate) const DEFAULT_READ_BATCH_SIZE: usize = 1024; /// Default row group size for parquet files. const DEFAULT_ROW_GROUP_SIZE: usize = 100 * DEFAULT_READ_BATCH_SIZE; -/// Default segment row count for inverted index. -const DEFAULT_INDEX_SEGMENT_ROW_COUNT: usize = 1024; /// Parquet write options. #[derive(Debug)] @@ -49,8 +47,6 @@ pub struct WriteOptions { pub write_buffer_size: ReadableSize, /// Row group size. pub row_group_size: usize, - /// Segment row count for inverted index. - pub index_segment_row_count: usize, } impl Default for WriteOptions { @@ -58,7 +54,6 @@ impl Default for WriteOptions { WriteOptions { write_buffer_size: DEFAULT_WRITE_BUFFER_SIZE, row_group_size: DEFAULT_ROW_GROUP_SIZE, - index_segment_row_count: DEFAULT_INDEX_SEGMENT_ROW_COUNT, } } }