From bc8e66052a8422483425c4522b018727bbbf4c37 Mon Sep 17 00:00:00 2001 From: yujun Date: Tue, 9 Jul 2024 23:52:53 +0800 Subject: [PATCH] [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 d17af1836fe762..17a4d34aa381e9 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 @@ 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); @@ -464,9 +466,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; } @@ -585,10 +590,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()); } @@ -602,10 +609,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 a19efadf740018..0a23d422a3f310 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' }