github-actions[bot] commented on code in PR #68844:
URL: https://github.com/apache/doris/pull/68844#discussion_r4254886865
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -1531,6 +1533,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] Read the shared value when displaying this global-only guard. Storing
the flag in `SessionVariable` also copies it into every connection. If
connection A opens while it is false and connection B executes `SET GLOBAL
enable_dynamic_partition_drop_protection=true`, A's `SHOW VARIABLES LIKE
'enable_dynamic_partition_drop_protection'` and `SELECT
@@enable_dynamic_partition_drop_protection` still return false:
`VariableMgr.dump` and `getLiteral` select A's clone for non-GLOBAL scope
without checking the GLOBAL flag. The inverse can report protection enabled
after cleanup has resumed. Resolve this global-only flag from the shared value
for default-scope reads/display and add a two-connection enable/disable test;
this is separate from the existing `UNSET ... ALL` reset failure.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -1531,6 +1533,18 @@ public enum IgnoreSplitType {
)
public boolean enableBinarySearchFilteringPartitions = true;
+ @VarAttrDef.VarAttr(
+ name = ENABLE_DYNAMIC_PARTITION_DROP_PROTECTION,
+ flag = VarAttrDef.GLOBAL,
Review Comment:
[P2] Keep named global resets successful through follower FEs. With this
flag enabled, an ADMIN connected to a follower can run `UNSET GLOBAL VARIABLE
enable_dynamic_partition_drop_protection`: the master resets and journals
`false`, but the follower then calls
`UnsetVariableCommand.afterForwardToMaster`. Every `SessionVariable` field is
registered with `SESSION | attr.flag()`, so that hook sees SESSION and calls
`VariableMgr.setVar` with SESSION scope; `checkUpdate` rejects the GLOBAL flag
and the client receives an error although automatic cleanup has already
resumed. This differs from the existing `UNSET ... ALL` issue, which skips the
reset. Skip GLOBAL-only fields in the follower hook (or update the clone
without session-scope validation) and add a forwarded-reset 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]
