924060929 commented on code in PR #68791:
URL: https://github.com/apache/doris/pull/68791#discussion_r4254051382
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/scan/FederationBackendPolicy.java:
##########
@@ -280,12 +297,15 @@ public Multimap<Backend, Split>
computeScanRangeAssignment(List<Split> splits) t
}
case RANDOM: {
randomCandidates.reset();
- candidateNodes =
selectNodes(Config.split_assigner_min_random_candidate_num, randomCandidates);
+ candidateNodes = consistentHashSpreadNum == 0 ?
backends
+ :
selectNodes(Config.split_assigner_min_random_candidate_num, randomCandidates);
break;
}
case CONSISTENT_HASHING: {
candidateNodes = consistentHash.getNode(split,
-
Config.split_assigner_min_consistent_hash_candidate_num);
+ consistentHashSpreadNum == 0 ? backends.size()
+ : isSpreadEnabled() ?
Math.min(consistentHashSpreadNum, backends.size())
Review Comment:
[P2] Remote Doris still has the same candidate-collapse issue at current
head `22d6ebafe16`.
`RemoteDorisSplit.getSplitIdentity()` returns only `location`, while its
start and length are both zero. `RemoteDorisScanNode.processFlightEndpoints()`
retains different tickets at the same URI. This is a supported case: with
parallel result sinks enabled, `DorisFlightSqlProducer` creates distinct
tickets for the result streams and can advertise the same public proxy address
for all remote BEs.
For example, with 8 eligible local BEs, hashing enabled and
`external_scan_consistent_hash_spread_num=2`, every such split hashes to the
same two candidates. The other six BEs cannot receive those streams because
spread mode disables global redistribution. This loses scan parallelism; I
found no wrong-result or crash consequence. The candidate-set restriction
follows directly from the code; I have not measured throughput locally.
Please include the ticket bytes in an immutable split identity, captured
without advancing the execution ByteBuffer, and add coverage for the same URI
with different tickets. This belongs in `RemoteDorisSplit`, keeping the generic
scheduler connector-agnostic. The other fixes in the latest revision look
addressed, but this part of the original finding remains.
--
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]
