You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "walterddr (via GitHub)" <gi...@apache.org> on 2023/06/07 20:06:14 UTC

[GitHub] [pinot] walterddr opened a new pull request, #10870: [multistage] register opChain finish callback for pipeline breaker to continue

walterddr opened a new pull request, #10870:
URL: https://github.com/apache/pinot/pull/10870

   this should fix #10829


-- 
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@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] codecov-commenter commented on pull request #10870: [multistage] register opChain finish callback for pipeline breaker to continue

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10870:
URL: https://github.com/apache/pinot/pull/10870#issuecomment-1581510188

   ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/10870?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   > Merging [#10870](https://app.codecov.io/gh/apache/pinot/pull/10870?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (3caaa99) into [master](https://app.codecov.io/gh/apache/pinot/commit/84688577e40ef34dcb2aadf558ec759ca80466da?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (8468857) will **decrease** coverage by `38.68%`.
   > The diff coverage is `0.00%`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             master   #10870       +/-   ##
   =============================================
   - Coverage     70.23%   31.55%   -38.68%     
   + Complexity     6585      453     -6132     
   =============================================
     Files          2170     2174        +4     
     Lines        116665   116880      +215     
     Branches      17656    17702       +46     
   =============================================
   - Hits          81942    36886    -45056     
   - Misses        29014    76634    +47620     
   + Partials       5709     3360     -2349     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `24.01% <0.00%> (-0.02%)` | :arrow_down: |
   | integration2 | `?` | |
   | unittests1 | `?` | |
   | unittests2 | `13.69% <0.00%> (+0.08%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://app.codecov.io/gh/apache/pinot/pull/10870?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [...ot/query/runtime/executor/RoundRobinScheduler.java](https://app.codecov.io/gh/apache/pinot/pull/10870?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9leGVjdXRvci9Sb3VuZFJvYmluU2NoZWR1bGVyLmphdmE=) | `0.00% <0.00%> (-92.69%)` | :arrow_down: |
   | [...g/apache/pinot/query/runtime/operator/OpChain.java](https://app.codecov.io/gh/apache/pinot/pull/10870?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9PcENoYWluLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...runtime/plan/pipeline/PipelineBreakerExecutor.java](https://app.codecov.io/gh/apache/pinot/pull/10870?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9wbGFuL3BpcGVsaW5lL1BpcGVsaW5lQnJlYWtlckV4ZWN1dG9yLmphdmE=) | `0.00% <0.00%> (-92.86%)` | :arrow_down: |
   | [...runtime/plan/pipeline/PipelineBreakerOperator.java](https://app.codecov.io/gh/apache/pinot/pull/10870?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9wbGFuL3BpcGVsaW5lL1BpcGVsaW5lQnJlYWtlck9wZXJhdG9yLmphdmE=) | `0.00% <0.00%> (-62.86%)` | :arrow_down: |
   
   ... and [1388 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/10870/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   


-- 
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@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] walterddr merged pull request #10870: [multistage] register opChain finish callback for pipeline breaker to continue

Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr merged PR #10870:
URL: https://github.com/apache/pinot/pull/10870


-- 
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@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] walterddr commented on a diff in pull request #10870: [multistage] register opChain finish callback for pipeline breaker to continue

Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10870:
URL: https://github.com/apache/pinot/pull/10870#discussion_r1222195019


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/executor/RoundRobinScheduler.java:
##########
@@ -169,6 +169,8 @@ public void deregister(OpChain operatorChain) {
     } finally {
       _lock.unlock();
     }
+    // invoke opChain deregister callback
+    operatorChain.getOpChainFinishCallback().accept(operatorChain.getId());

Review Comment:
   I dont actually like this way of doing things. 
   `opChain.finish` call back should be automatically called when deregistered. however this rely on a very specific impl of the Scheduler.deregister. 
   
   let's refactor this later as both the onDataAvai and onFinish callback seems a bit weird.



-- 
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@pinot.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org