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

Reply via email to