Copilot commented on code in PR #61511:
URL: https://github.com/apache/doris/pull/61511#discussion_r2958163215
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -4391,15 +4392,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 * 0.8) {
+ LOG.warn("Table {}.{} auto partition count {} is approaching limit
{} (>80%)."
+ + " Consider increasing max_auto_partition_num.",
+ db.getFullName(), olapTable.getName(), partitionNum,
autoPartitionLimit);
Review Comment:
This warning will fire on every createPartition call once the table is above
80% of the limit, which can become very noisy for workloads that continuously
auto-create partitions. Consider adding throttling (e.g.,
per-table/time-window) or only logging when crossing the threshold, relying on
the counter metric for continuous monitoring.
##########
fe/fe-core/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java:
##########
@@ -641,10 +642,19 @@ public static Map<String, String>
analyzeDynamicPartition(Map<String, String> pr
}
expectCreatePartitionNum = (long) end - start;
- if (!isReplay && hasEnd && (expectCreatePartitionNum >
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 > Config.max_dynamic_partition_num) {
+ throw new DdlException("Too many dynamic partitions: "
+ + expectCreatePartitionNum + ". Limit: " +
Config.max_dynamic_partition_num);
+ } else if (expectCreatePartitionNum >
Config.max_dynamic_partition_num * 0.8) {
+ LOG.warn("Dynamic partition count {} is approaching limit {}
(>80%)."
+ + " Consider increasing
max_dynamic_partition_num.",
+ expectCreatePartitionNum,
Config.max_dynamic_partition_num);
Review Comment:
`Config.max_dynamic_partition_num` is read multiple times and the near-limit
threshold uses floating-point math (`* 0.8`). Since the config is mutable,
capture the limit once into a local variable and use integer arithmetic for the
80% threshold to avoid inconsistent comparisons and implicit long/double
conversions.
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -4391,15 +4392,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 * 0.8) {
Review Comment:
The near-limit check uses floating-point math (`autoPartitionLimit * 0.8`),
which introduces an implicit int→double conversion and can lead to surprising
boundary behavior. Prefer integer math (e.g., compare against
`autoPartitionLimit * 8 / 10`, using long to avoid overflow) so the threshold
is exact and type-consistent.
--
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]
