github-actions[bot] commented on code in PR #67151:
URL: https://github.com/apache/doris/pull/67151#discussion_r3860460109
##########
fe/fe-core/src/main/java/org/apache/doris/planner/OlapScanNode.java:
##########
@@ -1062,16 +1054,54 @@ private void computeTabletInfo() throws UserException {
scanTabletIds.addAll(allTabletIds);
}
- if (!isPointQuery()) {
+ if (!isPointQuery) {
for (int i = 0; i < allTabletIds.size(); i++) {
tabletId2BucketSeq.put(allTabletIds.get(i), i);
}
}
totalTabletsNum += selectedTable.getTablets().size();
selectedSplitNum += tablets.size();
- addScanRangeLocations(partition, tablets, backendAlivePathHashs);
+ Map<Long, Set<Long>> currentBackendAlivePathHashes = isPointQuery
+ ? getBackendAlivePathHashes(allBackends, tablets) :
backendAlivePathHashes;
+ addScanRangeLocations(partition, tablets,
currentBackendAlivePathHashes);
+ }
+ }
+
+ private static Map<Long, Set<Long>>
getBackendAlivePathHashes(Collection<Backend> backends) {
+ Map<Long, Set<Long>> backendAlivePathHashes = Maps.newHashMap();
+ for (Backend backend : backends) {
+ backendAlivePathHashes.put(backend.getId(),
getBackendAlivePathHashes(backend));
+ }
+ return backendAlivePathHashes;
+ }
+
+ @VisibleForTesting
+ static Map<Long, Set<Long>> getBackendAlivePathHashes(
+ Map<Long, Backend> backends, List<Tablet> tablets) {
+ Map<Long, Set<Long>> backendAlivePathHashes = Maps.newHashMap();
+ for (Tablet tablet : tablets) {
+ for (Replica replica : tablet.getReplicas()) {
Review Comment:
[P2] Keep path filtering on the same replica snapshot
Point queries call `lazyEvaluateRangeLocations()` after the planning table
lock has been released. This helper snapshots `tablet.getReplicas()` here, but
`Tablet.getQueryableReplicas()` snapshots it again later. If a replica on a
distinct backend B is added and receives its reported offline path between
those reads, B is absent from this narrowed map; the later lookup gets `null`
and treats B as a normal candidate. The previous full-backend map would have
demoted B to `deadPathReplica`, so shuffle or fixed-replica selection can now
put it ahead of a healthy replica (retry only mitigates that). Please carry one
replica snapshot through both path collection and queryability filtering, and
cover the two-stage publication interval with a latch-based test.
##########
fe/fe-core/src/main/java/org/apache/doris/planner/OlapScanNode.java:
##########
@@ -1062,16 +1054,54 @@ private void computeTabletInfo() throws UserException {
scanTabletIds.addAll(allTabletIds);
}
- if (!isPointQuery()) {
+ if (!isPointQuery) {
for (int i = 0; i < allTabletIds.size(); i++) {
tabletId2BucketSeq.put(allTabletIds.get(i), i);
}
}
totalTabletsNum += selectedTable.getTablets().size();
selectedSplitNum += tablets.size();
- addScanRangeLocations(partition, tablets, backendAlivePathHashs);
+ Map<Long, Set<Long>> currentBackendAlivePathHashes = isPointQuery
+ ? getBackendAlivePathHashes(allBackends, tablets) :
backendAlivePathHashes;
+ addScanRangeLocations(partition, tablets,
currentBackendAlivePathHashes);
+ }
+ }
+
+ private static Map<Long, Set<Long>>
getBackendAlivePathHashes(Collection<Backend> backends) {
+ Map<Long, Set<Long>> backendAlivePathHashes = Maps.newHashMap();
+ for (Backend backend : backends) {
+ backendAlivePathHashes.put(backend.getId(),
getBackendAlivePathHashes(backend));
+ }
+ return backendAlivePathHashes;
+ }
+
+ @VisibleForTesting
+ static Map<Long, Set<Long>> getBackendAlivePathHashes(
+ Map<Long, Backend> backends, List<Tablet> tablets) {
+ Map<Long, Set<Long>> backendAlivePathHashes = Maps.newHashMap();
+ for (Tablet tablet : tablets) {
+ for (Replica replica : tablet.getReplicas()) {
+ long backendId = replica.getBackendIdWithoutException();
Review Comment:
[P2] Skip backend resolution for pathless cloud replicas
For a `CloudTablet`, this call dispatches through
`CloudReplica.getBackendId()` and `getCurrentClusterId()`, including
compute-group privilege/status/auto-start checks and potentially colocate or
multi-replica backend scans. But `CloudReplica` always has `pathHash == -1`, so
`Tablet.getQueryableReplicas()` never consults the alive-path set for it; the
result built here is unused, and the downstream path resolves the backend
again. This adds pure work to the latency-sensitive cloud point-query path.
Please skip `CloudReplica`/`CloudTablet` in this disk-path helper, or resolve
the cloud placement once and reuse it through scan-location construction; a
cloud-mode call-count test would pin this behavior. (A generic `pathHash == -1`
skip would be unsafe for local replicas whose reported path is populated later.)
--
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]
