This is an automated email from the ASF dual-hosted git repository.

morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/master by this push:
     new bc8e66052a8 [fix](dynamic partition) drop partition exclude 
history_partition_num (#37539)
bc8e66052a8 is described below

commit bc8e66052a8422483425c4522b018727bbbf4c37
Author: yujun <yu.jun.re...@gmail.com>
AuthorDate: Tue Jul 9 23:52:53 2024 +0800

    [fix](dynamic partition) drop partition exclude history_partition_num 
(#37539)
    
    FIX:
    
    When dropping dynamic partition, PR #35778 will use math.max(start,
    -history_partition_num) as the first partition, but it may delete users'
    partitions if they specify both start and history_partition_num
    inappropriately. For safety reason, revert this behavious changed, only
    use start as the first partition when dropping partitions.
    
    For those who had specified a very small start value, drop partitions
    will catch an exception , and stop dropping this table's partition and
    then record this error in dynamic info. Users can use command `SHOW
    DYNAMIC PARTITION TABLES FROM DBXXX` to know this error. From this
    error, it will give user hint to modify start if they really specify a
    error start.
    
    ---------
    
    Co-authored-by: Yongqiang YANG 
<98214048+dataroar...@users.noreply.github.com>
---
 .../doris/clone/DynamicPartitionScheduler.java     | 23 ++++++++++++++--------
 .../doris/catalog/DynamicPartitionTableTest.java   |  6 +++++-
 .../test_dynamic_partition_failed.groovy           | 13 ++++++++----
 3 files changed, 29 insertions(+), 13 deletions(-)

diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java
 
b/fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java
index d17af1836fe..17a4d34aa38 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java
@@ -448,8 +448,10 @@ public class DynamicPartitionScheduler extends 
MasterDaemon {
             return dropPartitionClauses;
         }
 
-        int realStart = 
DynamicPartitionUtil.getRealStart(dynamicPartitionProperty.getStart(),
-                dynamicPartitionProperty.getHistoryPartitionNum());
+        // drop partition only considering start, not considering 
history_partition_num.
+        // int realStart = 
DynamicPartitionUtil.getRealStart(dynamicPartitionProperty.getStart(),
+        //         dynamicPartitionProperty.getHistoryPartitionNum());
+        int realStart = dynamicPartitionProperty.getStart();
         ZonedDateTime now = 
ZonedDateTime.now(dynamicPartitionProperty.getTimeZone().toZoneId());
         String lowerBorder = 
DynamicPartitionUtil.getPartitionRangeString(dynamicPartitionProperty,
                 now, realStart, partitionFormat);
@@ -464,9 +466,12 @@ public class DynamicPartitionScheduler extends 
MasterDaemon {
         } catch (Exception e) {
             // AnalysisException: keys.size is always equal to column.size, 
cannot reach this exception
             // IllegalArgumentException: lb is greater than ub
-            LOG.warn("Error in gen reservePartitionKeyRange. db: {}, table: 
{}",
-                    db.getFullName(), olapTable.getName(), e);
-            recordDropPartitionFailedMsg(db.getFullName(), 
olapTable.getName(), e.getMessage(), olapTable.getId());
+            String hint = "'dynamic_partition.start' = " + 
dynamicPartitionProperty.getStart()
+                    + ", maybe it's too small, can use alter table sql to 
increase it. ";
+            LOG.warn("Error in gen reservePartitionKeyRange. db: {}, table: 
{}. {}",
+                    db.getFullName(), olapTable.getName(), hint, e);
+            recordDropPartitionFailedMsg(db.getFullName(), 
olapTable.getName(), hint + e.getMessage(),
+                    olapTable.getId());
             return dropPartitionClauses;
         }
 
@@ -585,10 +590,12 @@ public class DynamicPartitionScheduler extends 
MasterDaemon {
                     addPartitionClauses = getAddPartitionClause(db, olapTable, 
partitionColumn, partitionFormat,
                             executeFirstTime);
                 }
+                clearDropPartitionFailedMsg(olapTable.getId());
                 dropPartitionClauses = getDropPartitionClause(db, olapTable, 
partitionColumn, partitionFormat);
                 tableName = olapTable.getName();
             } catch (Exception e) {
-                LOG.warn("has error", e);
+                LOG.warn("db [{}-{}], table [{}-{}]'s dynamic partition has 
error",
+                        db.getId(), db.getName(), olapTable.getId(), 
olapTable.getName(), e);
                 if (executeFirstTime) {
                     throw new DdlException(e.getMessage());
                 }
@@ -602,10 +609,10 @@ public class DynamicPartitionScheduler extends 
MasterDaemon {
                 }
                 try {
                     Env.getCurrentEnv().dropPartition(db, olapTable, 
dropPartitionClause);
-                    clearDropPartitionFailedMsg(olapTable.getId());
                 } catch (Exception e) {
                     recordDropPartitionFailedMsg(db.getFullName(), tableName, 
e.getMessage(), olapTable.getId());
-                    LOG.warn("has error", e);
+                    LOG.warn("db [{}-{}], table [{}-{}]'s dynamic partition 
has error",
+                            db.getId(), db.getName(), olapTable.getId(), 
olapTable.getName(), e);
                     if (executeFirstTime) {
                         throw new DdlException(e.getMessage());
                     }
diff --git 
a/fe/fe-core/src/test/java/org/apache/doris/catalog/DynamicPartitionTableTest.java
 
b/fe/fe-core/src/test/java/org/apache/doris/catalog/DynamicPartitionTableTest.java
index c2f13837329..419707f7cee 100644
--- 
a/fe/fe-core/src/test/java/org/apache/doris/catalog/DynamicPartitionTableTest.java
+++ 
b/fe/fe-core/src/test/java/org/apache/doris/catalog/DynamicPartitionTableTest.java
@@ -744,7 +744,11 @@ public class DynamicPartitionTableTest {
         String alter5 = "alter table test.dynamic_partition4 set 
('dynamic_partition.history_partition_num' = '3')";
         ExceptionChecker.expectThrowsNoException(() -> alterTable(alter5));
         
Env.getCurrentEnv().getDynamicPartitionScheduler().executeDynamicPartitionFirstTime(db.getId(),
 tbl4.getId());
-        Assert.assertEquals(7, tbl4.getPartitionNames().size());
+        Assert.assertEquals(9, tbl4.getPartitionNames().size());
+        String dropPartitionErr = 
Env.getCurrentEnv().getDynamicPartitionScheduler()
+                .getRuntimeInfo(tbl4.getId(), 
DynamicPartitionScheduler.DROP_PARTITION_MSG);
+        Assert.assertTrue(dropPartitionErr.contains("'dynamic_partition.start' 
= -99999999, maybe it's too small, "
+                + "can use alter table sql to increase it."));
     }
 
     @Test
diff --git 
a/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition_failed.groovy
 
b/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition_failed.groovy
index a19efadf740..0a23d422a3f 100644
--- 
a/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition_failed.groovy
+++ 
b/regression-test/suites/partition_p0/dynamic_partition/test_dynamic_partition_failed.groovy
@@ -18,8 +18,8 @@
 suite('test_dynamic_partition_failed', 'nonConcurrent') {
     def old_max_dynamic_partition_num = 
getFeConfig('max_dynamic_partition_num')
     try {
-        sql 'DROP TABLE IF EXISTS test_dynamic_partition_failed_1'
-        sql '''CREATE TABLE test_dynamic_partition_failed_1
+        sql 'DROP TABLE IF EXISTS test_dynamic_partition_failed_ok1 FORCE'
+        sql '''CREATE TABLE test_dynamic_partition_failed_ok1
               ( `k1` datetime NULL )
               PARTITION BY RANGE (k1)()
               DISTRIBUTED BY HASH(`k1`) BUCKETS 1
@@ -36,8 +36,13 @@ suite('test_dynamic_partition_failed', 'nonConcurrent') {
                 "dynamic_partition.create_history_partition" = "true"
               )'''
 
-        def partitions = sql_return_maparray "SHOW PARTITIONS FROM 
test_dynamic_partition_failed_1"
+        def partitions = sql_return_maparray "SHOW PARTITIONS FROM 
test_dynamic_partition_failed_ok1"
         assertEquals(9, partitions.size());
+        def dynamicInfo = sql_return_maparray("SHOW DYNAMIC PARTITION 
TABLES").find { it.TableName == 'test_dynamic_partition_failed_ok1' }
+        logger.info("table dynamic info: " + dynamicInfo)
+        assertNotNull(dynamicInfo)
+        
assertTrue(dynamicInfo.LastDropPartitionMsg.contains("'dynamic_partition.start' 
= -99999999, maybe it's too small, "
+                + "can use alter table sql to increase it."))
 
         setFeConfig('max_dynamic_partition_num', Integer.MAX_VALUE)
 
@@ -96,7 +101,7 @@ suite('test_dynamic_partition_failed', 'nonConcurrent') {
         }
     } finally {
         setFeConfig('max_dynamic_partition_num', old_max_dynamic_partition_num)
-        sql 'DROP TABLE IF EXISTS test_dynamic_partition_failed_1'
+        sql 'DROP TABLE IF EXISTS test_dynamic_partition_failed_ok1 FORCE'
         sql 'DROP TABLE IF EXISTS test_dynamic_partition_failed_2'
         sql 'DROP TABLE IF EXISTS test_dynamic_partition_failed_3'
     }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to