[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

[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