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 2022/05/11 02:44:26 UTC
[GitHub] [incubator-doris] morningman opened a new pull request, #9492: [fix](resource-tag) Consider resource tags when assigning tasks for broker & routine load
morningman opened a new pull request, #9492:
URL: https://github.com/apache/incubator-doris/pull/9492
# Proposed changes
Issue Number: close #xxx
## Problem Summary:
This CL mainly changes:
1. Broker Load
When assigning backends, use user level resource tag to find available backends.
If user level resource tag is not set, broker load task can be assigned to any BE node,
otherwise, task can only be assigned to BE node which match the user level tags.
2. Routine Load
The current routine load job does not have user info, so it can not get user level tag when assigning tasks.
So there are 2 ways:
1. For old routine load job, use tags of replica allocation info to select BE nodes.
2. For new routine load job, the user info will be added and persisted in routine load job.
## Checklist(Required)
1. Does it affect the original behavior: (Yes)
4. Has unit tests been added: (No)
5. Has document been added or modified: (Yes)
6. Does it need to update dependencies: (No)
7. Are there any changes that cannot be rolled back: (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] [incubator-doris] morningman commented on a diff in pull request #9492: [fix](resource-tag) Consider resource tags when assigning tasks for broker & routine load
Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #9492:
URL: https://github.com/apache/incubator-doris/pull/9492#discussion_r874352562
##########
fe/fe-core/src/main/java/org/apache/doris/planner/BrokerScanNode.java:
##########
@@ -135,18 +144,26 @@ private static class ParamCreateContext {
private List<ParamCreateContext> paramCreateContexts;
+ // For broker load and external broker table
public BrokerScanNode(PlanNodeId id, TupleDescriptor destTupleDesc, String planNodeName,
List<List<TBrokerFileStatus>> fileStatusesList, int filesAdded) {
super(id, destTupleDesc, planNodeName, NodeType.BROKER_SCAN_NODE);
this.fileStatusesList = fileStatusesList;
this.filesAdded = filesAdded;
+ if (ConnectContext.get() != null) {
Review Comment:
I think I known what you mean. Because for query, the actually tags we use is based on replica tag, not user level tag.
So may be we don't need user info for query.
But here I insist to add this assignment this, for unification purpose.
--
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] [incubator-doris] morningman commented on a diff in pull request #9492: [fix](resource-tag) Consider resource tags when assigning tasks for broker & routine load
Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #9492:
URL: https://github.com/apache/incubator-doris/pull/9492#discussion_r874351615
##########
fe/fe-core/src/main/java/org/apache/doris/planner/BrokerScanNode.java:
##########
@@ -395,10 +405,21 @@ protected void getFileStatus() throws UserException {
}
private void assignBackends() throws UserException {
+ Set<Tag> tags = Sets.newHashSet();
+ if (userIdentity != null) {
+ tags = Catalog.getCurrentCatalog().getAuth().getResourceTags(userIdentity.getQualifiedUser());
+ if (tags == UserProperty.INVALID_RESOURCE_TAGS) {
+ throw new UserException("No valid resource tag for user: " + userIdentity.getQualifiedUser());
+ }
+ } else {
+ LOG.debug("user info in BrokerScanNode should not be null, add log to observer");
Review Comment:
Why? I think the user info must be set for both query and load. If not, may be we missed somewhere to set it.
--
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] [incubator-doris] xy720 commented on a diff in pull request #9492: [fix](resource-tag) Consider resource tags when assigning tasks for broker & routine load
Posted by GitBox <gi...@apache.org>.
xy720 commented on code in PR #9492:
URL: https://github.com/apache/incubator-doris/pull/9492#discussion_r871282917
##########
fe/fe-core/src/main/java/org/apache/doris/load/routineload/RoutineLoadManager.java:
##########
@@ -468,6 +473,60 @@ public long getAvailableBeForTask(long previousBeId, String clusterName) throws
}
}
+ /**
+ * The routine load task can only be scheduled on backends which has proper resource tags.
+ * The tags should be got from user property.
+ * But in the old version, the routine load job does not have user info, so for compatibility,
+ * if there is no user info, we will get tags from replica allocation if the first partition of the table.
Review Comment:
```suggestion
* if there is no user info, we will get tags from replica allocation of the first partition of the table.
```
##########
fe/fe-core/src/main/java/org/apache/doris/planner/BrokerScanNode.java:
##########
@@ -135,18 +144,26 @@ private static class ParamCreateContext {
private List<ParamCreateContext> paramCreateContexts;
+ // For broker load and external broker table
public BrokerScanNode(PlanNodeId id, TupleDescriptor destTupleDesc, String planNodeName,
List<List<TBrokerFileStatus>> fileStatusesList, int filesAdded) {
super(id, destTupleDesc, planNodeName, NodeType.BROKER_SCAN_NODE);
this.fileStatusesList = fileStatusesList;
this.filesAdded = filesAdded;
+ if (ConnectContext.get() != null) {
Review Comment:
May be we don't need to get user info for query here.
##########
fe/fe-core/src/main/java/org/apache/doris/planner/BrokerScanNode.java:
##########
@@ -395,10 +405,21 @@ protected void getFileStatus() throws UserException {
}
private void assignBackends() throws UserException {
+ Set<Tag> tags = Sets.newHashSet();
+ if (userIdentity != null) {
+ tags = Catalog.getCurrentCatalog().getAuth().getResourceTags(userIdentity.getQualifiedUser());
+ if (tags == UserProperty.INVALID_RESOURCE_TAGS) {
+ throw new UserException("No valid resource tag for user: " + userIdentity.getQualifiedUser());
+ }
+ } else {
+ LOG.debug("user info in BrokerScanNode should not be null, add log to observer");
Review Comment:
I think user info could be allowed null here in a BrokerScanNode used by query.
--
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] [incubator-doris] morningman commented on a diff in pull request #9492: [fix](resource-tag) Consider resource tags when assigning tasks for broker & routine load
Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #9492:
URL: https://github.com/apache/incubator-doris/pull/9492#discussion_r874352882
##########
fe/fe-core/src/main/java/org/apache/doris/planner/BrokerScanNode.java:
##########
@@ -395,10 +405,21 @@ protected void getFileStatus() throws UserException {
}
private void assignBackends() throws UserException {
+ Set<Tag> tags = Sets.newHashSet();
+ if (userIdentity != null) {
+ tags = Catalog.getCurrentCatalog().getAuth().getResourceTags(userIdentity.getQualifiedUser());
+ if (tags == UserProperty.INVALID_RESOURCE_TAGS) {
+ throw new UserException("No valid resource tag for user: " + userIdentity.getQualifiedUser());
+ }
+ } else {
+ LOG.debug("user info in BrokerScanNode should not be null, add log to observer");
Review Comment:
I see. I just want to see where do I miss to set the user info, so I add this debug log 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] [incubator-doris] github-actions[bot] commented on pull request #9492: [fix](resource-tag) Consider resource tags when assigning tasks for broker & routine load
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9492:
URL: https://github.com/apache/incubator-doris/pull/9492#issuecomment-1136620777
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] [incubator-doris] github-actions[bot] commented on pull request #9492: [fix](resource-tag) Consider resource tags when assigning tasks for broker & routine load
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9492:
URL: https://github.com/apache/incubator-doris/pull/9492#issuecomment-1136620761
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] [incubator-doris] morningman merged pull request #9492: [fix](resource-tag) Consider resource tags when assigning tasks for broker & routine load
Posted by GitBox <gi...@apache.org>.
morningman merged PR #9492:
URL: https://github.com/apache/incubator-doris/pull/9492
--
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