Skip to content

Commit

Permalink
[fix](dynamic partition) drop partition exclude history_partition_num (
Browse files Browse the repository at this point in the history
…apache#37539)

FIX:   

When dropping dynamic partition, PR apache#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>
  • Loading branch information
yujun777 and dataroaring authored Jul 9, 2024
1 parent d325b43 commit bc8e660
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -448,8 +448,10 @@ private ArrayList<DropPartitionClause> 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);
Expand All @@ -464,9 +466,12 @@ private ArrayList<DropPartitionClause> 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;
}

Expand Down Expand Up @@ -585,10 +590,12 @@ public void executeDynamicPartition(Collection<Pair<Long, Long>> 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());
}
Expand All @@ -602,10 +609,10 @@ public void executeDynamicPartition(Collection<Pair<Long, Long>> 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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand Down Expand Up @@ -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'
}
Expand Down

0 comments on commit bc8e660

Please sign in to comment.