morrySnow commented on code in PR #65219:
URL: https://github.com/apache/doris/pull/65219#discussion_r3628900758
##########
fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java:
##########
@@ -105,6 +104,11 @@ public class DynamicPartitionScheduler extends
MasterDaemon {
private static final long SLEEP_PIECE = 5000L;
+ /** Test-only: when non-null overrides the clock in both add and drop
+ * paths so tests can reproduce time-sensitive scenarios
deterministically. */
+ @VisibleForTesting
+ public static volatile ZonedDateTime testNow;
Review Comment:
不用加这个，把获取now的地方抽象成一个函数，测试里面mock那个函数就可以了
##########
fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java:
##########
@@ -201,23 +205,95 @@ private static long getNextPartitionSize(List<Long>
historyPartitionsSize) {
}
private static Pair<Integer, Integer>
getBucketsNum(DynamicPartitionProperty property, OlapTable table,
- String partitionName, String nowPartitionName, boolean
executeFirstTime) {
+ String partitionName, String nowPartitionName, boolean
executeFirstTime, String currentUtcBorder) {
AutoBucketCalculator.AutoBucketContext context = new
AutoBucketCalculator.AutoBucketContext(
- table, partitionName, nowPartitionName, executeFirstTime,
property.getBuckets());
+ table, partitionName, nowPartitionName, executeFirstTime,
property.getBuckets(),
+ currentUtcBorder);
AutoBucketCalculator.AutoBucketResult result =
AutoBucketCalculator.calculateAutoBuckets(context);
return Pair.of(result.getBuckets(), result.getPreviousBuckets());
}
- public static List<Partition> getHistoricalPartitions(OlapTable table,
String nowPartitionName) {
+ /**
+ * Get all partitions except the current one. When currentKey is non-null,
+ * the current partition is identified by checking if its range
+ * <em>contains</em> currentKey (i.e. lower <= currentKey < upper).
+ * When currentKey is null, fall back to name-based exclusion against
+ * nowPartitionName.
+ *
+ * <p>Using range containment instead of exact lower-bound equality
+ * handles non‑canonical current partitions whose lower endpoint does
+ * not coincide with the captured UTC‑midnight instant — for example a
+ * pre‑fix Asia/Shanghai partition p20260720 = [2026-07-19 16:00Z,
+ * 2026-07-20 16:00Z) where currentKey = 2026-07-22 00:00Z falls inside
+ * the range but does not equal the lower endpoint. Without containment
+ * such a populated, still‑growing partition would be treated as history
+ * and distort auto‑bucket calculations.
+ *
+ * <p>The two filters are mutually exclusive — when a range key is
available
+ * it alone identifies the current partition. Applying both filters on top
+ * of each other would double‑exclude: after the range match removes the
+ * current partition (whose name may use an old prefix after a prefix
change),
+ * the name filter could then catch a <em>different</em> partition whose
name
+ * happens to match the new prefix, losing two partitions and corrupting
+ * auto‑bucket calculations.
+ */
+ public static List<Partition> getHistoricalPartitions(OlapTable table,
String nowPartitionName,
+ String currentUtcBorder) {
table.readLock();
try {
RangePartitionInfo info = (RangePartitionInfo)
(table.getPartitionInfo());
+ // Parse the range key once, before the stream. A single parse
+ // avoids redundant PartitionKey.createPartitionKey calls for every
+ // partition while holding the table read lock.
+ PartitionKey currentKey = null;
+ if (currentUtcBorder != null) {
+ try {
+ Column partitionColumn = info.getPartitionColumns().get(0);
+ PartitionValue currentValue = new
PartitionValue(currentUtcBorder);
+ currentKey = PartitionKey.createPartitionKey(
+ Collections.singletonList(currentValue),
+ Collections.singletonList(partitionColumn));
+ } catch (AnalysisException e) {
+ // Parsing failed — fall back to name-based exclusion
below.
+ currentKey = null;
+ }
+ }
+
+ final PartitionKey rangeKey = currentKey;
+
List<Map.Entry<Long, PartitionItem>> idToItems = new
ArrayList<>(info.getIdToItem(false).entrySet());
idToItems.sort(Comparator.comparing(o -> ((RangePartitionItem)
o.getValue()).getItems().upperEndpoint()));
return idToItems.stream()
.map(entry -> table.getPartition(entry.getKey()))
- .filter(partition -> partition != null &&
!partition.getName().equals(nowPartitionName))
+ .filter(partition -> {
+ if (partition == null) {
+ return false;
+ }
+ if (rangeKey != null) {
+ // Range-based exclusion — mutually exclusive with
+ // name-based exclusion to avoid double-filtering.
+ // Exclude the partition whose range contains
+ // rangeKey (lower <= rangeKey < upper). This
+ // handles both canonical partitions where
+ // rangeKey equals the lower endpoint and
+ // non-canonical partitions where rangeKey falls
+ // inside the range.
+ RangePartitionItem item = (RangePartitionItem)
info.getItem(partition.getId());
+ if (item != null) {
+ Range<PartitionKey> partitionRange =
item.getItems();
+ PartitionKey lower =
partitionRange.lowerEndpoint();
+ PartitionKey upper =
partitionRange.upperEndpoint();
+ if (lower.compareTo(rangeKey) <= 0
+ && rangeKey.compareTo(upper) < 0) {
+ return false; // current partition
+ }
+ }
+ return true; // not the current — keep it
+ }
Review Comment:
这代码是为了兼容之前的错误行为吗？如果是的话，没必要，现在应该没有人用
##########
fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java:
##########
@@ -372,23 +457,29 @@ private ArrayList<AddPartitionOp>
getAddPartitionOp(Database db, OlapTable olapT
dynamicPartitionProperty.getReplicaAllocation().toCreateStmt());
}
- // set storage_medium and storage_cooldown_time based on
dynamic_partition.hot_partition_num
- setStorageMediumProperty(partitionProperties,
dynamicPartitionProperty, now, hotPartitionNum, idx);
+ // set storage_medium and storage_cooldown_time based on
dynamic_partition.hot_partition_num.
+ // Use `now` (UTC-based for TIMESTAMPTZ) so the cooldown boundary
+ // aligns with the partition's UTC range, not the configured
timezone.
+ // columns; for TIMESTAMPTZ it appends a +00:00 suffix so
PropertyAnalyzer
+ // can detect this as an unambiguous UTC instant.
+ setStorageMediumProperty(partitionProperties,
dynamicPartitionProperty, now,
+ hotPartitionNum, idx);
if (StringUtils.isNotEmpty(storagePolicyName)) {
- setStoragePolicyProperty(partitionProperties,
dynamicPartitionProperty, now, idx, storagePolicyName);
+ setStoragePolicyProperty(partitionProperties,
dynamicPartitionProperty, now, idx,
+ storagePolicyName);
Review Comment:
不要做无意义的格式化
##########
fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java:
##########
@@ -278,11 +354,20 @@ public static Pair<Integer, Integer>
calculateBuckets(List<Partition> hasDataPar
private ArrayList<AddPartitionOp> getAddPartitionOp(Database db, OlapTable
olapTable,
Column
partitionColumn, String partitionFormat,
- boolean
executeFirstTime) throws DdlException {
+ boolean
executeFirstTime)
+ throws DdlException {
ArrayList<AddPartitionOp> addPartitionOps = new ArrayList<>();
DynamicPartitionProperty dynamicPartitionProperty =
olapTable.getTableProperty().getDynamicPartitionProperty();
RangePartitionInfo rangePartitionInfo = (RangePartitionInfo)
olapTable.getPartitionInfo();
- ZonedDateTime now =
ZonedDateTime.now(dynamicPartitionProperty.getTimeZone().toZoneId());
+ // For TIMESTAMPTZ, both partition boundaries and names are UTC-based
+ // (00:00—24:00 in UTC). This keeps partition names and values
consistent.
+ boolean isTimestampTz = partitionColumn.getDataType() ==
PrimitiveType.TIMESTAMPTZ;
+ ZonedDateTime nowTz = testNow != null ? testNow
+ :
ZonedDateTime.now(dynamicPartitionProperty.getTimeZone().toZoneId());
+ ZonedDateTime now = isTimestampTz ?
nowTz.withZoneSameInstant(ZoneOffset.UTC) : nowTz;
+ TimeZone borderTimeZone = isTimestampTz
Review Comment:
这个直接从now获取tz就可以了吧？不用这个变量
##########
fe/fe-core/src/main/java/org/apache/doris/common/util/DynamicPartitionUtil.java:
##########
@@ -870,8 +879,12 @@ public static String
getHistoryPartitionRangeString(DynamicPartitionProperty dyn
throw new AnalysisException("Parse dynamic partition periods
error. Error=" + e.getMessage());
}
timestamp = Timestamp.valueOf(dateTime);
- return getFormattedTimeWithoutMinuteSecond(
+ String result = getFormattedTimeWithoutMinuteSecond(
ZonedDateTime.parse(timestamp.toString(), dateTimeFormatter),
format);
+ if (isTimestampTz) {
+ result += "+00:00";
+ }
+ return result;
Review Comment:
这里我印象中完全没必要改，最后他还是能生成正确的返回值
##########
fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java:
##########
@@ -296,23 +381,23 @@ private ArrayList<AddPartitionOp>
getAddPartitionOp(Database db, OlapTable olapT
int hotPartitionNum = dynamicPartitionProperty.getHotPartitionNum();
String storagePolicyName = dynamicPartitionProperty.getStoragePolicy();
- String nowPartitionPrevBorder =
DynamicPartitionUtil.getPartitionRangeString(
+ // Partition naming uses the same `now` clock as border computation,
+ // so names and values are based on the same timezone (UTC for
TIMESTAMPTZ).
+ // getPartitionRangeString appends +00:00 when now is UTC-based,
+ // which getFormattedPartitionName naturally strips away.
+ String currentUtcBorder = DynamicPartitionUtil.getPartitionRangeString(
Review Comment:
为什么要改变量名字？
##########
fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java:
##########
@@ -409,10 +437,32 @@ public static DataProperty
analyzeDataProperty(Map<String, String> properties, f
}
} else if (key.equalsIgnoreCase(PROPERTIES_STORAGE_COOLDOWN_TIME))
{
try {
- DateLiteral dateLiteral =
DateLiteralUtils.createDateLiteral(value,
- ScalarType.getDefaultDateType(Type.DATETIME));
- cooldownTimestamp =
dateLiteral.unixTimestamp(TimeUtils.getTimeZone());
- } catch (AnalysisException e) {
+ // A value with an explicit timezone offset (e.g.
+ // "2027-01-01 00:00:00+00:00" or, for TIMESTAMPTZ(6),
+ // "2027-01-01 00:00:00.000000+00:00") represents an
+ // unambiguous UTC instant. Parse it directly as a
+ // ZonedDateTime and convert to epoch millis, bypassing
+ // the DATETIME-based DateLiteralUtils.createDateLiteral()
+ // path which uses Instant.now() for the initial offset
+ // computation and produces incorrect results across DST
+ // boundaries.
+ if (TZ_OFFSET_PATTERN.matcher(value).find()) {
+ // TZ_FORMATTER only understands ±HH:MM offsets.
+ // Normalize Z, UTC, GMT to the canonical zero
+ // offset so that every explicit-zone spelling is
+ // parsed as an unambiguous instant rather than
+ // falling through to the DATETIME path (which
+ // uses Instant.now() for the DST offset and
+ // produces incorrect results across DST changes).
+ String normalized = value.replaceAll("(Z|UTC|GMT)$",
"+00:00");
+ cooldownTimestamp = TZ_FORMATTER.parse(normalized,
ZonedDateTime::from)
+ .toInstant().toEpochMilli();
+ } else {
+ DateLiteral dateLiteral =
DateLiteralUtils.createDateLiteral(value,
+ ScalarType.getDefaultDateType(Type.DATETIME));
+ cooldownTimestamp =
dateLiteral.unixTimestamp(TimeUtils.getTimeZone());
Review Comment:
这里的改法，我感觉比较好的是传入的 properties 永远带上时区，去初始化date literal，就能是对的，这里也不用去做parse什么的
--
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]
