[hotfix][multistage] fix error not returned issue#9760
Merged
siddharthteotia merged 1 commit intoapache:masterfrom Nov 8, 2022
Merged
[hotfix][multistage] fix error not returned issue#9760siddharthteotia merged 1 commit intoapache:masterfrom
siddharthteotia merged 1 commit intoapache:masterfrom
Conversation
walterddr
changed the title
agavra
approved these changes
Nov 8, 2022
Contributor
agavra
left a comment
agavra
left a comment
There was a problem hiding this comment.
Thanks @walterddr - lost this one in translation! I'll definitely work on adding unit tests for all the operators...
LGTM, one question inline
| if (!block.isEndOfStreamBlock()) { | ||
| return block; | ||
| } else { | ||
| eosCount++; |
Contributor
There was a problem hiding this comment.
I think we can actually remove eosCount - I forgot to do that when I removed the check below
| "ArithmeticFunctions.least(double,double) with arguments"}, | ||
| // Function that tries to cast String to Number should throw runtime exception | ||
| new Object[]{"SELECT a.col2, b.col1 FROM a JOIN b ON a.col1 = b.col3", "transform function: cast"}, | ||
| // TODO: this error is thrown but not returned through mailbox. need another test for asserting failure |
Contributor
There was a problem hiding this comment.
I'm confused - what's the new behavior like if we don't throw an exception?
Contributor
Author
There was a problem hiding this comment.
this maybe related to 2 issues @61yao filed: #9671 and #9723
after queryplan is generated.
- QueryDispatcher dispatches the query stages to servers
- b/c of this behavior, the query dispatch API referred to in [multi-stage] [stability] QueryDispatcher Grpc needs to have a deadline and should be async #9671 (which is not async) doesn't need to be async since it return immediately after dispatch successful & the GRPC context return immediately after the query plan is valid.
- however it also should assert initialization of the query plan has no exception, which is what is doesn't do, and what this TODO meant to fix.
- Server executes the query and returns data via mailboxes
- once the query plan can be fully translate to an executable operator chain, any error occur during this will be returned by mailbox and it will bubble up eventually. this is what this PR is trying to fix.
Contributor
Author
There was a problem hiding this comment.
echo on the 2 issues filed.
- [multi-stage] [stability] QueryDispatcher Grpc needs to have a deadline and should be async #9671 dispatch GRPC doesn't need to be async as it doesn't wait for stage complete, it wait for stage validation complete
- [multi-stage] [error-handling] MailboxContentStreamObserver on error doesn't close rpc stream #9723 this occurs when mailboxsender or any of the downstream operators throw an exception outsideof the "nextBlock()" method, for example the init() method. thus the RPC stream is never closed.
Contributor
There was a problem hiding this comment.
Sorry, just saw this. Can you explain more on why it doesn't need to be async?
siddharthteotia
approved these changes
Nov 8, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
data block error were not returned properly. this PR fixes the issue, (seems to be introduced in #9484)