You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "ankitsultana (via GitHub)" <gi...@apache.org> on 2023/02/22 21:25:22 UTC
[GitHub] [pinot] ankitsultana opened a new pull request, #10319: [multistage] Close OpChains That Return Error-Block
ankitsultana opened a new pull request, #10319:
URL: https://github.com/apache/pinot/pull/10319
I added this bug in the last PR. I have added a UT also to catch 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.
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 #10319: [multistage] Close OpChains That Return Error-Block
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10319:
URL: https://github.com/apache/pinot/pull/10319#issuecomment-1440903448
# [Codecov](https://codecov.io/gh/apache/pinot/pull/10319?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#10319](https://codecov.io/gh/apache/pinot/pull/10319?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8b9b62b) into [master](https://codecov.io/gh/apache/pinot/commit/1aa1368a1a4f6467a2e49866f3de51f4d519942d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1aa1368) will **increase** coverage by `0.02%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #10319 +/- ##
============================================
+ Coverage 70.19% 70.21% +0.02%
+ Complexity 5875 5854 -21
============================================
Files 2028 2027 -1
Lines 110084 109891 -193
Branches 16719 16685 -34
============================================
- Hits 77275 77162 -113
+ Misses 27380 27293 -87
- Partials 5429 5436 +7
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `24.45% <0.00%> (+0.04%)` | :arrow_up: |
| integration2 | `24.36% <0.00%> (+0.13%)` | :arrow_up: |
| unittests1 | `67.69% <100.00%> (-0.03%)` | :arrow_down: |
| unittests2 | `13.79% <0.00%> (+0.05%)` | :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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/10319?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...uery/runtime/executor/OpChainSchedulerService.java](https://codecov.io/gh/apache/pinot/pull/10319?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9leGVjdXRvci9PcENoYWluU2NoZWR1bGVyU2VydmljZS5qYXZh) | `96.29% <100.00%> (+5.55%)` | :arrow_up: |
| [...requesthandler/MultiStageBrokerRequestHandler.java](https://codecov.io/gh/apache/pinot/pull/10319?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvTXVsdGlTdGFnZUJyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `61.53% <0.00%> (-12.83%)` | :arrow_down: |
| [...che/pinot/core/query/reduce/BaseReduceService.java](https://codecov.io/gh/apache/pinot/pull/10319?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9yZWR1Y2UvQmFzZVJlZHVjZVNlcnZpY2UuamF2YQ==) | `88.23% <0.00%> (-9.17%)` | :arrow_down: |
| [...or/transform/function/IsNullTransformFunction.java](https://codecov.io/gh/apache/pinot/pull/10319?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXNOdWxsVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `79.31% <0.00%> (-6.90%)` | :arrow_down: |
| [...he/pinot/segment/local/segment/store/IndexKey.java](https://codecov.io/gh/apache/pinot/pull/10319?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L3N0b3JlL0luZGV4S2V5LmphdmE=) | `75.00% <0.00%> (-5.00%)` | :arrow_down: |
| [...l/segment/index/readers/OnHeapFloatDictionary.java](https://codecov.io/gh/apache/pinot/pull/10319?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvT25IZWFwRmxvYXREaWN0aW9uYXJ5LmphdmE=) | `86.36% <0.00%> (-4.55%)` | :arrow_down: |
| [...nction/DistinctCountBitmapAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/10319?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdENvdW50Qml0bWFwQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `46.63% <0.00%> (-3.11%)` | :arrow_down: |
| [...pache/pinot/query/planner/stage/AggregateNode.java](https://codecov.io/gh/apache/pinot/pull/10319?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9zdGFnZS9BZ2dyZWdhdGVOb2RlLmphdmE=) | `89.47% <0.00%> (-2.20%)` | :arrow_down: |
| [.../org/apache/pinot/core/startree/StarTreeUtils.java](https://codecov.io/gh/apache/pinot/pull/10319?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9TdGFyVHJlZVV0aWxzLmphdmE=) | `76.28% <0.00%> (-2.07%)` | :arrow_down: |
| [...core/startree/operator/StarTreeFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/10319?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9vcGVyYXRvci9TdGFyVHJlZUZpbHRlck9wZXJhdG9yLmphdmE=) | `91.44% <0.00%> (-1.32%)` | :arrow_down: |
| ... and [47 more](https://codecov.io/gh/apache/pinot/pull/10319?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
: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=The+Apache+Software+Foundation)
--
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] ankitsultana commented on a diff in pull request #10319: [multistage] Close OpChains That Return Error-Block
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #10319:
URL: https://github.com/apache/pinot/pull/10319#discussion_r1114989027
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/executor/OpChainSchedulerService.java:
##########
@@ -100,6 +102,7 @@ public void runJob() {
if (isFinished) {
closeOpChain(operatorChain);
} else if (thrown != null) {
+ // TODO: It would make sense to cancel OpChains if they returned an error-block.
Review Comment:
Yeah right now that's the case. In a follow-up I'll most likely change this behavior to:
```
finally {
if (thrown != null || returnedErrorBlock) {
cancelOpChain(operatorChain);
} else if (isFinished) {
closeOpChain(operatorChain);
}
}
```
--
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 #10319: [multistage] Close OpChains That Return Error-Block
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr merged PR #10319:
URL: https://github.com/apache/pinot/pull/10319
--
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] ankitsultana commented on a diff in pull request #10319: [multistage] Close OpChains That Return Error-Block
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #10319:
URL: https://github.com/apache/pinot/pull/10319#discussion_r1114984941
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/executor/OpChainSchedulerServiceTest.java:
##########
@@ -144,6 +144,7 @@ public void shouldScheduleOpChainEvenIfNoOpChainsInAWhile()
OpChain opChain = getChain(_operatorA);
CountDownLatch latch = new CountDownLatch(3);
AtomicBoolean returnedOpChain = new AtomicBoolean(false);
+ Mockito.when(_operatorA.nextBlock()).thenReturn(TransferableBlockUtils.getNoOpTransferableBlock());
Review Comment:
Added this because otherwise we were getting a silent NullPointerException logged in the UTs
--
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 #10319: [multistage] Close OpChains That Return Error-Block
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10319:
URL: https://github.com/apache/pinot/pull/10319#discussion_r1114987353
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/executor/OpChainSchedulerService.java:
##########
@@ -100,6 +102,7 @@ public void runJob() {
if (isFinished) {
closeOpChain(operatorChain);
} else if (thrown != null) {
+ // TODO: It would make sense to cancel OpChains if they returned an error-block.
Review Comment:
won't `isFinished` be set to true if opChain returns an error block?
--
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