Skip to content

[multistage] fix error propagate physical planner#11173

Merged
walterddr merged 2 commits intoapache:masterfrom
walterddr:pr_fix_error_propagate_physical_planner
Jul 28, 2023
Merged

[multistage] fix error propagate physical planner#11173
walterddr merged 2 commits intoapache:masterfrom
walterddr:pr_fix_error_propagate_physical_planner

Conversation

@walterddr
Copy link
Copy Markdown
Contributor

@walterddr walterddr commented Jul 25, 2023

Details

  • adding submission service to generically wait for all Future to return and check for success or failure
  • propagate failure messages along

TODO

  • also refactor broker QueryDispatcher side logic to unify with the QueryServer side behavior (using SubmissionService)

this fixes: #11167

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 25, 2023

Codecov Report

Merging #11173 (ae12033) into master (4a12ebf) will decrease coverage by 0.01%.
Report is 6 commits behind head on master.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master   #11173      +/-   ##
==========================================
- Coverage    0.11%    0.11%   -0.01%     
==========================================
  Files        2223     2225       +2     
  Lines      119273   119377     +104     
  Branches    18055    18071      +16     
==========================================
  Hits          137      137              
- Misses     119116   119220     +104     
  Partials       20       20              
Flag Coverage Δ
integration1temurin11 0.00% <0.00%> (ø)
integration1temurin17 0.00% <0.00%> (ø)
integration1temurin20 0.00% <0.00%> (ø)
integration2temurin11 ?
integration2temurin17 0.00% <0.00%> (?)
integration2temurin20 0.00% <0.00%> (ø)
unittests1temurin11 0.00% <0.00%> (ø)
unittests1temurin17 0.00% <0.00%> (ø)
unittests1temurin20 0.00% <0.00%> (ø)
unittests2temurin11 0.11% <0.00%> (-0.01%) ⬇️
unittests2temurin17 0.11% <0.00%> (-0.01%) ⬇️
unittests2temurin20 0.11% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
.../apache/pinot/query/service/SubmissionService.java 0.00% <0.00%> (ø)
...apache/pinot/query/service/server/QueryServer.java 0.00% <0.00%> (ø)
.../apache/pinot/server/worker/WorkerQueryServer.java 0.00% <ø> (ø)

... and 18 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@walterddr walterddr marked this pull request as ready for review July 25, 2023 21:05
@walterddr walterddr force-pushed the pr_fix_error_propagate_physical_planner branch 2 times, most recently from 72b4191 to d60f73a Compare July 27, 2023 20:59
@walterddr walterddr force-pushed the pr_fix_error_propagate_physical_planner branch 2 times, most recently from 4b756d7 to 814a9b6 Compare July 27, 2023 22:24
@walterddr walterddr force-pushed the pr_fix_error_propagate_physical_planner branch from 814a9b6 to ae12033 Compare July 27, 2023 22:26
@walterddr walterddr merged commit 35328c0 into apache:master Jul 28, 2023
s0nskar pushed a commit to s0nskar/pinot that referenced this pull request Aug 10, 2023
* [multistage] propagate planner error back to broker
* use CompletableFuture to monitor and cancel stages when error occurs.

---------

Co-authored-by: Rong Rong <rongr@startree.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[multistage] physical plan compile error not piped back to broker

4 participants