Skip to content

Commit

Permalink
[fix](dynamic partition) fix dynamic partition thread met uncatch exc…
Browse files Browse the repository at this point in the history
…eption (apache#35778)

dynamic partition thread get table's drop clause met an error due to
dynamic_partition.start = -99999999

```
2024-05-23 10:40:32,072 ERROR (DynamicPartitionScheduler|53) [Daemon.run():118] daemon thread got exception. name: DynamicPartitionScheduler
org.apache.doris.nereids.exceptions.AnalysisException: date/datetime literal [+271768-09-11 00:00:00] is invalid
	at org.apache.doris.nereids.trees.expressions.literal.DateLiteral.normalize(DateLiteral.java:202) ~[doris-fe.jar:1.2-SNAPSHOT]
	at org.apache.doris.nereids.trees.expressions.literal.DateTimeLiteral.determineScale(DateTimeLiteral.java:107) ~[doris-fe.jar:1.2-SNAPSHOT]
	at org.apache.doris.nereids.types.DateTimeV2Type.forTypeFromString(DateTimeV2Type.java:91) ~[doris-fe.jar:1.2-SNAPSHOT]
	at org.apache.doris.nereids.trees.expressions.literal.DateTimeV2Literal.<init>(DateTimeV2Literal.java:38) ~[doris-fe.jar:1.2-SNAPSHOT]
	at org.apache.doris.catalog.PartitionKey.getDateTimeLiteral(PartitionKey.java:120) ~[doris-fe.jar:1.2-SNAPSHOT]
	at org.apache.doris.catalog.PartitionKey.createPartitionKey(PartitionKey.java:98) ~[doris-fe.jar:1.2-SNAPSHOT]
	at org.apache.doris.clone.DynamicPartitionScheduler.getDropPartitionClause(DynamicPartitionScheduler.java:431) ~[doris-fe.jar:1.2-SNAPSHOT]
	at org.apache.doris.clone.DynamicPartitionScheduler.executeDynamicPartition(DynamicPartitionScheduler.java:555) ~[doris-fe.jar:1.2-SNAPSHOT]
	at org.apache.doris.clone.DynamicPartitionScheduler.runAfterCatalogReady(DynamicPartitionScheduler.java:641) ~[doris-fe.jar:1.2-SNAPSHOT]
	at org.apache.doris.common.util.MasterDaemon.runOneCycle(MasterDaemon.java:58) ~[doris-fe.jar:1.2-SNAPSHOT]
	at org.apache.doris.common.util.Daemon.run(Daemon.java:116) ~[doris-fe.jar:1.2-SNAPSHOT] 
```

BUG:
1. dynamic partition thread not catch
org.apache.doris.nereids.exceptions.AnalysisException;
2. getDropClauses use dynamic_partition.start as the partitions's start.
But it's error, it should should max(dynamic_partition.start,
-dynamic_partition.history_partition_num).

FIX:
1. dynamic partition thread catch
org.apache.doris.nereids.exceptions.AnalysisException;
2. getDropClauses use start = max(dynamic_partition.start,
-dynamic_partition.history_partition_num);
3. When create table (i.e. create dynamic partion first time), if met an
exception, then throw this exception out, then create table will fail,
so user can known this error;
  • Loading branch information
yujun777 authored and seawinde committed Jun 5, 2024
1 parent d0c8b4d commit 43f0b76
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,13 @@ public void checkDistribution(DistributionInfo distributionInfo) throws DdlExcep
HashDistributionInfo info = (HashDistributionInfo) distributionInfo;
// buckets num
if (info.getBucketNum() != bucketsNum) {
ErrorReport.reportDdlException(ErrorCode.ERR_COLOCATE_TABLE_MUST_HAS_SAME_BUCKET_NUM, bucketsNum);
ErrorReport.reportDdlException(ErrorCode.ERR_COLOCATE_TABLE_MUST_HAS_SAME_BUCKET_NUM,
info.getBucketNum(), bucketsNum);
}
// distribution col size
if (info.getDistributionColumns().size() != distributionColTypes.size()) {
ErrorReport.reportDdlException(ErrorCode.ERR_COLOCATE_TABLE_MUST_HAS_SAME_DISTRIBUTION_COLUMN_SIZE,
distributionColTypes.size());
info.getDistributionColumns().size(), distributionColTypes.size());
}
// distribution col type
for (int i = 0; i < distributionColTypes.size(); i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import org.apache.doris.catalog.RangePartitionInfo;
import org.apache.doris.catalog.RangePartitionItem;
import org.apache.doris.catalog.Table;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Config;
import org.apache.doris.common.DdlException;
import org.apache.doris.common.FeConstants;
Expand Down Expand Up @@ -101,7 +100,7 @@ public DynamicPartitionScheduler(String name, long intervalMs) {
this.initialize = false;
}

public void executeDynamicPartitionFirstTime(Long dbId, Long tableId) {
public void executeDynamicPartitionFirstTime(Long dbId, Long tableId) throws DdlException {
List<Pair<Long, Long>> tempDynamicPartitionTableInfo = Lists.newArrayList(Pair.of(dbId, tableId));
executeDynamicPartition(tempDynamicPartitionTableInfo, true);
}
Expand Down Expand Up @@ -218,23 +217,18 @@ private static int getBucketsNum(DynamicPartitionProperty property, OlapTable ta
}

private ArrayList<AddPartitionClause> getAddPartitionClause(Database db, OlapTable olapTable,
Column partitionColumn, String partitionFormat, boolean executeFirstTime) {
Column partitionColumn, String partitionFormat, boolean executeFirstTime) throws DdlException {
ArrayList<AddPartitionClause> addPartitionClauses = new ArrayList<>();
DynamicPartitionProperty dynamicPartitionProperty = olapTable.getTableProperty().getDynamicPartitionProperty();
RangePartitionInfo rangePartitionInfo = (RangePartitionInfo) olapTable.getPartitionInfo();
ZonedDateTime now = ZonedDateTime.now(dynamicPartitionProperty.getTimeZone().toZoneId());

boolean createHistoryPartition = dynamicPartitionProperty.isCreateHistoryPartition();
int idx;
int start = dynamicPartitionProperty.getStart();
int historyPartitionNum = dynamicPartitionProperty.getHistoryPartitionNum();
// When enable create_history_partition, will check the valid value from start and history_partition_num.
if (createHistoryPartition) {
if (historyPartitionNum == DynamicPartitionProperty.NOT_SET_HISTORY_PARTITION_NUM) {
idx = start;
} else {
idx = Math.max(start, -historyPartitionNum);
}
idx = DynamicPartitionUtil.getRealStart(dynamicPartitionProperty.getStart(),
dynamicPartitionProperty.getHistoryPartitionNum());
} else {
idx = 0;
}
Expand Down Expand Up @@ -263,12 +257,14 @@ private ArrayList<AddPartitionClause> getAddPartitionClause(Database db, OlapTab
PartitionKey upperBound = PartitionKey.createPartitionKey(Collections.singletonList(upperValue),
Collections.singletonList(partitionColumn));
addPartitionKeyRange = Range.closedOpen(lowerBound, upperBound);
} catch (AnalysisException | IllegalArgumentException e) {
} 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 addPartitionKeyRange. Error={}, db: {}, table: {}", e.getMessage(),
db.getFullName(), olapTable.getName());
continue;
LOG.warn("Error in gen addPartitionKeyRange. db: {}, table: {}, partition idx: {}",
db.getFullName(), olapTable.getName(), idx, e);
recordCreatePartitionFailedMsg(db.getFullName(), olapTable.getName(),
e.getMessage(), olapTable.getId());
throw new DdlException(e.getMessage());
}
for (PartitionItem partitionItem : rangePartitionInfo.getIdToItem(false).values()) {
// only support single column partition now
Expand All @@ -278,13 +274,16 @@ private ArrayList<AddPartitionClause> getAddPartitionClause(Database db, OlapTab
isPartitionExists = true;
if (addPartitionKeyRange.equals(partitionItem.getItems())) {
if (LOG.isDebugEnabled()) {
LOG.debug("partition range {} exist in table {}, clear fail msg",
addPartitionKeyRange, olapTable.getName());
LOG.debug("partition range {} exist in db {} table {} partition idx {}, clear fail msg",
addPartitionKeyRange, db.getFullName(), olapTable.getName(), idx);
}
clearCreatePartitionFailedMsg(olapTable.getId());
} else {
LOG.warn("check partition range {} in db {} table {} partiton idx {} fail",
addPartitionKeyRange, db.getFullName(), olapTable.getName(), idx, e);
recordCreatePartitionFailedMsg(db.getFullName(), olapTable.getName(),
e.getMessage(), olapTable.getId());
throw new DdlException(e.getMessage());
}
break;
}
Expand Down Expand Up @@ -394,11 +393,11 @@ private Range<PartitionKey> getClosedRange(Database db, OlapTable olapTable, Col
PartitionKey upperBorderBound = PartitionKey.createPartitionKey(
Collections.singletonList(upperBorderPartitionValue), Collections.singletonList(partitionColumn));
reservedHistoryPartitionKeyRange = Range.closed(lowerBorderBound, upperBorderBound);
} catch (AnalysisException e) {
} catch (org.apache.doris.common.AnalysisException | org.apache.doris.nereids.exceptions.AnalysisException 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. Error={}, db: {}, table: {}", e.getMessage(),
db.getFullName(), olapTable.getName());
LOG.warn("Error in gen reservePartitionKeyRange. {}, table: {}",
db.getFullName(), olapTable.getName(), e);
}
return reservedHistoryPartitionKeyRange;
}
Expand All @@ -416,9 +415,11 @@ private ArrayList<DropPartitionClause> getDropPartitionClause(Database db, OlapT
return dropPartitionClauses;
}

int realStart = DynamicPartitionUtil.getRealStart(dynamicPartitionProperty.getStart(),
dynamicPartitionProperty.getHistoryPartitionNum());
ZonedDateTime now = ZonedDateTime.now(dynamicPartitionProperty.getTimeZone().toZoneId());
String lowerBorder = DynamicPartitionUtil.getPartitionRangeString(dynamicPartitionProperty,
now, dynamicPartitionProperty.getStart(), partitionFormat);
now, realStart, partitionFormat);
PartitionValue lowerPartitionValue = new PartitionValue(lowerBorder);
List<Range<PartitionKey>> reservedHistoryPartitionKeyRangeList = new ArrayList<Range<PartitionKey>>();
Range<PartitionKey> reservePartitionKeyRange;
Expand All @@ -427,11 +428,12 @@ private ArrayList<DropPartitionClause> getDropPartitionClause(Database db, OlapT
Collections.singletonList(partitionColumn));
reservePartitionKeyRange = Range.atLeast(lowerBound);
reservedHistoryPartitionKeyRangeList.add(reservePartitionKeyRange);
} catch (AnalysisException | IllegalArgumentException e) {
} 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. Error={}, db: {}, table: {}", e.getMessage(),
db.getFullName(), olapTable.getName());
LOG.warn("Error in gen reservePartitionKeyRange. db: {}, table: {}",
db.getFullName(), olapTable.getName(), e);
recordDropPartitionFailedMsg(db.getFullName(), olapTable.getName(), e.getMessage(), olapTable.getId());
return dropPartitionClauses;
}

Expand All @@ -452,7 +454,8 @@ private ArrayList<DropPartitionClause> getDropPartitionClause(Database db, OlapT
reservedHistoryPartitionKeyRangeList.add(reservedHistoryPartitionKeyRange);
} catch (IllegalArgumentException e) {
return dropPartitionClauses;
} catch (AnalysisException e) {
} catch (org.apache.doris.common.AnalysisException
| org.apache.doris.nereids.exceptions.AnalysisException e) {
throw new DdlException(e.getMessage());
}
}
Expand Down Expand Up @@ -485,7 +488,7 @@ private ArrayList<DropPartitionClause> getDropPartitionClause(Database db, OlapT
}

private void executeDynamicPartition(Collection<Pair<Long, Long>> dynamicPartitionTableInfoCol,
boolean executeFirstTime) {
boolean executeFirstTime) throws DdlException {
Iterator<Pair<Long, Long>> iterator = dynamicPartitionTableInfoCol.iterator();
while (iterator.hasNext()) {
Pair<Long, Long> tableInfo = iterator.next();
Expand Down Expand Up @@ -549,8 +552,11 @@ private void executeDynamicPartition(Collection<Pair<Long, Long>> dynamicPartiti
}
dropPartitionClauses = getDropPartitionClause(db, olapTable, partitionColumn, partitionFormat);
tableName = olapTable.getName();
} catch (DdlException e) {
LOG.warn("should not happen", e);
} catch (Exception e) {
LOG.warn("has error", e);
if (executeFirstTime) {
throw new DdlException(e.getMessage());
}
} finally {
olapTable.readUnlock();
}
Expand All @@ -564,6 +570,10 @@ private void executeDynamicPartition(Collection<Pair<Long, Long>> dynamicPartiti
clearDropPartitionFailedMsg(olapTable.getId());
} catch (Exception e) {
recordDropPartitionFailedMsg(db.getFullName(), tableName, e.getMessage(), olapTable.getId());
LOG.warn("has error", e);
if (executeFirstTime) {
throw new DdlException(e.getMessage());
}
} finally {
olapTable.writeUnlock();
}
Expand All @@ -576,6 +586,10 @@ private void executeDynamicPartition(Collection<Pair<Long, Long>> dynamicPartiti
clearCreatePartitionFailedMsg(olapTable.getId());
} catch (Exception e) {
recordCreatePartitionFailedMsg(db.getFullName(), tableName, e.getMessage(), olapTable.getId());
LOG.warn("has error", e);
if (executeFirstTime) {
throw new DdlException(e.getMessage());
}
}
}
}
Expand Down Expand Up @@ -633,7 +647,14 @@ protected void runAfterCatalogReady() {
}
setInterval(Config.dynamic_partition_check_interval_seconds * 1000L);
if (Config.dynamic_partition_enable) {
executeDynamicPartition(dynamicPartitionTableInfo, false);
try {
executeDynamicPartition(dynamicPartitionTableInfo, false);
} catch (Exception e) {
// previous had log DdlException
if (LOG.isDebugEnabled()) {
LOG.debug("dynamic partition has error: ", e);
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1123,9 +1123,9 @@ public enum ErrorCode {
ERR_COLOCATE_TABLE_MUST_HAS_SAME_REPLICATION_ALLOCATION(5063, new byte[]{'4', '2', '0', '0', '0'},
"Colocate tables must have same replication allocation: { %s } should be { %s }"),
ERR_COLOCATE_TABLE_MUST_HAS_SAME_BUCKET_NUM(5063, new byte[]{'4', '2', '0', '0', '0'},
"Colocate tables must have same bucket num: %s"),
"Colocate tables must have same bucket num: %s should be %s"),
ERR_COLOCATE_TABLE_MUST_HAS_SAME_DISTRIBUTION_COLUMN_SIZE(5063, new byte[]{'4', '2', '0', '0', '0'},
"Colocate tables distribution columns size must be same : %s"),
"Colocate tables distribution columns size must be same: %s should be %s"),
ERR_COLOCATE_TABLE_MUST_HAS_SAME_DISTRIBUTION_COLUMN_TYPE(5063, new byte[]{'4', '2', '0', '0', '0'},
"Colocate tables distribution columns must have the same data type: %s should be %s"),
ERR_COLOCATE_NOT_COLOCATE_TABLE(5064, new byte[]{'4', '2', '0', '0', '0'},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -583,21 +583,18 @@ public static Map<String, String> analyzeDynamicPartition(Map<String, String> pr
long expectCreatePartitionNum = 0;
if (!createHistoryPartition) {
start = 0;
expectCreatePartitionNum = (long) end - start;
} else {
int historyPartitionNum = Integer.parseInt(analyzedProperties.getOrDefault(
DynamicPartitionProperty.HISTORY_PARTITION_NUM,
String.valueOf(DynamicPartitionProperty.NOT_SET_HISTORY_PARTITION_NUM)));
if (historyPartitionNum != DynamicPartitionProperty.NOT_SET_HISTORY_PARTITION_NUM) {
expectCreatePartitionNum = (long) end - Math.max(start, -historyPartitionNum);
} else {
if (start == Integer.MIN_VALUE) {
throw new DdlException("Provide start or history_partition_num property"
+ " when create_history_partition=true. Otherwise set create_history_partition=false");
}
expectCreatePartitionNum = (long) end - start;
start = getRealStart(start, historyPartitionNum);
if (start == Integer.MIN_VALUE) {
throw new DdlException("Provide start or history_partition_num property"
+ " when create_history_partition=true. Otherwise set create_history_partition=false");
}
}
expectCreatePartitionNum = (long) end - start;

if (hasEnd && (expectCreatePartitionNum > Config.max_dynamic_partition_num)
&& Boolean.parseBoolean(analyzedProperties.getOrDefault(DynamicPartitionProperty.ENABLE, "true"))) {
throw new DdlException("Too many dynamic partitions: "
Expand Down Expand Up @@ -683,6 +680,14 @@ public static Map<String, String> analyzeDynamicPartition(Map<String, String> pr
return analyzedProperties;
}

public static int getRealStart(int start, int historyPartitionNum) {
if (historyPartitionNum == DynamicPartitionProperty.NOT_SET_HISTORY_PARTITION_NUM) {
return start;
} else {
return Math.max(start, -historyPartitionNum);
}
}

public static void checkAlterAllowed(OlapTable olapTable) throws DdlException {
TableProperty tableProperty = olapTable.getTableProperty();
if (tableProperty != null && tableProperty.getDynamicPartitionProperty() != null
Expand Down
10 changes: 8 additions & 2 deletions fe/fe-core/src/test/java/org/apache/doris/alter/AlterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,12 @@ public static void beforeClass() throws Exception {
+ "PROPERTIES\n"
+ "(\n"
+ "\"colocate_with\" = \"group_3\",\n"
+ "\"replication_num\" = \"1\",\n"
+ "\"dynamic_partition.enable\" = \"true\",\n"
+ "\"dynamic_partition.time_unit\" = \"DAY\",\n"
+ "\"dynamic_partition.end\" = \"3\",\n"
+ "\"dynamic_partition.prefix\" = \"p\",\n"
+ "\"dynamic_partition.buckets\" = \"32\",\n"
+ "\"dynamic_partition.buckets\" = \"3\",\n"
+ "\"dynamic_partition.replication_num\" = \"1\",\n"
+ "\"dynamic_partition.create_history_partition\"=\"true\",\n"
+ "\"dynamic_partition.start\" = \"-3\"\n"
Expand Down Expand Up @@ -254,7 +255,12 @@ public static void tearDown() {
private static void createTable(String sql) throws Exception {
Config.enable_odbc_mysql_broker_table = true;
CreateTableStmt createTableStmt = (CreateTableStmt) UtFrameUtils.parseAndAnalyzeStmt(sql, connectContext);
Env.getCurrentEnv().createTable(createTableStmt);
try {
Env.getCurrentEnv().createTable(createTableStmt);
} catch (Exception e) {
e.printStackTrace();
throw e;
}
}

private static void createRemoteStorageResource(String sql) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ public void testBucketNum() throws Exception {
+ ");");

expectedEx.expect(DdlException.class);
expectedEx.expectMessage("Colocate tables must have same bucket num: 1");
expectedEx.expectMessage("Colocate tables must have same bucket num: 2 should be 1");
createTable("create table " + dbName + "." + tableName2 + " (\n"
+ " `k1` int NULL COMMENT \"\",\n"
+ " `k2` varchar(10) NULL COMMENT \"\"\n"
Expand Down Expand Up @@ -282,7 +282,7 @@ public void testDistributionColumnsSize() throws Exception {
+ ");");

expectedEx.expect(DdlException.class);
expectedEx.expectMessage("Colocate tables distribution columns size must be same : 2");
expectedEx.expectMessage("Colocate tables distribution columns size must be same: 1 should be 2");
createTable("create table " + dbName + "." + tableName2 + " (\n"
+ " `k1` int NULL COMMENT \"\",\n"
+ " `k2` varchar(10) NULL COMMENT \"\"\n"
Expand Down
Loading

0 comments on commit 43f0b76

Please sign in to comment.