From 7f8a8464f9f3419521188adbe3cec79d230e7984 Mon Sep 17 00:00:00 2001 From: yujun Date: Tue, 9 Jul 2024 23:52:53 +0800 Subject: [PATCH 1/2] [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+dataroaring@users.noreply.github.com> --- .../clone/DynamicPartitionScheduler.java | 23 ++++++++++++------- .../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 38d68c320ec10a..f884499e8eea2e 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 @@ -432,8 +432,10 @@ private ArrayList getDropPartitionClause(Database db, OlapT 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); @@ -448,9 +450,12 @@ private ArrayList getDropPartitionClause(Database db, OlapT } 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; } @@ -569,10 +574,12 @@ public void executeDynamicPartition(Collection> dynamicPartitio 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()); } @@ -586,10 +593,10 @@ public void executeDynamicPartition(Collection> dynamicPartitio } 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 c2f13837329c88..419707f7cee771 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 void testFillHistoryDynamicPartition3() throws Exception { 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 7da145a36bcd0e..3447834a2af3ac 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) @@ -68,7 +73,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' } } From 3b74dbd65c79b472ed91e611638648fe1db64348 Mon Sep 17 00:00:00 2001 From: yujun777 Date: Wed, 10 Jul 2024 10:45:34 +0800 Subject: [PATCH 2/2] update --- .../org/apache/doris/clone/DynamicPartitionScheduler.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 f884499e8eea2e..4636eb30ca29fd 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 @@ -454,7 +454,7 @@ private ArrayList getDropPartitionClause(Database db, OlapT + ", 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(), + recordDropPartitionFailedMsg(db.getFullName(), olapTable.getName(), hint + ", error: " + e.getMessage(), olapTable.getId()); return dropPartitionClauses; } @@ -579,7 +579,7 @@ public void executeDynamicPartition(Collection> dynamicPartitio tableName = olapTable.getName(); } catch (Exception e) { LOG.warn("db [{}-{}], table [{}-{}]'s dynamic partition has error", - db.getId(), db.getName(), olapTable.getId(), olapTable.getName(), e); + db.getId(), db.getFullName(), olapTable.getId(), olapTable.getName(), e); if (executeFirstTime) { throw new DdlException(e.getMessage()); } @@ -596,7 +596,7 @@ public void executeDynamicPartition(Collection> dynamicPartitio } catch (Exception e) { recordDropPartitionFailedMsg(db.getFullName(), tableName, e.getMessage(), olapTable.getId()); LOG.warn("db [{}-{}], table [{}-{}]'s dynamic partition has error", - db.getId(), db.getName(), olapTable.getId(), olapTable.getName(), e); + db.getId(), db.getFullName(), olapTable.getId(), olapTable.getName(), e); if (executeFirstTime) { throw new DdlException(e.getMessage()); }