You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/09/12 22:41:04 UTC
[GitHub] [pinot] jasperjiaguo opened a new pull request, #9386: Terminate the query after plan generation if timeout
jasperjiaguo opened a new pull request, #9386:
URL: https://github.com/apache/pinot/pull/9386
End query execution early if timeout reached after planning.
--
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] siddharthteotia commented on a diff in pull request #9386: Terminate the query after plan generation if timeout
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on code in PR #9386:
URL: https://github.com/apache/pinot/pull/9386#discussion_r969052587
##########
pinot-core/src/test/java/org/apache/pinot/queries/BaseQueriesTest.java:
##########
@@ -264,10 +271,22 @@ private BrokerResponseNative getBrokerResponseDistinctInstances(PinotQuery pinot
Plan plan1 = planMaker.makeInstancePlan(instances.get(0), serverQueryContext, EXECUTOR_SERVICE, null);
Plan plan2 = planMaker.makeInstancePlan(instances.get(1), serverQueryContext, EXECUTOR_SERVICE, null);
- DataTable instanceResponse1 =
- queryContext.isExplain() ? ServerQueryExecutorV1Impl.processExplainPlanQueries(plan1) : plan1.execute();
- DataTable instanceResponse2 =
- queryContext.isExplain() ? ServerQueryExecutorV1Impl.processExplainPlanQueries(plan2) : plan2.execute();
+ DataTable instanceResponse1;
+ try {
+ instanceResponse1 =
+ queryContext.isExplain() ? ServerQueryExecutorV1Impl.processExplainPlanQueries(plan1) : plan1.execute();
+ } catch (TimeoutException e) {
+ e.printStackTrace();
Review Comment:
We don't typically do `e.printStackTrace();`
--
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] jasperjiaguo commented on a diff in pull request #9386: Terminate the query after plan generation if timeout
Posted by GitBox <gi...@apache.org>.
jasperjiaguo commented on code in PR #9386:
URL: https://github.com/apache/pinot/pull/9386#discussion_r969062370
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/GlobalPlanImplV0.java:
##########
@@ -45,11 +46,15 @@ public PlanNode getPlanNode() {
}
@Override
- public DataTable execute() {
+ public DataTable execute()
+ throws TimeoutException {
long startTime = System.currentTimeMillis();
InstanceResponseOperator instanceResponseOperator = _instanceResponsePlanNode.run();
long endTime1 = System.currentTimeMillis();
LOGGER.debug("InstanceResponsePlanNode.run() took: {}ms", endTime1 - startTime);
+ if (endTime1 > _instanceResponsePlanNode._queryContext.getEndTimeMs()) {
+ throw new TimeoutException("Timeout after plan generation");
Review Comment:
Done
--
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 #9386: Terminate the query after plan generation if timeout
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9386:
URL: https://github.com/apache/pinot/pull/9386#issuecomment-1244811173
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9386?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 [#9386](https://codecov.io/gh/apache/pinot/pull/9386?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6d60383) into [master](https://codecov.io/gh/apache/pinot/commit/987480ba2d6c6575ccce06aa10702a73e6a28f9a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (987480b) will **decrease** coverage by `48.07%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #9386 +/- ##
=============================================
- Coverage 63.44% 15.36% -48.08%
+ Complexity 5069 170 -4899
=============================================
Files 1832 1832
Lines 98122 98147 +25
Branches 15017 15021 +4
=============================================
- Hits 62250 15079 -47171
- Misses 31283 81924 +50641
+ Partials 4589 1144 -3445
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `?` | |
| unittests2 | `15.36% <0.00%> (+<0.01%)` | :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/9386?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/pinot/core/plan/GlobalPlanImplV0.java](https://codecov.io/gh/apache/pinot/pull/9386/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0dsb2JhbFBsYW5JbXBsVjAuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...src/main/java/org/apache/pinot/sql/FilterKind.java](https://codecov.io/gh/apache/pinot/pull/9386/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvRmlsdGVyS2luZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/9386/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...in/java/org/apache/pinot/spi/utils/BytesUtils.java](https://codecov.io/gh/apache/pinot/pull/9386/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQnl0ZXNVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...n/java/org/apache/pinot/core/data/table/Table.java](https://codecov.io/gh/apache/pinot/pull/9386/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1RhYmxlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/9386/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/9386/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/9386/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/BaseRecording.java](https://codecov.io/gh/apache/pinot/pull/9386/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvQmFzZVJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/NoOpRecording.java](https://codecov.io/gh/apache/pinot/pull/9386/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvTm9PcFJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1179 more](https://codecov.io/gh/apache/pinot/pull/9386/diff?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] jasperjiaguo commented on a diff in pull request #9386: Terminate the query after plan generation if timeout
Posted by GitBox <gi...@apache.org>.
jasperjiaguo commented on code in PR #9386:
URL: https://github.com/apache/pinot/pull/9386#discussion_r969005089
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/GlobalPlanImplV0.java:
##########
@@ -49,6 +49,9 @@ public DataTable execute() {
long startTime = System.currentTimeMillis();
InstanceResponseOperator instanceResponseOperator = _instanceResponsePlanNode.run();
long endTime1 = System.currentTimeMillis();
+ if (endTime1 > _instanceResponsePlanNode._queryContext.getEndTimeMs()) {
+ throw new RuntimeException("Timeout after plan generation");
Review Comment:
Done
--
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] jasperjiaguo commented on a diff in pull request #9386: Terminate the query after plan generation if timeout
Posted by GitBox <gi...@apache.org>.
jasperjiaguo commented on code in PR #9386:
URL: https://github.com/apache/pinot/pull/9386#discussion_r969004921
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/GlobalPlanImplV0.java:
##########
@@ -49,6 +49,9 @@ public DataTable execute() {
long startTime = System.currentTimeMillis();
InstanceResponseOperator instanceResponseOperator = _instanceResponsePlanNode.run();
long endTime1 = System.currentTimeMillis();
+ if (endTime1 > _instanceResponsePlanNode._queryContext.getEndTimeMs()) {
Review Comment:
good point, moved
--
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] jasperjiaguo commented on a diff in pull request #9386: Terminate the query after plan generation if timeout
Posted by GitBox <gi...@apache.org>.
jasperjiaguo commented on code in PR #9386:
URL: https://github.com/apache/pinot/pull/9386#discussion_r969057027
##########
pinot-core/src/test/java/org/apache/pinot/queries/BaseQueriesTest.java:
##########
@@ -264,10 +271,22 @@ private BrokerResponseNative getBrokerResponseDistinctInstances(PinotQuery pinot
Plan plan1 = planMaker.makeInstancePlan(instances.get(0), serverQueryContext, EXECUTOR_SERVICE, null);
Plan plan2 = planMaker.makeInstancePlan(instances.get(1), serverQueryContext, EXECUTOR_SERVICE, null);
- DataTable instanceResponse1 =
- queryContext.isExplain() ? ServerQueryExecutorV1Impl.processExplainPlanQueries(plan1) : plan1.execute();
- DataTable instanceResponse2 =
- queryContext.isExplain() ? ServerQueryExecutorV1Impl.processExplainPlanQueries(plan2) : plan2.execute();
+ DataTable instanceResponse1;
+ try {
+ instanceResponse1 =
+ queryContext.isExplain() ? ServerQueryExecutorV1Impl.processExplainPlanQueries(plan1) : plan1.execute();
+ } catch (TimeoutException e) {
+ e.printStackTrace();
Review Comment:
fixed
--
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] jasperjiaguo commented on a diff in pull request #9386: Terminate the query after plan generation if timeout
Posted by GitBox <gi...@apache.org>.
jasperjiaguo commented on code in PR #9386:
URL: https://github.com/apache/pinot/pull/9386#discussion_r969049354
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/GlobalPlanImplV0.java:
##########
@@ -49,6 +49,9 @@ public DataTable execute() {
long startTime = System.currentTimeMillis();
InstanceResponseOperator instanceResponseOperator = _instanceResponsePlanNode.run();
long endTime1 = System.currentTimeMillis();
+ if (endTime1 > _instanceResponsePlanNode._queryContext.getEndTimeMs()) {
+ throw new RuntimeException("Timeout after plan generation");
Review Comment:
Caught and print in [BaseQueriesTest.java](https://github.com/apache/pinot/pull/9386/commits/e17f3492f78a7505f2fa1971b05ea7f8851ffde4#diff-a418235c421d9d09b2328265ddedb7424839f9a6722f8bf4dacacba624f5f1ab) or it will incur too many changes for test cases
--
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] siddharthteotia commented on pull request #9386: Terminate the query after plan generation if timeout
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on PR #9386:
URL: https://github.com/apache/pinot/pull/9386#issuecomment-1244729926
What about `timeOut` during per segment `planNode` creation, that is before execute is called and physical operators are created. ? Can happen for extremely large number of segments I guess.
```
Plan queryPlan =
enableStreaming ? _planMaker.makeStreamingInstancePlan(selectedSegments, queryContext, executorService,
responseObserver, _serverMetrics) : _planMaker.makeInstancePlan(selectedSegments, queryContext,
executorService, _serverMetrics);
planBuildTimer.stopAndRecord();
```
^^ can potentially timeOut
```
TimerContext.Timer planExecTimer = timerContext.startNewPhaseTimer(ServerQueryPhase.QUERY_PLAN_EXECUTION);
DataTable dataTable = queryContext.isExplain() ? processExplainPlanQueries(queryPlan) : queryPlan.execute();
```
--
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] jasperjiaguo commented on pull request #9386: Terminate the query after plan generation if timeout
Posted by GitBox <gi...@apache.org>.
jasperjiaguo commented on PR #9386:
URL: https://github.com/apache/pinot/pull/9386#issuecomment-1244772891
> What about `timeOut` during per segment `planNode` creation, that is before `execute` is called and physical operators are created ? Can happen for extremely large number of segments I guess.
>
> ```
> Plan queryPlan =
> enableStreaming ? _planMaker.makeStreamingInstancePlan(selectedSegments, queryContext, executorService,
> responseObserver, _serverMetrics) : _planMaker.makeInstancePlan(selectedSegments, queryContext,
> executorService, _serverMetrics);
> planBuildTimer.stopAndRecord();
> ```
>
> ^^ can potentially timeOut
>
> ```
> TimerContext.Timer planExecTimer = timerContext.startNewPhaseTimer(ServerQueryPhase.QUERY_PLAN_EXECUTION);
> DataTable dataTable = queryContext.isExplain() ? processExplainPlanQueries(queryPlan) : queryPlan.execute();
> ```
The work here I think is to do `makeSegmentPlanNode` for each segment to create top level plan nodes, shouldn't be too much work?
--
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] siddharthteotia commented on a diff in pull request #9386: Terminate the query after plan generation if timeout
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on code in PR #9386:
URL: https://github.com/apache/pinot/pull/9386#discussion_r969060973
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/GlobalPlanImplV0.java:
##########
@@ -45,11 +46,15 @@ public PlanNode getPlanNode() {
}
@Override
- public DataTable execute() {
+ public DataTable execute()
+ throws TimeoutException {
long startTime = System.currentTimeMillis();
InstanceResponseOperator instanceResponseOperator = _instanceResponsePlanNode.run();
long endTime1 = System.currentTimeMillis();
LOGGER.debug("InstanceResponsePlanNode.run() took: {}ms", endTime1 - startTime);
+ if (endTime1 > _instanceResponsePlanNode._queryContext.getEndTimeMs()) {
+ throw new TimeoutException("Timeout after plan generation");
Review Comment:
(nit) suggest changing this to `"Query timed out while generating physical execution plan"`
--
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] vvivekiyer commented on a diff in pull request #9386: Terminate the query after plan generation if timeout
Posted by GitBox <gi...@apache.org>.
vvivekiyer commented on code in PR #9386:
URL: https://github.com/apache/pinot/pull/9386#discussion_r969002649
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/GlobalPlanImplV0.java:
##########
@@ -49,6 +49,9 @@ public DataTable execute() {
long startTime = System.currentTimeMillis();
InstanceResponseOperator instanceResponseOperator = _instanceResponsePlanNode.run();
long endTime1 = System.currentTimeMillis();
+ if (endTime1 > _instanceResponsePlanNode._queryContext.getEndTimeMs()) {
+ throw new RuntimeException("Timeout after plan generation");
Review Comment:
Should we throw a TimeoutException?
--
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] siddharthteotia commented on pull request #9386: Terminate the query after plan generation if timeout
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on PR #9386:
URL: https://github.com/apache/pinot/pull/9386#issuecomment-1244790320
> The work here I think is to do makeSegmentPlanNode for each segment to create top level plan nodes, shouldn't be too much work?
Yes I agree. Not a big deal. I was overthinking it
--
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] siddharthteotia merged pull request #9386: Terminate the query after plan generation if timeout
Posted by GitBox <gi...@apache.org>.
siddharthteotia merged PR #9386:
URL: https://github.com/apache/pinot/pull/9386
--
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] vvivekiyer commented on a diff in pull request #9386: Terminate the query after plan generation if timeout
Posted by GitBox <gi...@apache.org>.
vvivekiyer commented on code in PR #9386:
URL: https://github.com/apache/pinot/pull/9386#discussion_r969002991
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/GlobalPlanImplV0.java:
##########
@@ -49,6 +49,9 @@ public DataTable execute() {
long startTime = System.currentTimeMillis();
InstanceResponseOperator instanceResponseOperator = _instanceResponsePlanNode.run();
long endTime1 = System.currentTimeMillis();
+ if (endTime1 > _instanceResponsePlanNode._queryContext.getEndTimeMs()) {
Review Comment:
(nit): Perhaps move below LOGGER.debug
--
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