morrySnow commented on code in PR #65173:
URL: https://github.com/apache/doris/pull/65173#discussion_r3793792411
##########
fe/fe-core/src/main/java/org/apache/doris/qe/PointQueryExecutor.java:
##########
@@ -138,13 +138,18 @@ void setScanRangeLocations() throws Exception {
candidateBackends.add(backend);
}
}
- // Random read replicas
- Collections.shuffle(this.candidateBackends);
+ if (shouldShuffleCandidateBackends(scanNode)) {
+ Collections.shuffle(candidateBackends);
+ }
if (LOG.isDebugEnabled()) {
LOG.debug("set scan locations, backend ids {}, tablet id {}",
candidateBackends, tabletID);
}
}
+ static boolean shouldShuffleCandidateBackends(OlapScanNode scanNode) {
+ return !scanNode.isScanBackendOrderBySelection();
+ }
Review Comment:
过分的抽象
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/distribute/worker/BackendDistributedPlanWorkerManager.java:
##########
@@ -152,6 +166,10 @@ public DistributedPlanWorker randomAvailableWorker(long
catalogId) {
try {
Reference<Long> selectedBackendId = new Reference<>();
ImmutableMap<Long, Backend> backends =
this.currentClusterBackends.get(catalogId);
Review Comment:
不应该去修改这个类，而是创建一个他的子类，覆盖你修改的这两个函数，给load用
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/distribute/worker/LoadBalanceScanWorkerSelector.java:
##########
@@ -56,10 +61,15 @@
/** LoadBalanceScanWorkerSelector */
public class LoadBalanceScanWorkerSelector implements ScanWorkerSelector {
private final DistributedPlanWorkerManager workerManager;
+ private final ConnectContext context;
+ private final boolean useLoadBackendSelection;
private final Map<DistributedPlanWorker, WorkerWorkload> workloads =
Maps.newLinkedHashMap();
- public LoadBalanceScanWorkerSelector(DistributedPlanWorkerManager
workerManager) {
+ public LoadBalanceScanWorkerSelector(DistributedPlanWorkerManager
workerManager,
+ ConnectContext context, boolean useLoadBackendSelection) {
this.workerManager = workerManager;
+ this.context = context;
+ this.useLoadBackendSelection = useLoadBackendSelection;
Review Comment:
这个变量很奇怪，这个类本来就是给load用的，然后又加了一个`useLoadBackendSelection`
需要给这个变量加注释，解释为false的时候处理什么场景，为true的时候处理什么场景
##########
fe/fe-core/src/main/java/org/apache/doris/qe/Coordinator.java:
##########
@@ -2172,6 +2176,102 @@ private TNetworkAddress
getGroupCommitBackend(Map<TNetworkAddress, Long> address
return execHostport;
}
+ private boolean isLoadSelectionCoordinator() {
Review Comment:
为load单独写一个子类，或者将choose的逻辑，封装在不同selector里实现，避免if else，会更为健壮
##########
fe/fe-core/src/main/java/org/apache/doris/qe/AuditLogHelper.java:
##########
@@ -232,7 +233,15 @@ private static void logAuditLogImpl(ConnectContext ctx,
String origStmt, Stateme
} catch (ComputeGroupException e) {
LOG.warn("Failed to get cloud cluster", e);
}
- String cluster = Config.isCloudMode() ? cloudCluster : "";
+ // Load statements resolve their own hint at the scheduling sites and
record it on the
+ // context; prefer it over the scan-side query decision so load audits
are accurate.
+ BackendSelection.SelectionHint selectionHint =
ctx.getLoadBackendSelectionDecisionForAudit();
+ if (selectionHint == null) {
+ selectionHint = ctx.getQueryBackendSelectionDecisionForAudit();
+ }
+ // In cloud mode, compute_group keeps its existing cloud compute group
meaning. In integrated
+ // mode, resource groups provide compute affinity, so reuse
compute_group for the preferred group.
+ String cluster = Config.isCloudMode() ? cloudCluster :
selectionHint.getPreferredKey();
Review Comment:
这种最好能有接口来提供，现在cloud的 if else满天飞，后面没法维护
--
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]
