xiaokang commented on code in PR #22159: URL: https://github.com/apache/doris/pull/22159#discussion_r1278966598
########## fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java: ########## @@ -3120,6 +3120,40 @@ public static void getDdlStmt(DdlStmt ddlStmt, String dbName, TableIf table, Lis sb.append(olapTable.skipWriteIndexOnLoad()).append("\""); } + // compaction policy + if (olapTable.getCompactionPolicy() != null && !olapTable.getCompactionPolicy().equals("") + && !olapTable.getCompactionPolicy().equals(PropertyAnalyzer.SIZE_BASED_COMPACTION_POLICY)) { + sb.append(",\n\"").append(PropertyAnalyzer.PROPERTIES_COMPACTION_POLICY).append("\" = \""); + sb.append(olapTable.getCompactionPolicy()).append("\""); + } + + // time series compaction goal size + if (olapTable.getTimeSeriesCompactionGoalSizeMbytes() != null + && olapTable.getTimeSeriesCompactionGoalSizeMbytes() + != PropertyAnalyzer.TIME_SERIES_COMPACTION_GOAL_SIZE_MBYTES_DEFAULT_VALUE) { Review Comment: show default value as normal ########## gensrc/thrift/AgentService.thrift: ########## @@ -142,6 +142,10 @@ struct TCreateTabletReq { 19: optional bool enable_unique_key_merge_on_write = false 20: optional i64 storage_policy_id 21: optional TBinlogConfig binlog_config + 22: optional string compaction_policy = "size_based" + 23: optional i64 time_series_compaction_goal_size_mbytes = 512 Review Comment: default 1024 ########## fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyTablePropertiesClause.java: ########## @@ -142,6 +182,65 @@ public void analyze(Analyzer analyzer) throws AnalysisException { || properties.containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_MAX_BYTES) || properties.containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_MAX_HISTORY_NUMS)) { // do nothing, will be alter in SchemaChangeHandler.updateBinlogConfig + } else if (properties.containsKey(PropertyAnalyzer.PROPERTIES_COMPACTION_POLICY)) { + String compactionPolicy = properties.getOrDefault(PropertyAnalyzer.PROPERTIES_COMPACTION_POLICY, ""); + if (compactionPolicy != null + && !compactionPolicy.equals(PropertyAnalyzer.TIME_SERIES_COMPACTION_POLICY) + && !compactionPolicy.equals(PropertyAnalyzer.SIZE_BASED_COMPACTION_POLICY)) { + throw new AnalysisException( + "Table compaction policy only support for " + PropertyAnalyzer.TIME_SERIES_COMPACTION_POLICY + + " or " + PropertyAnalyzer.SIZE_BASED_COMPACTION_POLICY); + } + this.needTableStable = false; + setCompactionPolicy(compactionPolicy); + } else if (properties.containsKey(PropertyAnalyzer.PROPERTIES_TIME_SERIES_COMPACTION_GOAL_SIZE_MBYTES)) { + long goalSizeMbytes; + String goalSizeMbytesStr = properties + .get(PropertyAnalyzer.PROPERTIES_TIME_SERIES_COMPACTION_GOAL_SIZE_MBYTES); + try { + goalSizeMbytes = Long.parseLong(goalSizeMbytesStr); + if (goalSizeMbytes < 0) { + throw new AnalysisException("Invalid time_series_compaction_goal_size_mbytes format: " + + goalSizeMbytesStr); + } + } catch (NumberFormatException e) { + throw new AnalysisException("Invalid time_series_compaction_goal_size_mbytes format: " + + goalSizeMbytesStr); + } + this.needTableStable = false; + setTimeSeriesCompactionGoalSizeMbytes(goalSizeMbytes); + } else if (properties.containsKey(PropertyAnalyzer.PROPERTIES_TIME_SERIES_COMPACTION_FILE_COUNT_THRESHOLD)) { + long fileCountThreshold; + String fileCountThresholdStr = properties + .get(PropertyAnalyzer.PROPERTIES_TIME_SERIES_COMPACTION_FILE_COUNT_THRESHOLD); + try { + fileCountThreshold = Long.parseLong(fileCountThresholdStr); + if (fileCountThreshold < 0) { Review Comment: 10 ########## be/src/olap/tablet_manager.cpp: ########## @@ -729,7 +731,10 @@ TabletSharedPtr TabletManager::find_best_tablet_to_compaction( continue; } } - + auto cumulative_compaction_policy = + tablet_ptr->tablet_meta()->compaction_policy() == CUMULATIVE_TIME_SERIES_POLICY Review Comment: at(tablet_ptr->tablet_meta()->compaction_policy()) ########## fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyTablePropertiesClause.java: ########## @@ -142,6 +182,65 @@ public void analyze(Analyzer analyzer) throws AnalysisException { || properties.containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_MAX_BYTES) || properties.containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_MAX_HISTORY_NUMS)) { // do nothing, will be alter in SchemaChangeHandler.updateBinlogConfig + } else if (properties.containsKey(PropertyAnalyzer.PROPERTIES_COMPACTION_POLICY)) { + String compactionPolicy = properties.getOrDefault(PropertyAnalyzer.PROPERTIES_COMPACTION_POLICY, ""); + if (compactionPolicy != null + && !compactionPolicy.equals(PropertyAnalyzer.TIME_SERIES_COMPACTION_POLICY) + && !compactionPolicy.equals(PropertyAnalyzer.SIZE_BASED_COMPACTION_POLICY)) { + throw new AnalysisException( + "Table compaction policy only support for " + PropertyAnalyzer.TIME_SERIES_COMPACTION_POLICY + + " or " + PropertyAnalyzer.SIZE_BASED_COMPACTION_POLICY); + } + this.needTableStable = false; + setCompactionPolicy(compactionPolicy); + } else if (properties.containsKey(PropertyAnalyzer.PROPERTIES_TIME_SERIES_COMPACTION_GOAL_SIZE_MBYTES)) { + long goalSizeMbytes; + String goalSizeMbytesStr = properties + .get(PropertyAnalyzer.PROPERTIES_TIME_SERIES_COMPACTION_GOAL_SIZE_MBYTES); + try { + goalSizeMbytes = Long.parseLong(goalSizeMbytesStr); + if (goalSizeMbytes < 0) { + throw new AnalysisException("Invalid time_series_compaction_goal_size_mbytes format: " + + goalSizeMbytesStr); + } + } catch (NumberFormatException e) { + throw new AnalysisException("Invalid time_series_compaction_goal_size_mbytes format: " + + goalSizeMbytesStr); + } + this.needTableStable = false; + setTimeSeriesCompactionGoalSizeMbytes(goalSizeMbytes); + } else if (properties.containsKey(PropertyAnalyzer.PROPERTIES_TIME_SERIES_COMPACTION_FILE_COUNT_THRESHOLD)) { + long fileCountThreshold; + String fileCountThresholdStr = properties + .get(PropertyAnalyzer.PROPERTIES_TIME_SERIES_COMPACTION_FILE_COUNT_THRESHOLD); + try { + fileCountThreshold = Long.parseLong(fileCountThresholdStr); + if (fileCountThreshold < 0) { + throw new AnalysisException("Invalid time_series_compaction_file_count_threshold format: " + + fileCountThresholdStr); + } + } catch (NumberFormatException e) { + throw new AnalysisException("Invalid time_series_compaction_file_count_threshold format: " + + fileCountThresholdStr); + } + this.needTableStable = false; + setTimeSeriesCompactionFileCountThreshold(fileCountThreshold); + } else if (properties.containsKey(PropertyAnalyzer.PROPERTIES_TIME_SERIES_COMPACTION_TIME_THRESHOLD_SECONDS)) { + long timeThresholdSeconds; + String timeThresholdSecondsStr = properties + .get(PropertyAnalyzer.PROPERTIES_TIME_SERIES_COMPACTION_TIME_THRESHOLD_SECONDS); + try { + timeThresholdSeconds = Long.parseLong(timeThresholdSecondsStr); + if (timeThresholdSeconds < 0) { Review Comment: 60s ########## fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyTablePropertiesClause.java: ########## @@ -142,6 +182,65 @@ public void analyze(Analyzer analyzer) throws AnalysisException { || properties.containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_MAX_BYTES) || properties.containsKey(PropertyAnalyzer.PROPERTIES_BINLOG_MAX_HISTORY_NUMS)) { // do nothing, will be alter in SchemaChangeHandler.updateBinlogConfig + } else if (properties.containsKey(PropertyAnalyzer.PROPERTIES_COMPACTION_POLICY)) { + String compactionPolicy = properties.getOrDefault(PropertyAnalyzer.PROPERTIES_COMPACTION_POLICY, ""); + if (compactionPolicy != null + && !compactionPolicy.equals(PropertyAnalyzer.TIME_SERIES_COMPACTION_POLICY) + && !compactionPolicy.equals(PropertyAnalyzer.SIZE_BASED_COMPACTION_POLICY)) { + throw new AnalysisException( + "Table compaction policy only support for " + PropertyAnalyzer.TIME_SERIES_COMPACTION_POLICY + + " or " + PropertyAnalyzer.SIZE_BASED_COMPACTION_POLICY); + } + this.needTableStable = false; + setCompactionPolicy(compactionPolicy); + } else if (properties.containsKey(PropertyAnalyzer.PROPERTIES_TIME_SERIES_COMPACTION_GOAL_SIZE_MBYTES)) { + long goalSizeMbytes; + String goalSizeMbytesStr = properties + .get(PropertyAnalyzer.PROPERTIES_TIME_SERIES_COMPACTION_GOAL_SIZE_MBYTES); + try { + goalSizeMbytes = Long.parseLong(goalSizeMbytesStr); + if (goalSizeMbytes < 0) { Review Comment: 10MB ########## be/src/olap/olap_server.cpp: ########## @@ -976,8 +981,18 @@ Status StorageEngine::_submit_compaction_task(TabletSharedPtr tablet, Status StorageEngine::submit_compaction_task(TabletSharedPtr tablet, CompactionType compaction_type, bool force) { _update_cumulative_compaction_policy(); - if (tablet->get_cumulative_compaction_policy() == nullptr) { - tablet->set_cumulative_compaction_policy(_cumulative_compaction_policy); + // alter table tableName set ("compaction_policy"="time_series") + // if atler table's compaction policy, we need to modify tablet compaction policy shared ptr + if (tablet->get_cumulative_compaction_policy() == nullptr || + tablet->get_cumulative_compaction_policy()->name() != + tablet->tablet_meta()->compaction_policy()) { + if (tablet->tablet_meta()->compaction_policy() == CUMULATIVE_TIME_SERIES_POLICY) { Review Comment: at(tablet->tablet_meta()->compaction_policy()) ########## regression-test/suites/compaction/test_table_level_compaction_policy.groovy: ########## @@ -0,0 +1,131 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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. + +import org.codehaus.groovy.runtime.IOGroovyMethods + +suite("test_table_level_compaction_policy") { + def tableName = "test_table_level_compaction_policy" + sql """ DROP TABLE IF EXISTS ${tableName} """ + + sql """ + CREATE TABLE ${tableName} ( + `c_custkey` int(11) NOT NULL COMMENT "", + `c_name` varchar(26) NOT NULL COMMENT "", + `c_address` varchar(41) NOT NULL COMMENT "", + `c_city` varchar(11) NOT NULL COMMENT "" + ) + DUPLICATE KEY (`c_custkey`) + DISTRIBUTED BY HASH(`c_custkey`) BUCKETS 1 + PROPERTIES ( + "replication_num" = "1", + "compaction_policy" = "time_series", + "time_series_compaction_goal_size_mbytes" = "1024", + "time_series_compaction_file_count_threshold" = "5000", + "time_series_compaction_time_threshold_seconds" = "86400" + ); + """ + result = sql """show create table ${tableName}""" + logger.info("${result}") + assertTrue(result.toString().containsIgnoreCase('"compaction_policy" = "time_series"')) + assertTrue(result.toString().containsIgnoreCase('"time_series_compaction_goal_size_mbytes" = "1024"')) + assertTrue(result.toString().containsIgnoreCase('"time_series_compaction_file_count_threshold" = "5000"')) + assertTrue(result.toString().containsIgnoreCase('"time_series_compaction_time_threshold_seconds" = "86400"')) + + sql """ + alter table ${tableName} set ("time_series_compaction_goal_size_mbytes" = "2048") + """ + + result = sql """show create table ${tableName}""" + logger.info("${result}") + assertTrue(result.toString().containsIgnoreCase('"time_series_compaction_goal_size_mbytes" = "2048"')) + + sql """ + alter table ${tableName} set ("time_series_compaction_file_count_threshold" = "6000") + """ + + result = sql """show create table ${tableName}""" + logger.info("${result}") + assertTrue(result.toString().containsIgnoreCase('"time_series_compaction_file_count_threshold" = "6000"')) + + sql """ + alter table ${tableName} set ("time_series_compaction_time_threshold_seconds" = "3000") + """ + + result = sql """show create table ${tableName}""" + logger.info("${result}") + assertTrue(result.toString().containsIgnoreCase('"time_series_compaction_time_threshold_seconds" = "3000"')) + + sql """ DROP TABLE IF EXISTS ${tableName} """ + + test { + sql """ + CREATE TABLE ${tableName} ( + `c_custkey` int(11) NOT NULL COMMENT "", + `c_name` varchar(26) NOT NULL COMMENT "", + `c_address` varchar(41) NOT NULL COMMENT "", + `c_city` varchar(11) NOT NULL COMMENT "" + ) + DUPLICATE KEY (`c_custkey`) + DISTRIBUTED BY HASH(`c_custkey`) BUCKETS 1 + PROPERTIES ( + "replication_num" = "1", + "compaction_policy" = "ok" + ); + """ + exception "compaction_policy must be time_series or size_based" + } + + test { + sql """ + CREATE TABLE ${tableName} ( + `c_custkey` int(11) NOT NULL COMMENT "", + `c_name` varchar(26) NOT NULL COMMENT "", + `c_address` varchar(41) NOT NULL COMMENT "", + `c_city` varchar(11) NOT NULL COMMENT "" + ) + DUPLICATE KEY (`c_custkey`) + DISTRIBUTED BY HASH(`c_custkey`) BUCKETS 1 + PROPERTIES ( + "replication_num" = "1", + "time_series_compaction_goal_size_mbytes" = "1024" + ); + """ + exception "only time series compaction policy support for time series config" + } + + test { + sql """ + CREATE TABLE ${tableName} ( + `c_custkey` int(11) NOT NULL COMMENT "", + `c_name` varchar(26) NOT NULL COMMENT "", + `c_address` varchar(41) NOT NULL COMMENT "", + `c_city` varchar(11) NOT NULL COMMENT "" + ) + DUPLICATE KEY (`c_custkey`) + DISTRIBUTED BY HASH(`c_custkey`) BUCKETS 1 + PROPERTIES ( + "replication_num" = "1" + ); + """ + + sql """ + alter table ${tableName} set ("compaction_policy" = "ok") + """ + exception "Table compaction policy only support for time_series or size_based" + } + sql """ DROP TABLE IF EXISTS ${tableName} """ +} Review Comment: add a test case for no compaction_policy config and check its output -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org