Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SPARK-25863][SPARK-21871][SQL] Check if code size statistics is empty or not in updateAndGetCompilationStats #23947

Closed
wants to merge 1 commit into from
Closed

[SPARK-25863][SPARK-21871][SQL] Check if code size statistics is empty or not in updateAndGetCompilationStats #23947

wants to merge 1 commit into from

Conversation

maropu
Copy link
Member

@maropu maropu commented Mar 3, 2019

What changes were proposed in this pull request?

CodeGenerator.updateAndGetCompilationStats throws an unsupported exception for empty code size statistics. This pr added code to check if it is empty or not.

How was this patch tested?

Pass Jenkins.

@maropu
Copy link
Member Author

maropu commented Mar 3, 2019

@srowen

srowen
srowen approved these changes Mar 4, 2019
View reviewed changes
Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, was just wondering whether this is a case that should be handled normally or whether it actually indicated an earlier problem.

@maropu
Copy link
Member Author

maropu commented Mar 4, 2019

Yea, I think this is an earlier problem and just a bug I made. For example, in case that we cannot collect these stats in some reasons, codeSizes possibly gets empty.

gatorsmile
gatorsmile reviewed Mar 4, 2019
View reviewed changes
...atalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
if (codeSizes.nonEmpty) {
codeSizes.max
} else {
0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a test case? cc @rednaxelafx

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, no. I tried to add tests though, I couldn't find an input for this issue... Any idea, Kris?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my 2 cents - We only have this issue on our most heavy prod jobs with a lot of columns and a lot of partitions.. I wonder if Spark struggles to collect some stats in this case, and then this becomes zero. We don't have a reproducer as it again only happens on our heaviest prod jobs.. but it does happen consistently when we're over a certain limit of number of columns or number of partitions/tasks, or both. Thanks.

@SparkQA
Copy link

SparkQA commented Mar 4, 2019

Test build #102957 has finished for PR 23947 at commit c359b8d.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@maropu
Copy link
Member Author

maropu commented Mar 7, 2019

retest this please

@maropu
Copy link
Member Author

maropu commented Mar 7, 2019

Pending, Jenkins. If anyone can add test cases, please open a pr as follow-up.

@SparkQA
Copy link

SparkQA commented Mar 7, 2019

Test build #103115 has finished for PR 23947 at commit c359b8d.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@maropu
Copy link
Member Author

maropu commented Mar 7, 2019 •
edited
Loading

Merged to master/branch-2.4/branch-2.3.

@maropu maropu closed this Mar 7, 2019
maropu added a commit that referenced this pull request Mar 7, 2019
@maropu
[SPARK-25863][SPARK-21871][SQL] Check if code size statistics is empt…
315c95c 
…y or not in updateAndGetCompilationStats

## What changes were proposed in this pull request?
`CodeGenerator.updateAndGetCompilationStats` throws an unsupported exception for empty code size statistics. This pr added code to check if it is empty or not.

## How was this patch tested?
Pass Jenkins.

Closes #23947 from maropu/SPARK-21871-FOLLOWUP.

Authored-by: Takeshi Yamamuro <[email protected]>
Signed-off-by: Takeshi Yamamuro <[email protected]>
maropu added a commit that referenced this pull request Mar 7, 2019
@maropu
[SPARK-25863][SPARK-21871][SQL] Check if code size statistics is empt…
dfde0c6 
…y or not in updateAndGetCompilationStats

## What changes were proposed in this pull request?
`CodeGenerator.updateAndGetCompilationStats` throws an unsupported exception for empty code size statistics. This pr added code to check if it is empty or not.

## How was this patch tested?
Pass Jenkins.

Closes #23947 from maropu/SPARK-21871-FOLLOWUP.

Authored-by: Takeshi Yamamuro <[email protected]>
Signed-off-by: Takeshi Yamamuro <[email protected]>
maropu added a commit that referenced this pull request Mar 7, 2019
@maropu
[SPARK-25863][SPARK-21871][SQL] Check if code size statistics is empt…
f7ad4ff 
…y or not in updateAndGetCompilationStats

## What changes were proposed in this pull request?
`CodeGenerator.updateAndGetCompilationStats` throws an unsupported exception for empty code size statistics. This pr added code to check if it is empty or not.

## How was this patch tested?
Pass Jenkins.

Closes #23947 from maropu/SPARK-21871-FOLLOWUP.

Authored-by: Takeshi Yamamuro <[email protected]>
Signed-off-by: Takeshi Yamamuro <[email protected]>
rdblue pushed a commit to rdblue/spark that referenced this pull request Apr 3, 2019
@maropu @rdblue
[SPARK-25863][SPARK-21871][SQL] Check if code size statistics is empt…
d2eeca4 
…y or not in updateAndGetCompilationStats

## What changes were proposed in this pull request?
`CodeGenerator.updateAndGetCompilationStats` throws an unsupported exception for empty code size statistics. This pr added code to check if it is empty or not.

## How was this patch tested?
Pass Jenkins.

Closes apache#23947 from maropu/SPARK-21871-FOLLOWUP.

Authored-by: Takeshi Yamamuro <[email protected]>
Signed-off-by: Takeshi Yamamuro <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Jul 23, 2019
@maropu @kai-chi
[SPARK-25863][SPARK-21871][SQL] Check if code size statistics is empt…
1bfe52d 
…y or not in updateAndGetCompilationStats

## What changes were proposed in this pull request?
`CodeGenerator.updateAndGetCompilationStats` throws an unsupported exception for empty code size statistics. This pr added code to check if it is empty or not.

## How was this patch tested?
Pass Jenkins.

Closes apache#23947 from maropu/SPARK-21871-FOLLOWUP.

Authored-by: Takeshi Yamamuro <[email protected]>
Signed-off-by: Takeshi Yamamuro <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Jul 25, 2019
@maropu @kai-chi
[SPARK-25863][SPARK-21871][SQL] Check if code size statistics is empt…
461a8c8 
…y or not in updateAndGetCompilationStats

## What changes were proposed in this pull request?
`CodeGenerator.updateAndGetCompilationStats` throws an unsupported exception for empty code size statistics. This pr added code to check if it is empty or not.

## How was this patch tested?
Pass Jenkins.

Closes apache#23947 from maropu/SPARK-21871-FOLLOWUP.

Authored-by: Takeshi Yamamuro <[email protected]>
Signed-off-by: Takeshi Yamamuro <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Aug 1, 2019
@maropu @kai-chi
[SPARK-25863][SPARK-21871][SQL] Check if code size statistics is empt…
08c4494 
…y or not in updateAndGetCompilationStats

## What changes were proposed in this pull request?
`CodeGenerator.updateAndGetCompilationStats` throws an unsupported exception for empty code size statistics. This pr added code to check if it is empty or not.

## How was this patch tested?
Pass Jenkins.

Closes apache#23947 from maropu/SPARK-21871-FOLLOWUP.

Authored-by: Takeshi Yamamuro <[email protected]>
Signed-off-by: Takeshi Yamamuro <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tagar Tagar Tagar left review comments

@gatorsmile gatorsmile gatorsmile left review comments

@srowen srowen srowen approved these changes

@kiszk kiszk kiszk approved these changes

@HyukjinKwon HyukjinKwon HyukjinKwon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@maropu @SparkQA @srowen @kiszk @Tagar @HyukjinKwon @gatorsmile