Copilot commented on code in PR #61765:
URL: https://github.com/apache/doris/pull/61765#discussion_r2993151203
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -3795,15 +3796,24 @@ public TCreatePartitionResult
createPartition(TCreatePartitionRequest request) t
// check partition's number limit. because partitions in
addPartitionClauseMap may be duplicated with existing
// partitions, which would lead to false positive. so we should check
the partition number AFTER adding new
// partitions using its ACTUAL NUMBER, rather than the sum of existing
and requested partitions.
- if (olapTable.getPartitionNum() > Config.max_auto_partition_num) {
+ int partitionNum = olapTable.getPartitionNum();
+ int autoPartitionLimit = Config.max_auto_partition_num;
+ if (partitionNum > autoPartitionLimit) {
String errorMessage = String.format(
"partition numbers %d exceeded limit of variable
max_auto_partition_num %d",
- olapTable.getPartitionNum(),
Config.max_auto_partition_num);
+ partitionNum, autoPartitionLimit);
LOG.warn(errorMessage);
errorStatus.setErrorMsgs(Lists.newArrayList(errorMessage));
result.setStatus(errorStatus);
LOG.warn("send create partition error status: {}", result);
return result;
+ } else if (partitionNum > autoPartitionLimit * 8 / 10) {
Review Comment:
`autoPartitionLimit * 8 / 10` is computed using `int` arithmetic and can
overflow if `max_auto_partition_num` is configured to a large value (this is a
mutable config). Use `long` arithmetic (cast before multiply) or compare via
`partitionNum * 10 > autoPartitionLimit * 8` using `long` to avoid
overflow/truncation pitfalls.
```suggestion
} else if ((long) partitionNum * 10 > (long) autoPartitionLimit * 8)
{
```
##########
fe/fe-common/src/main/java/org/apache/doris/common/Config.java:
##########
@@ -2966,9 +2966,8 @@ public class Config extends ConfigBase {
@ConfField(mutable = true, masterOnly = true, description = {
"对于自动分区表，防止用户意外创建大量分区，每个 OLAP 表允许的分区数量为`max_auto_partition_num`。默认
2000。",
Review Comment:
The Chinese description still says the default is 2000 (“默认 2000。”) but the
actual default was changed to 20000. Update the Chinese string to match the new
default to avoid misleading operators.
```suggestion
"对于自动分区表，防止用户意外创建大量分区，每个 OLAP
表允许的分区数量为`max_auto_partition_num`。默认 20000。",
```
##########
fe/fe-core/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java:
##########
@@ -641,10 +642,20 @@ public static Map<String, String>
analyzeDynamicPartition(Map<String, String> pr
}
expectCreatePartitionNum = (long) end - start;
- if (!isReplay && hasEnd && (expectCreatePartitionNum >
Config.max_dynamic_partition_num)
+ int dynamicPartitionLimit = Config.max_dynamic_partition_num;
+ if (!isReplay && hasEnd
&&
Boolean.parseBoolean(analyzedProperties.getOrDefault(DynamicPartitionProperty.ENABLE,
"true"))) {
- throw new DdlException("Too many dynamic partitions: "
- + expectCreatePartitionNum + ". Limit: " +
Config.max_dynamic_partition_num);
+ if (expectCreatePartitionNum > dynamicPartitionLimit) {
+ throw new DdlException("Too many dynamic partitions: "
+ + expectCreatePartitionNum + ". Limit: " +
dynamicPartitionLimit);
+ } else if (expectCreatePartitionNum > dynamicPartitionLimit * 8L /
10) {
+ LOG.warn("Dynamic partition count {} is approaching limit {}
(>80%)."
+ + " Consider increasing max_dynamic_partition_num.",
+ expectCreatePartitionNum, dynamicPartitionLimit);
+ if (MetricRepo.isInit) {
+
MetricRepo.COUNTER_DYNAMIC_PARTITION_NEAR_LIMIT.increase(1L);
+ }
Review Comment:
Similar to the auto-partition path, this will warn and increment the counter
on every analysis call above the 80% threshold, which can be very frequent (DDL
validations and retries). Consider throttling/deduping, or incrementing only on
threshold crossing to keep logs/metrics actionable and avoid alert fatigue.
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -3795,15 +3796,24 @@ public TCreatePartitionResult
createPartition(TCreatePartitionRequest request) t
// check partition's number limit. because partitions in
addPartitionClauseMap may be duplicated with existing
// partitions, which would lead to false positive. so we should check
the partition number AFTER adding new
// partitions using its ACTUAL NUMBER, rather than the sum of existing
and requested partitions.
- if (olapTable.getPartitionNum() > Config.max_auto_partition_num) {
+ int partitionNum = olapTable.getPartitionNum();
+ int autoPartitionLimit = Config.max_auto_partition_num;
+ if (partitionNum > autoPartitionLimit) {
String errorMessage = String.format(
"partition numbers %d exceeded limit of variable
max_auto_partition_num %d",
- olapTable.getPartitionNum(),
Config.max_auto_partition_num);
+ partitionNum, autoPartitionLimit);
LOG.warn(errorMessage);
errorStatus.setErrorMsgs(Lists.newArrayList(errorMessage));
result.setStatus(errorStatus);
LOG.warn("send create partition error status: {}", result);
return result;
+ } else if (partitionNum > autoPartitionLimit * 8 / 10) {
+ LOG.warn("Table {}.{} auto partition count {} is approaching limit
{} (>80%)."
+ + " Consider increasing max_auto_partition_num.",
+ db.getFullName(), olapTable.getName(), partitionNum,
autoPartitionLimit);
+ if (MetricRepo.isInit) {
+ MetricRepo.COUNTER_AUTO_PARTITION_NEAR_LIMIT.increase(1L);
+ }
Review Comment:
This warning (and counter increment) will trigger on every `createPartition`
call once the table is above the 80% threshold, which can produce noisy logs
and rapidly increasing counters in busy clusters. Consider adding
throttling/deduping (e.g., log at most once per table per time window, or only
when crossing the threshold) and similarly gate the metric increment to
threshold-crossing events rather than per-request.
```suggestion
} else {
// Only emit the warning and increment the metric when crossing
the 80% threshold.
// Estimate the partition count before this request by
subtracting the number of
// partitions requested to be added. This avoids noisy
logs/metrics when the table
// is already above the threshold.
int prevPartitionNumEstimate = partitionNum -
addPartitionClauseMap.size();
if (prevPartitionNumEstimate < 0) {
prevPartitionNumEstimate = 0;
}
int threshold80 = autoPartitionLimit * 8 / 10;
if (partitionNum > threshold80 && prevPartitionNumEstimate <=
threshold80) {
LOG.warn("Table {}.{} auto partition count {} is approaching
limit {} (>80%)."
+ " Consider increasing max_auto_partition_num.",
db.getFullName(), olapTable.getName(), partitionNum,
autoPartitionLimit);
if (MetricRepo.isInit) {
MetricRepo.COUNTER_AUTO_PARTITION_NEAR_LIMIT.increase(1L);
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
