github-actions[bot] commented on code in PR #68844:
URL: https://github.com/apache/doris/pull/68844#discussion_r4236567328
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -1530,6 +1532,18 @@ public enum IgnoreSplitType {
)
public boolean enableBinarySearchFilteringPartitions = true;
+ @VarAttrDef.VarAttr(
+ name = ENABLE_DYNAMIC_PARTITION_DROP_PROTECTION,
+ flag = VarAttrDef.GLOBAL,
+ description = "Whether to prevent background dynamic partition
tasks from automatically dropping "
+ + "partitions. OFF by default."
+ )
+ private boolean enableDynamicPartitionDropProtection = false;
+
Review Comment:
[P2] Reset the live global value in `UNSET GLOBAL VARIABLE ALL`. This new
false-default flag lives in `SessionVariable`, but
`VariableMgr.setAllVarsToDefaultValue` compares the caller's session copy
before deciding whether to reset it. On the master, if an ADMIN connection
predates another connection's `SET GLOBAL ...=true`, its copy is still false,
so `UNSET GLOBAL VARIABLE ALL` skips this flag, reports OK, and leaves
automatic drops paused. Compare the shared global value for GLOBAL scope and
add a two-connection reset test.
##########
fe/fe-core/src/main/java/org/apache/doris/clone/DynamicPartitionScheduler.java:
##########
@@ -811,6 +814,9 @@ public void executeDynamicPartition(Collection<Pair<Long,
Long>> dynamicPartitio
continue;
}
try {
+ if (VariableMgr.isDynamicPartitionDropProtectionEnabled())
{
+ break;
Review Comment:
[P1] Serialize protection enablement with the actual partition drop. After
this check returns false, it releases VariableMgr's read lock before
`dropPartition` removes the partition. An ADMIN `SET GLOBAL
enable_dynamic_partition_drop_protection=true` can acquire the write lock,
journal true, and return during that gap; this scheduler then drops the
partition despite the guard being enabled. The table write lock does not block
SET. Make SET wait for in-flight automatic drops and order each final check
with removal; cover the interleaving with a latch-based test.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -1530,6 +1532,18 @@ public enum IgnoreSplitType {
)
public boolean enableBinarySearchFilteringPartitions = true;
+ @VarAttrDef.VarAttr(
+ name = ENABLE_DYNAMIC_PARTITION_DROP_PROTECTION,
+ flag = VarAttrDef.GLOBAL,
+ description = "Whether to prevent background dynamic partition
tasks from automatically dropping "
Review Comment:
[P1] Enforce ADMIN for DEFAULT assignments to this global guard. The new
flag defaults to false, and `SET GLOBAL
enable_dynamic_partition_drop_protection = DEFAULT` reaches
`SetSessionVarOp.validate`'s early return before its ADMIN check. `VariableMgr`
then writes false to the global value and edit log, so a user without ADMIN can
resume automatic drops after an administrator enabled protection. Move the
privilege check ahead of the DEFAULT return and add a restricted-user test.
--
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]
