You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "xiangfu0 (via GitHub)" <gi...@apache.org> on 2023/08/13 10:06:29 UTC
[GitHub] [pinot] xiangfu0 opened a new pull request, #11334: [multistage] Early terminate SortOperator if there is a limit
xiangfu0 opened a new pull request, #11334:
URL: https://github.com/apache/pinot/pull/11334
This is for join query or selection query without order by.
--
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] xiangfu0 commented on a diff in pull request #11334: [multistage] Early terminate SortOperator if there is a limit
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #11334:
URL: https://github.com/apache/pinot/pull/11334#discussion_r1294234098
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/SortOperator.java:
##########
@@ -175,6 +175,14 @@ private void consumeInputBlocks() {
_rows.addAll(container.subList(0, _numRowsToKeep - _rows.size()));
}
}
+ // '_fetch > 0' means this operator can early terminate, so we can stop consuming input blocks
+ if (_rows.size() >= _numRowsToKeep && _fetch > 0) {
+ _readyToConstruct = true;
+ _upstreamOperator.close();
Review Comment:
removed.
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/SortOperator.java:
##########
@@ -175,6 +175,14 @@ private void consumeInputBlocks() {
_rows.addAll(container.subList(0, _numRowsToKeep - _rows.size()));
}
}
+ // '_fetch > 0' means this operator can early terminate, so we can stop consuming input blocks
+ if (_rows.size() >= _numRowsToKeep && _fetch > 0) {
Review Comment:
updated.
--
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] xiangfu0 commented on a diff in pull request #11334: [multistage] Early terminate SortOperator if there is a limit
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #11334:
URL: https://github.com/apache/pinot/pull/11334#discussion_r1294234242
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/GrpcSendingMailbox.java:
##########
@@ -66,13 +67,19 @@ public void send(TransferableBlock block)
}
Preconditions.checkState(!_statusObserver.isFinished(), "Mailbox: %s is already closed", _id);
_contentObserver.onNext(toMailboxContent(block));
+ _isEarlyTerminated = _statusObserver.isFinished();
Review Comment:
removed
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/SendingMailbox.java:
##########
@@ -50,6 +50,11 @@ void send(TransferableBlock block)
*/
void complete();
+ /**
+ * Returns true if the mailbox has been early terminated. This can happen if the receiver has cancelled the mailbox
+ */
+ boolean isEarlyTerminated();
Review Comment:
removed.
--
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 #11334: [multistage] Early terminate SortOperator if there is a limit
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11334:
URL: https://github.com/apache/pinot/pull/11334#issuecomment-1676316754
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11334?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11334](https://app.codecov.io/gh/apache/pinot/pull/11334?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (ce76feb) into [master](https://app.codecov.io/gh/apache/pinot/commit/51da2f5f67607ecff4903356d5ef0da47ab5ac06?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (51da2f5) will **decrease** coverage by `53.59%`.
> Report is 1 commits behind head on master.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #11334 +/- ##
=============================================
- Coverage 68.18% 14.59% -53.59%
+ Complexity 6534 494 -6040
=============================================
Files 2239 2165 -74
Lines 120475 117318 -3157
Branches 18230 17784 -446
=============================================
- Hits 82141 17120 -65021
- Misses 32496 98781 +66285
+ Partials 5838 1417 -4421
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| java-11 | `?` | |
| java-17 | `?` | |
| java-20 | `14.59% <ø> (-53.41%)` | :arrow_down: |
| temurin | `14.59% <ø> (-53.59%)` | :arrow_down: |
| unittests1 | `?` | |
| unittests2 | `14.59% <ø> (-0.01%)` | :arrow_down: |
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.
[see 1711 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11334/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] Jackie-Jiang commented on a diff in pull request #11334: [multistage] Early terminate SortOperator if there is a limit
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #11334:
URL: https://github.com/apache/pinot/pull/11334#discussion_r1294110002
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/GrpcSendingMailbox.java:
##########
@@ -66,13 +67,19 @@ public void send(TransferableBlock block)
}
Preconditions.checkState(!_statusObserver.isFinished(), "Mailbox: %s is already closed", _id);
_contentObserver.onNext(toMailboxContent(block));
+ _isEarlyTerminated = _statusObserver.isFinished();
Review Comment:
This will almost always throw exception on line 68
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/SortOperator.java:
##########
@@ -175,6 +175,14 @@ private void consumeInputBlocks() {
_rows.addAll(container.subList(0, _numRowsToKeep - _rows.size()));
}
}
+ // '_fetch > 0' means this operator can early terminate, so we can stop consuming input blocks
+ if (_rows.size() >= _numRowsToKeep && _fetch > 0) {
+ _readyToConstruct = true;
+ _upstreamOperator.close();
Review Comment:
We should not close operator here
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/SendingMailbox.java:
##########
@@ -50,6 +50,11 @@ void send(TransferableBlock block)
*/
void complete();
+ /**
+ * Returns true if the mailbox has been early terminated. This can happen if the receiver has cancelled the mailbox
+ */
+ boolean isEarlyTerminated();
Review Comment:
I don't see this being used
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/SortOperator.java:
##########
@@ -175,6 +175,14 @@ private void consumeInputBlocks() {
_rows.addAll(container.subList(0, _numRowsToKeep - _rows.size()));
}
}
+ // '_fetch > 0' means this operator can early terminate, so we can stop consuming input blocks
+ if (_rows.size() >= _numRowsToKeep && _fetch > 0) {
Review Comment:
We don't allow unlimited rows. Checking `if (_rows.size() == _numRowsToKeep)` is good enough
--
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] Jackie-Jiang merged pull request #11334: [multistage] Early terminate SortOperator if there is a limit
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #11334:
URL: https://github.com/apache/pinot/pull/11334
--
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