Skip to content

[fix](fe) Preserve effective Compute Group for audit logs (#66281) #67216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
deardeng wants to merge 1 commit into
base: branch-4.0
Choose a base branch
from
Open

[fix](fe) Preserve effective Compute Group for audit logs (#66281) #67216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions fe/fe-core/src/main/java/org/apache/doris/qe/AuditLogHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ private static void logAuditLogImpl(ConnectContext ctx, String origStmt, Stateme
String cloudCluster = "";
try {
if (Config.isCloudMode()) {
cloudCluster = ctx.getCloudCluster(false);
cloudCluster = getCloudClusterForAudit(ctx);
}
} catch (ComputeGroupException e) {
LOG.warn("Failed to get cloud cluster", e);
Expand Down Expand Up @@ -372,6 +372,13 @@ private static long getQueueTimeMs(ConnectContext ctx) {
return queueToken == null ? -1 : queueToken.getQueueEndTime() - queueToken.getQueueStartTime();
}

static String getCloudClusterForAudit(ConnectContext ctx) throws ComputeGroupException {
if (!Strings.isNullOrEmpty(ctx.getEffectiveCloudCluster())) {
return ctx.getEffectiveCloudCluster();
}
return ctx.getCloudCluster(false);
}

/**
* Update query metrics without writing audit log. This is used when
* enable_prepared_stmt_audit_log is disabled, to ensure QPS metrics
Expand Down Expand Up @@ -407,7 +414,7 @@ private static void updateMetricsImpl(ConnectContext ctx) {
String physicalClusterName = "";
try {
if (Config.isCloudMode()) {
cloudCluster = ctx.getCloudCluster(false);
cloudCluster = getCloudClusterForAudit(ctx);
physicalClusterName = ((CloudSystemInfoService) Env.getCurrentSystemInfo())
.getPhysicalCluster(cloudCluster);
if (!cloudCluster.equals(physicalClusterName)) {
Expand Down Expand Up @@ -471,4 +478,3 @@ private static String getStmtType(StatementBase stmt) {
}
}
}

11 changes: 11 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/qe/ConnectContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,9 @@ public enum ConnectType {

// cloud cluster name
protected volatile String cloudCluster = null;
// The compute group selected for the statement currently being executed. Unlike cloudCluster,
// this value is query-scoped and remains available after a per-query SET_VAR is reverted.
protected volatile String effectiveCloudCluster = null;

// If set to true, the nondeterministic function will not be rewrote to constant.
private boolean notEvalNondeterministicFunction = false;
Expand Down Expand Up @@ -1430,6 +1433,14 @@ public void setCloudCluster(String cluster) {
this.getSessionVariable().setCloudCluster(cluster);
}

public String getEffectiveCloudCluster() {
return effectiveCloudCluster;
}

public void setEffectiveCloudCluster(String cluster) {
this.effectiveCloudCluster = cluster;
}

public String getCloudCluster() throws ComputeGroupException {
return getCloudCluster(true);
}
Expand Down
6 changes: 6 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,7 @@ boolean shouldDisableCloudVersionCacheOnRetry(String errorMessage) {

public void execute(TUniqueId queryId) throws Exception {
SessionVariable sessionVariable = context.getSessionVariable();
context.setEffectiveCloudCluster(null);
if (context.getConnectType() == ConnectType.ARROW_FLIGHT_SQL) {
context.setReturnResultFromLocal(true);
}
Expand All @@ -615,6 +616,11 @@ public void execute(TUniqueId queryId) throws Exception {
throw e;
}
} finally {
// Preserve the effective per-query compute group before SET_VAR values are reverted.
// Audit logging runs after this method returns and otherwise sees the session value.
if (Config.isCloudMode()) {
context.setEffectiveCloudCluster(sessionVariable.getCloudCluster());
}
// Snapshot changed session variables (including SET_VAR hint values) BEFORE revert,
// so the audit log (logged after execute() returns, i.e. after the revert below) can
// reflect what was actually in effect for this statement instead of the reverted values.
Expand Down
Loading