github-actions[bot] commented on code in PR #65173:
URL: https://github.com/apache/doris/pull/65173#discussion_r3520516038
##########
fe/fe-core/src/main/java/org/apache/doris/planner/OlapScanNode.java:
##########
@@ -703,7 +715,8 @@ private void addScanRangeLocations(Partition partition,
continue;
}
String beTagName = backend.getLocationTag().value;
- if (isInvalidComputeGroup || (isNotCloudComputeGroup &&
!computeGroup.containsBackend(beTagName))) {
+ if (shouldFilterReplicaByResourceTag(isInvalidComputeGroup,
isNotCloudComputeGroup,
+ computeGroup.containsBackend(beTagName))) {
Review Comment:
This helper call lost the old invalid-compute-group short-circuit. In the
previous condition, `isInvalidComputeGroup || (isNotCloudComputeGroup &&
!computeGroup.containsBackend(beTagName))` never called `containsBackend()` for
`ComputeGroup.INVALID_COMPUTE_GROUP`, so the code recorded the replica-access
error and later raised the normal `UserException`. Here Java evaluates
`computeGroup.containsBackend(beTagName)` before entering
`shouldFilterReplicaByResourceTag(...)`; the invalid sentinel's
`containsBackend()` calls `checkInvalidComputeGroup()` and throws
`RuntimeException`. A non-cloud scan planned with an invalid user compute group
can now fail before the existing tablet error path. Please keep the helper
lazy, or compute `backendInComputeGroup` only after `!isInvalidComputeGroup &&
isNotCloudComputeGroup`.
##########
fe/fe-core/src/main/java/org/apache/doris/load/GroupCommitManager.java:
##########
@@ -223,13 +226,15 @@ public Backend selectBackendForGroupCommit(long tableId,
ConnectContext context)
}
return be;
} catch (Exception e) {
- throw new LoadException(e.getMessage());
+ String message = e.getMessage() != null ? e.getMessage() :
e.toString();
+ throw new LoadException(message, e);
}
} else {
try {
// Master FE will select BE by itself.
return Env.getCurrentSystemInfo()
-
.getBackend(selectBackendForGroupCommitInternal(tableId, clusterName));
+
.getBackend(selectBackendForGroupCommitInternal(tableId, clusterName,
+
ResourceGroupAffinityPolicyFactory.get().decideForLoad(context)));
Review Comment:
This path still bypasses the load-affinity enable gate. The master-side
group-commit selector calls `decideForLoad(context)` here even when
`enable_load_local_affinity` is false, and the follower path does the same in
`MasterOpExecutor.setGroupCommitLoadAffinity()` before forwarding
`loadAffinityPreferredGroup`/`loadAffinityPolicy` to the master. Other load
paths now check `isLoadAffinityEnabled(context)` before consulting the SPI, so
a downstream policy that returns or validates a decision despite the disabled
session flag can still change group-commit routing. Please gate both the direct
master path and forwarded `TGroupCommitInfo` population before calling
`decideForLoad()`, and cover the disabled group-commit path with a policy that
would fail if invoked.
--
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]
