You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2020/12/01 12:36:36 UTC

[GitHub] [incubator-dolphinscheduler] GitFireMan opened a new pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

GitFireMan opened a new pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140


   To improve the performance, remove some unuseful lines. #4137 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] dailidong closed pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
dailidong closed pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] GitFireMan removed a comment on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
GitFireMan removed a comment on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-738527037


   > > @CalvinKirs When a new task put into the `queue`, sleep one second will make the comsumer waiting util one second pass.
   > > But use `poll(timeout,Unit)` can solve this question, and can have some performance promote when the Tasks comes low frequency.
   > > Also it can resolve that an always failed high-priority task block master consumer.
   > > Queue implement used here is `PriorityBlockingQueue` , and poll(timeout,Unit) belongs to `PriorityBlockingQueue` .
   > 
   > Sorry, I think you can do this.
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] GitFireMan commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
GitFireMan commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-738541778


   > > > > @CalvinKirs When a new task put into the `queue`, sleep one second will make the comsumer waiting util one second pass.
   > > > > But use `poll(timeout,Unit)` can solve this question, and can have some performance promote when the Tasks comes low frequency.
   > > > > Also it can resolve that an always failed high-priority task block master consumer.
   > > > > Queue implement used here is `PriorityBlockingQueue` , and poll(timeout,Unit) belongs to `PriorityBlockingQueue` .
   > > > 
   > > > 
   > > > Sorry, I think you can do this.
   > > 
   > > 
   > > Can not do this ?
   > 
   > You can do as you said, sounds good to me, btw, code style check fail, you can refer to this article to complete the modification https://dolphinscheduler.apache.org/zh-cn/docs/development/pull -request.html
   
   Quite appreciate to your suggestion. I will submit a new pr according to the code standard.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] GitFireMan closed pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
GitFireMan closed pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] GitFireMan edited a comment on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
GitFireMan edited a comment on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-737009454


   @lgcareer I get your mean. To avoid an always failed high-priority task block master consumer. 
   Maybe we can use `public E poll(long timeout, TimeUnit unit)` .
   code like this
   ![image](https://user-images.githubusercontent.com/18112654/100834014-1830d100-34a6-11eb-8ddd-5735c75a236d.png)
   ![image](https://user-images.githubusercontent.com/18112654/100834029-2252cf80-34a6-11eb-9550-ffe5ad63645b.png)
   
   what do you think about?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] GitFireMan commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
GitFireMan commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-738527037


   > > @CalvinKirs When a new task put into the `queue`, sleep one second will make the comsumer waiting util one second pass.
   > > But use `poll(timeout,Unit)` can solve this question, and can have some performance promote when the Tasks comes low frequency.
   > > Also it can resolve that an always failed high-priority task block master consumer.
   > > Queue implement used here is `PriorityBlockingQueue` , and poll(timeout,Unit) belongs to `PriorityBlockingQueue` .
   > 
   > Sorry, I think you can do this.
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] sonarcloud[bot] commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-736531342


   Kudos, SonarCloud Quality Gate passed!
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4140&metric=coverage&view=list) No Coverage information  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4140&metric=duplicated_lines_density&view=list) No Duplication information
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] CalvinKirs commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-738524514


   > @CalvinKirs When a new task put into the `queue`, sleep one second will make the comsumer waiting util one second pass.
   > But use `poll(timeout,Unit)` can solve this question, and can have some performance promote when the Tasks comes low frequency.
   > Also it can resolve that an always failed high-priority task block master consumer.
   > 
   > Queue implement used here is `PriorityBlockingQueue` , and poll(timeout,Unit) belongs to `PriorityBlockingQueue` .
   
   Sorry, I think you can do this.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] GitFireMan edited a comment on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
GitFireMan edited a comment on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-737009454


   @lgcareer I get your mean. To avoid an always failed high-priority task block master consumer. 
   Maybe we can use `public E poll(long timeout, TimeUnit unit)` .
   code like this
   `String taskPriorityInfo = taskPriorityQueue.poll(Constants.SLEEP_TIME_MILLIS, TimeUnit.MILLISECONDS);
                       if(Objects.isNull(taskPriorityInfo)){
                           continue;
                       }`
   
   what do you think about?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] CalvinKirs edited a comment on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
CalvinKirs edited a comment on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-738528829


   > > > @CalvinKirs When a new task put into the `queue`, sleep one second will make the comsumer waiting util one second pass.
   > > > But use `poll(timeout,Unit)` can solve this question, and can have some performance promote when the Tasks comes low frequency.
   > > > Also it can resolve that an always failed high-priority task block master consumer.
   > > > Queue implement used here is `PriorityBlockingQueue` , and poll(timeout,Unit) belongs to `PriorityBlockingQueue` .
   > > 
   > > 
   > > Sorry, I think you can do this.
   > 
   > Can not do this ?
   
   You can do as you said, sounds good to me, btw, code style check fail, you can refer to this article to complete the modification https://dolphinscheduler.apache.org/zh-cn/docs/development/pull -request.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] GitFireMan commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
GitFireMan commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-737009454


   @lgcareer I get your mean. To avoid an always failed high-priority task block master consumer. 
   Maybe we can use `public E poll(long timeout, TimeUnit unit)` .
   code like this
   ![image](https://user-images.githubusercontent.com/18112654/100833941-e91a5f80-34a5-11eb-8419-bccda61a734c.png)
   ![image](https://user-images.githubusercontent.com/18112654/100833879-ca1bcd80-34a5-11eb-81cf-690db60b94e3.png)
   
   what do you think about?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] dailidong commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
dailidong commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-773145201


   I will close this PR , please re-submit a new PR
   thanks for your first contribution


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] GitFireMan edited a comment on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
GitFireMan edited a comment on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-737009454


   @lgcareer I get your mean. To avoid an always failed high-priority task block master consumer. 
   Maybe we can use `public E poll(long timeout, TimeUnit unit)` .
   code like this
   ![image](https://user-images.githubusercontent.com/18112654/100834014-1830d100-34a6-11eb-8ddd-5735c75a236d.png)
   
   what do you think about?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] codecov-io commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-738531076


   # [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4140?src=pr&el=h1) Report
   > Merging [#4140](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4140?src=pr&el=desc) (abf7eda) into [dev](https://codecov.io/gh/apache/incubator-dolphinscheduler/commit/aa0974fd1f759e96430d3f1b8dac291d6ea7388c?el=desc) (aa0974f) will **decrease** coverage by `0.04%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4140/graphs/tree.svg?width=650&height=150&src=pr&token=bv9iXXRLi9)](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4140?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev    #4140      +/-   ##
   ============================================
   - Coverage     41.37%   41.33%   -0.05%     
   + Complexity     3116     3111       -5     
   ============================================
     Files           467      467              
     Lines         22168    22168              
     Branches       2720     2720              
   ============================================
   - Hits           9173     9164       -9     
   - Misses        12113    12122       +9     
     Partials        882      882              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4140?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...ver/master/consumer/TaskPriorityQueueConsumer.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4140/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9jb25zdW1lci9UYXNrUHJpb3JpdHlRdWV1ZUNvbnN1bWVyLmphdmE=) | `5.96% <0.00%> (+0.03%)` | `5.00 <0.00> (ø)` | |
   | [...scheduler/service/queue/TaskPriorityQueueImpl.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4140/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvcXVldWUvVGFza1ByaW9yaXR5UXVldWVJbXBsLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...inscheduler/service/zk/CuratorZookeeperClient.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4140/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvemsvQ3VyYXRvclpvb2tlZXBlckNsaWVudC5qYXZh) | `56.09% <0.00%> (-9.76%)` | `6.00% <0.00%> (-2.00%)` | |
   | [...er/master/dispatch/host/assign/RandomSelector.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4140/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9kaXNwYXRjaC9ob3N0L2Fzc2lnbi9SYW5kb21TZWxlY3Rvci5qYXZh) | `77.77% <0.00%> (-5.56%)` | `3.00% <0.00%> (-1.00%)` | |
   | [...e/dolphinscheduler/remote/NettyRemotingClient.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4140/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL05ldHR5UmVtb3RpbmdDbGllbnQuamF2YQ==) | `50.00% <0.00%> (-2.78%)` | `9.00% <0.00%> (-2.00%)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4140?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4140?src=pr&el=footer). Last update [aa0974f...abf7eda](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4140?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] GitFireMan edited a comment on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
GitFireMan edited a comment on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-737009454


   @lgcareer I get your mean. To avoid an always failed high-priority task block master consumer. 
   Maybe we can use `public E poll(long timeout, TimeUnit unit)` .
   code like this
   `String taskPriorityInfo = taskPriorityQueue.poll(Constants.SLEEP_TIME_MILLIS, TimeUnit.MILLISECONDS);
                       if(Objects.isNull(taskPriorityInfo)){
                           continue;
                       }`
   
   And `TaskPriorityQueueImpl` add a method like this:
   `@Override
       public String poll(long timeout, TimeUnit unit) throws Exception {
           return queue.poll(timeout,unit);
       }`
   
   what do you think about?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] dailidong commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
dailidong commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-743746166


   @GitFireMan , good first contribution,  your implemention is better,  I like this.  
   by the way, this PR need to do update following the doc : https://dolphinscheduler.apache.org/zh-cn/docs/development/pull -request.html,  because the code-style not unify, and please solve confilct files locally
   
   ![image](https://user-images.githubusercontent.com/15833811/101983271-f8bf5280-3cb4-11eb-869d-6cd542f6e774.png)
   
   if you need any help , please contact me(lidongdai@apache.org or wechat:510570367) , when added, please tell me your github id
   
   you have done good job, let's make it better
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] CalvinKirs commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-738492886


   > @lgcareer I get your mean. To avoid an always failed high-priority task block master consumer.
   > Maybe we can use `public E poll(long timeout, TimeUnit unit)` .
   > code like this
   > `String taskPriorityInfo = taskPriorityQueue.poll(Constants.SLEEP_TIME_MILLIS, TimeUnit.MILLISECONDS); if(Objects.isNull(taskPriorityInfo)){ continue; }`
   > 
   > And `TaskPriorityQueueImpl` add a method like this:
   > `@Override public String poll(long timeout, TimeUnit unit) throws Exception { return queue.poll(timeout,unit); }`
   > 
   > what do you think about?
   
   Why do you need to do this? Generally speaking, this is a good solution, but it may not be appropriate here. It does not belong to the PriorityQueue method. To be precise, it does not belong to the PriorityQueue category.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] GitFireMan commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
GitFireMan commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-738523176


   @CalvinKirs When a new task put into the `queue`, sleep one second will make the comsumer waiting util one second pass.
   But use `poll(timeout,Unit)` can solve this question, and can have some performance promote when the Tasks comes low frequency.
   Also it can resolve than an always failed high-priority task block master consumer.
   
   Queue implement used here is `PriorityBlockingQueue` , and poll(timeout,Unit) belongs to `PriorityBlockingQueue` .
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] sonarcloud[bot] commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-738532333


   SonarCloud Quality Gate failed.
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/B.png' alt='B' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4140&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4140&metric=new_coverage&view=list)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4140&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4140&metric=new_duplicated_lines_density&view=list)
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] CalvinKirs commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-736606225


   @lgcareer hi, sleep 1 min,Is there another reason 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] dailidong closed pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
dailidong closed pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] sonarcloud[bot] removed a comment on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-736531342


   Kudos, SonarCloud Quality Gate passed!
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=BUG)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=VULNERABILITY)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=SECURITY_HOTSPOT)  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4140&resolved=false&types=CODE_SMELL)
   
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4140&metric=coverage&view=list) No Coverage information  
   [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4140&metric=duplicated_lines_density&view=list) No Duplication information
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] lgcareer commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
lgcareer commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-736968034


   > @lgcareer hi, sleep 1 min,Is there another reason here?
   
   If removed `sleep`,it will be blocked when here is no task in queue.If here is some failed dispatch tasks,it need add them to the queue back.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] dailidong commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
dailidong commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-773145201


   I will close this PR , please re-submit a new PR
   thanks for your first contribution


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] GitFireMan commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
GitFireMan commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-738527792


   > > @CalvinKirs When a new task put into the `queue`, sleep one second will make the comsumer waiting util one second pass.
   > > But use `poll(timeout,Unit)` can solve this question, and can have some performance promote when the Tasks comes low frequency.
   > > Also it can resolve that an always failed high-priority task block master consumer.
   > > Queue implement used here is `PriorityBlockingQueue` , and poll(timeout,Unit) belongs to `PriorityBlockingQueue` .
   > 
   > Sorry, I think you can do this.
   
   Can not do this ? 
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] GitFireMan edited a comment on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
GitFireMan edited a comment on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-738523176


   @CalvinKirs When a new task put into the `queue`, sleep one second will make the comsumer waiting util one second pass.
   But use `poll(timeout,Unit)` can solve this question, and can have some performance promote when the Tasks comes low frequency.
   Also it can resolve that an always failed high-priority task block master consumer.
   
   Queue implement used here is `PriorityBlockingQueue` , and poll(timeout,Unit) belongs to `PriorityBlockingQueue` .
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-dolphinscheduler] CalvinKirs commented on pull request #4140: [Improvement-4137] remove priorityQueue.size=0 sleep 1 second

Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #4140:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4140#issuecomment-738528829


   > > > @CalvinKirs When a new task put into the `queue`, sleep one second will make the comsumer waiting util one second pass.
   > > > But use `poll(timeout,Unit)` can solve this question, and can have some performance promote when the Tasks comes low frequency.
   > > > Also it can resolve that an always failed high-priority task block master consumer.
   > > > Queue implement used here is `PriorityBlockingQueue` , and poll(timeout,Unit) belongs to `PriorityBlockingQueue` .
   > > 
   > > 
   > > Sorry, I think you can do this.
   > 
   > Can not do this ?
   
   You can do as you said, sounds good to me, btw, code style chack fail, you can refer to this article to complete the modification https://dolphinscheduler.apache.org/zh-cn/docs/development/pull -request.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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org