codeDing18 commented on code in PR #67518:
URL: https://github.com/apache/doris/pull/67518#discussion_r3977398242
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java:
##########
@@ -2758,14 +2758,34 @@ public PlanFragment
visitPhysicalWindow(PhysicalWindow<? extends Plan> physicalW
return inputPlanFragment;
}
+ /**
+ * Collects backends of the remote doris clusters referenced by the lazy
materialized
+ * relations. Rowids of remote tables are generated by the remote
cluster's BEs, so the
+ * second phase fetch must be able to reach them. Duplicate relations may
resolve to the
+ * same remote table (e.g. self join), so merge by backend id.
+ */
+ private List<Backend> collectRemoteBackends(PhysicalLazyMaterialize<?
extends Plan> materialize) {
+ Map<Long, Backend> mergedBackends = Maps.newHashMap();
Review Comment:
Agreed on the direction — collecting the backends from the relations
actually referenced by the plan (both local and remote)
would narrow nodes_info from the whole compute group down to only the
backends the second-phase fetch can ever route to, and it
would also eliminate the eager per-backend client setup for backends that
hold no rows of the plan.
However, I'd like to keep it out of this PR and do it as a follow-up, for
two reasons:
1. It changes the current behavior of every local TopN lazy
materialization query, not just the remote-catalog case this PR
fixes. The collection for local tables needs to walk pruned partitions →
tablets → all replicas (the replica actually read in
phase 1 is not known at plan time), plus the cloud-mode replica resolution
(CloudReplica.getBackendIdWithClusterId). Missing any
of these shapes would put a legitimate backend id outside the address
book and reintroduce the failed to find rpc_struct
failure this PR fixes — on the mainstream local path.
2. This PR is scoped to making the remote catalog path work (routing +
address book + wg fallback + collision guard), and the
change above is a pure performance refinement of the address book size
with no correctness gain on its own.
So this PR keeps the local address book unchanged (full compute group, as
today) and only adds the remote backends into the same
policy-filtered candidate list. I'll file the relation-based collection
as a follow-up issue/PR where it can be tested in
isolation.
--
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]
