You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2023/01/17 17:32:00 UTC
[GitHub] [doris] caiconghui opened a new pull request, #16037: [enhancement](query) Make query scan nodes more evenly distributed
caiconghui opened a new pull request, #16037:
URL: https://github.com/apache/doris/pull/16037
# Proposed changes
Issue Number: close #xxx
## Problem summary
Describe your changes.
## Checklist(Required)
1. Does it affect the original behavior:
- [ ] Yes
- [ ] No
- [ ] I don't know
2. Has unit tests been added:
- [ ] Yes
- [ ] No
- [ ] No Need
3. Has document been added or modified:
- [ ] Yes
- [ ] No
- [ ] No Need
4. Does it need to update dependencies:
- [ ] Yes
- [ ] No
5. Are there any changes that cannot be rolled back:
- [ ] Yes (If Yes, please explain WHY)
- [ ] No
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] caiconghui commented on a diff in pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed in cluster
Posted by GitBox <gi...@apache.org>.
caiconghui commented on code in PR #16037:
URL: https://github.com/apache/doris/pull/16037#discussion_r1081082105
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SimpleScheduler.java:
##########
@@ -134,8 +134,8 @@ public static TNetworkAddress getHost(ImmutableMap<Long, Backend> backends,
Map.Entry<Long, Backend> backendEntry = backends.entrySet().stream().skip(id).filter(
e -> isAvailable(e.getValue())).findFirst().orElse(null);
if (backendEntry == null && id > 0) {
- backendEntry = backends.entrySet().stream().filter(
- e -> isAvailable(e.getValue())).limit(id).findFirst().orElse(null);
+ backendEntry = backends.entrySet().stream().limit(id).filter(
Review Comment:
just keep same with the origin algorithm.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] caiconghui commented on a diff in pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed in cluster
Posted by GitBox <gi...@apache.org>.
caiconghui commented on code in PR #16037:
URL: https://github.com/apache/doris/pull/16037#discussion_r1081068103
##########
fe/fe-core/src/main/java/org/apache/doris/qe/Coordinator.java:
##########
@@ -1542,10 +1541,34 @@ private void computeColocateJoinInstanceParam(PlanFragmentId fragmentId,
}
}
+ private Map<TNetworkAddress, Long> getReplicaNumPerHost() {
+ Map<TNetworkAddress, Long> replicaNumPerHost = Maps.newHashMap();
+ for (ScanNode scanNode : scanNodes) {
+ List<TScanRangeLocations> locationsList = scanNode.getScanRangeLocations(0);
+ if (locationsList == null) {
+ // only analysis olap scan node
+ continue;
+ }
+ for (TScanRangeLocations locations : locationsList) {
+ for (TScanRangeLocation location : locations.locations) {
+ if (replicaNumPerHost.containsKey(location.server)) {
+ replicaNumPerHost.put(location.server, replicaNumPerHost.get(location.server) + 1L);
+ } else {
+ replicaNumPerHost.put(location.server, 1L);
+ }
+ }
+
+ }
+ }
+ return replicaNumPerHost;
+ }
+
// Populates scan_range_assignment_.
// <fragment, <server, nodeId>>
private void computeScanRangeAssignment() throws Exception {
- HashMap<TNetworkAddress, Long> assignedBytesPerHost = Maps.newHashMap();
+ Map<TNetworkAddress, Long> assignedBytesPerHost = Maps.newHashMap();
+ Map<TNetworkAddress, Long> replicaNumPerHost = getReplicaNumPerHost();
+ Collections.shuffle(scanNodes);
Review Comment:
yes, for one query, it is necessary to shuffle scanNodes to make all replicas shuffled
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wangbo commented on a diff in pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed in cluster
Posted by GitBox <gi...@apache.org>.
wangbo commented on code in PR #16037:
URL: https://github.com/apache/doris/pull/16037#discussion_r1081055342
##########
fe/fe-core/src/main/java/org/apache/doris/qe/Coordinator.java:
##########
@@ -1542,10 +1541,34 @@ private void computeColocateJoinInstanceParam(PlanFragmentId fragmentId,
}
}
+ private Map<TNetworkAddress, Long> getReplicaNumPerHost() {
+ Map<TNetworkAddress, Long> replicaNumPerHost = Maps.newHashMap();
+ for (ScanNode scanNode : scanNodes) {
+ List<TScanRangeLocations> locationsList = scanNode.getScanRangeLocations(0);
+ if (locationsList == null) {
+ // only analysis olap scan node
+ continue;
+ }
+ for (TScanRangeLocations locations : locationsList) {
+ for (TScanRangeLocation location : locations.locations) {
+ if (replicaNumPerHost.containsKey(location.server)) {
+ replicaNumPerHost.put(location.server, replicaNumPerHost.get(location.server) + 1L);
+ } else {
+ replicaNumPerHost.put(location.server, 1L);
+ }
+ }
+
+ }
+ }
+ return replicaNumPerHost;
+ }
+
// Populates scan_range_assignment_.
// <fragment, <server, nodeId>>
private void computeScanRangeAssignment() throws Exception {
- HashMap<TNetworkAddress, Long> assignedBytesPerHost = Maps.newHashMap();
+ Map<TNetworkAddress, Long> assignedBytesPerHost = Maps.newHashMap();
+ Map<TNetworkAddress, Long> replicaNumPerHost = getReplicaNumPerHost();
+ Collections.shuffle(scanNodes);
Review Comment:
Is it neccessary to shuffle ```scanNodes``` ?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] caiconghui commented on pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed
Posted by GitBox <gi...@apache.org>.
caiconghui commented on PR #16037:
URL: https://github.com/apache/doris/pull/16037#issuecomment-1396573174
> Please add more detail explain in your PR comment
more detail could be seen in issue #16308
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed in cluster
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #16037:
URL: https://github.com/apache/doris/pull/16037#issuecomment-1396798388
PR approved by anyone and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed in cluster
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #16037:
URL: https://github.com/apache/doris/pull/16037#issuecomment-1396798279
PR approved by at least one committer and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wangbo commented on a diff in pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed in cluster
Posted by GitBox <gi...@apache.org>.
wangbo commented on code in PR #16037:
URL: https://github.com/apache/doris/pull/16037#discussion_r1081076069
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SimpleScheduler.java:
##########
@@ -134,8 +134,8 @@ public static TNetworkAddress getHost(ImmutableMap<Long, Backend> backends,
Map.Entry<Long, Backend> backendEntry = backends.entrySet().stream().skip(id).filter(
e -> isAvailable(e.getValue())).findFirst().orElse(null);
if (backendEntry == null && id > 0) {
- backendEntry = backends.entrySet().stream().filter(
- e -> isAvailable(e.getValue())).limit(id).findFirst().orElse(null);
+ backendEntry = backends.entrySet().stream().limit(id).filter(
Review Comment:
Why first limit then filter?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] caiconghui commented on a diff in pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed in cluster
Posted by GitBox <gi...@apache.org>.
caiconghui commented on code in PR #16037:
URL: https://github.com/apache/doris/pull/16037#discussion_r1081055044
##########
fe/fe-core/src/main/java/org/apache/doris/qe/Coordinator.java:
##########
@@ -1363,8 +1363,7 @@ private void computeFragmentHosts() throws Exception {
if (params.instanceExecParams.isEmpty()) {
Reference<Long> backendIdRef = new Reference<Long>();
TNetworkAddress execHostport;
- if (ConnectContext.get() != null
- && !ConnectContext.get().isResourceTagsSet()
+ if (ConnectContext.get() != null && ConnectContext.get().isResourceTagsSet()
Review Comment:
the following the comment said，if resource tag is set，so I think it is a bug here
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] hello-stephen commented on pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed
Posted by GitBox <gi...@apache.org>.
hello-stephen commented on PR #16037:
URL: https://github.com/apache/doris/pull/16037#issuecomment-1386151727
TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 35.9 seconds
load time: 505 seconds
storage size: 17122720790 Bytes
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20230117221230_clickbench_pr_82679.html
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] caiconghui commented on a diff in pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed in cluster
Posted by GitBox <gi...@apache.org>.
caiconghui commented on code in PR #16037:
URL: https://github.com/apache/doris/pull/16037#discussion_r1081055044
##########
fe/fe-core/src/main/java/org/apache/doris/qe/Coordinator.java:
##########
@@ -1363,8 +1363,7 @@ private void computeFragmentHosts() throws Exception {
if (params.instanceExecParams.isEmpty()) {
Reference<Long> backendIdRef = new Reference<Long>();
TNetworkAddress execHostport;
- if (ConnectContext.get() != null
- && !ConnectContext.get().isResourceTagsSet()
+ if (ConnectContext.get() != null && ConnectContext.get().isResourceTagsSet()
Review Comment:
the following the comment said，”by the tag is located“，so I think it is a bug here
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wangbo commented on a diff in pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed in cluster
Posted by GitBox <gi...@apache.org>.
wangbo commented on code in PR #16037:
URL: https://github.com/apache/doris/pull/16037#discussion_r1081086373
##########
fe/fe-core/src/main/java/org/apache/doris/qe/Coordinator.java:
##########
@@ -1646,47 +1672,51 @@ public TScanRangeLocation selectBackendsByRoundRobin(TScanRangeLocations seqLoca
}
try {
- return selectBackendsByRoundRobin(localLocations, assignedBytesPerHost, backendIdRef);
+ return selectBackendsByRoundRobin(localLocations, assignedBytesPerHost, replicaNumPerHost, backendIdRef);
} catch (UserException ue) {
if (!Config.enable_local_replica_selection_fallback) {
throw ue;
}
- return selectBackendsByRoundRobin(nonlocalLocations, assignedBytesPerHost, backendIdRef);
+ return selectBackendsByRoundRobin(nonlocalLocations, assignedBytesPerHost, replicaNumPerHost, backendIdRef);
}
}
public TScanRangeLocation selectBackendsByRoundRobin(List<TScanRangeLocation> locations,
- HashMap<TNetworkAddress, Long> assignedBytesPerHost, Reference<Long> backendIdRef) throws UserException {
+ Map<TNetworkAddress, Long> assignedBytesPerHost, Map<TNetworkAddress, Long> replicaNumPerHost,
+ Reference<Long> backendIdRef) throws UserException {
Long minAssignedBytes = Long.MAX_VALUE;
+ Long minReplicaNum = Long.MAX_VALUE;
TScanRangeLocation minLocation = null;
Long step = 1L;
for (final TScanRangeLocation location : locations) {
Long assignedBytes = findOrInsert(assignedBytesPerHost, location.server, 0L);
- if (assignedBytes < minAssignedBytes) {
+ if (assignedBytes < minAssignedBytes || (assignedBytes.equals(minAssignedBytes)
+ && replicaNumPerHost.get(location.server) < minReplicaNum)) {
minAssignedBytes = assignedBytes;
+ minReplicaNum = replicaNumPerHost.get(location.server);
minLocation = location;
}
}
+ for (TScanRangeLocation location : locations) {
+ replicaNumPerHost.put(location.server, replicaNumPerHost.get(location.server) - 1);
+ }
TScanRangeLocation location = SimpleScheduler.getLocation(minLocation, locations,
this.idToBackend, backendIdRef);
- if (assignedBytesPerHost.containsKey(location.server)) {
- assignedBytesPerHost.put(location.server,
- assignedBytesPerHost.get(location.server) + step);
- } else {
- assignedBytesPerHost.put(location.server, step);
- }
+ assignedBytesPerHost.put(location.server, assignedBytesPerHost.get(location.server) + step);
Review Comment:
If minLocation is not location, NPE may happend here?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] caiconghui commented on a diff in pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed in cluster
Posted by GitBox <gi...@apache.org>.
caiconghui commented on code in PR #16037:
URL: https://github.com/apache/doris/pull/16037#discussion_r1081090795
##########
fe/fe-core/src/main/java/org/apache/doris/qe/Coordinator.java:
##########
@@ -1646,47 +1672,51 @@ public TScanRangeLocation selectBackendsByRoundRobin(TScanRangeLocations seqLoca
}
try {
- return selectBackendsByRoundRobin(localLocations, assignedBytesPerHost, backendIdRef);
+ return selectBackendsByRoundRobin(localLocations, assignedBytesPerHost, replicaNumPerHost, backendIdRef);
} catch (UserException ue) {
if (!Config.enable_local_replica_selection_fallback) {
throw ue;
}
- return selectBackendsByRoundRobin(nonlocalLocations, assignedBytesPerHost, backendIdRef);
+ return selectBackendsByRoundRobin(nonlocalLocations, assignedBytesPerHost, replicaNumPerHost, backendIdRef);
}
}
public TScanRangeLocation selectBackendsByRoundRobin(List<TScanRangeLocation> locations,
- HashMap<TNetworkAddress, Long> assignedBytesPerHost, Reference<Long> backendIdRef) throws UserException {
+ Map<TNetworkAddress, Long> assignedBytesPerHost, Map<TNetworkAddress, Long> replicaNumPerHost,
+ Reference<Long> backendIdRef) throws UserException {
Long minAssignedBytes = Long.MAX_VALUE;
+ Long minReplicaNum = Long.MAX_VALUE;
TScanRangeLocation minLocation = null;
Long step = 1L;
for (final TScanRangeLocation location : locations) {
Long assignedBytes = findOrInsert(assignedBytesPerHost, location.server, 0L);
- if (assignedBytes < minAssignedBytes) {
+ if (assignedBytes < minAssignedBytes || (assignedBytes.equals(minAssignedBytes)
+ && replicaNumPerHost.get(location.server) < minReplicaNum)) {
minAssignedBytes = assignedBytes;
+ minReplicaNum = replicaNumPerHost.get(location.server);
minLocation = location;
}
}
+ for (TScanRangeLocation location : locations) {
+ replicaNumPerHost.put(location.server, replicaNumPerHost.get(location.server) - 1);
+ }
TScanRangeLocation location = SimpleScheduler.getLocation(minLocation, locations,
this.idToBackend, backendIdRef);
- if (assignedBytesPerHost.containsKey(location.server)) {
- assignedBytesPerHost.put(location.server,
- assignedBytesPerHost.get(location.server) + step);
- } else {
- assignedBytesPerHost.put(location.server, step);
- }
+ assignedBytesPerHost.put(location.server, assignedBytesPerHost.get(location.server) + step);
Review Comment:
findOrInsert(assignedBytesPerHost, location.server, 0L)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morningman commented on pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed
Posted by GitBox <gi...@apache.org>.
morningman commented on PR #16037:
URL: https://github.com/apache/doris/pull/16037#issuecomment-1386315855
Please add more detail explain in your PR comment
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] caiconghui merged pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed in cluster
Posted by GitBox <gi...@apache.org>.
caiconghui merged PR #16037:
URL: https://github.com/apache/doris/pull/16037
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] wangbo commented on a diff in pull request #16037: [enhancement](query) Make query scan nodes more evenly distributed in cluster
Posted by GitBox <gi...@apache.org>.
wangbo commented on code in PR #16037:
URL: https://github.com/apache/doris/pull/16037#discussion_r1081052864
##########
fe/fe-core/src/main/java/org/apache/doris/qe/Coordinator.java:
##########
@@ -1363,8 +1363,7 @@ private void computeFragmentHosts() throws Exception {
if (params.instanceExecParams.isEmpty()) {
Reference<Long> backendIdRef = new Reference<Long>();
TNetworkAddress execHostport;
- if (ConnectContext.get() != null
- && !ConnectContext.get().isResourceTagsSet()
+ if (ConnectContext.get() != null && ConnectContext.get().isResourceTagsSet()
Review Comment:
Why change here?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org