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-4333][SQL] Correctly log number of iterations in RuleExecutor #3180

Closed
wants to merge 2 commits into from
Closed

[SPARK-4333][SQL] Correctly log number of iterations in RuleExecutor #3180

wants to merge 2 commits into from

Conversation

pzzs
Copy link
Contributor

@pzzs pzzs commented Nov 10, 2014

When iterator of RuleExecutor breaks, the num of iterator should be (iteration - 1) not (iteration ).Because log looks like "Fixed point reached for batch ${batch.name} after 3 iterations.", but it did 2 iterations really!

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@srowen
Copy link
Member

srowen commented Nov 10, 2014

Looks good. I find the handling of this variable a little funky as it reasons about its next value in the current iteration.

@pzzs pzzs mentioned this pull request Nov 10, 2014
Closed
@pzzs pzzs changed the title RuleExecutor breaks, num of iterator should be iterator -1, because initil value is 1. RuleExecutor breaks, num of iteration should be (iteration -1).Because log looks like "Fixed point reached for batch ${batch.name} after 3 iterations.", but it did 2 iterations really! Nov 10, 2014
@marmbrus
Copy link
Contributor

ok to test

@marmbrus
Copy link
Contributor

Can you open a JIRA and change the title to something like:

[SPARK-XXX][SQL] Correctly log number of iterations in RuleExecutor

@SparkQA
Copy link

SparkQA commented Nov 11, 2014

Test build #23176 has started for PR 3180 at commit 46514b6.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 11, 2014

Test build #23176 has finished for PR 3180 at commit 46514b6.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23176/
Test FAILed.

@marmbrus
Copy link
Contributor

[error] /home/jenkins/workspace/SparkPullRequestBuilder@4/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala:82: File line length exceeds 100 characters

@pzzs
Copy link
Contributor Author

pzzs commented Nov 11, 2014

@marmbrus ok i will change it.

@pzzs pzzs changed the title RuleExecutor breaks, num of iteration should be (iteration -1).Because log looks like "Fixed point reached for batch ${batch.name} after 3 iterations.", but it did 2 iterations really! [SPARK-4333][SQL] Change num of iteration printed in trace log from ${iteration} to ${iteration - 1} Nov 11, 2014
@pzzs
Update RuleExecutor.scala
571e2ed 
File line length can not exceeds 100 characters, so i split it to two lines.
@pzzs pzzs changed the title [SPARK-4333][SQL] Change num of iteration printed in trace log from ${iteration} to ${iteration - 1} [SPARK-4333][SQL] Correctly log number of iterations in RuleExecutor Nov 11, 2014
@SparkQA
Copy link

SparkQA commented Nov 11, 2014

Test build #23189 has started for PR 3180 at commit 571e2ed.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 11, 2014

Test build #23189 has finished for PR 3180 at commit 571e2ed.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23189/
Test PASSed.

@pzzs
Copy link
Contributor Author

pzzs commented Nov 11, 2014

thanks @srowen @marmbrus

@pzzs
Copy link
Contributor Author

pzzs commented Nov 12, 2014

Test PASSed.Could you merge this patch? @marmbrus @srowen

@srowen srowen mentioned this pull request Nov 14, 2014
Closed
@marmbrus
Copy link
Contributor

Thanks! Merged to master and 1.2

asfgit pushed a commit that referenced this pull request Nov 14, 2014
@pzzs @marmbrus
[SPARK-4333][SQL] Correctly log number of iterations in RuleExecutor
f8810b6 
When iterator of RuleExecutor breaks, the num of iterator should be (iteration - 1) not (iteration ).Because log looks like "Fixed point reached for batch ${batch.name} after 3 iterations.", but it did 2 iterations really!

Author: DoingDone9 <[email protected]>

Closes #3180 from DoingDone9/issue_01 and squashes the following commits:

571e2ed [DoingDone9] Update RuleExecutor.scala
46514b6 [DoingDone9] When iterator of RuleExecutor breaks, the num of iterator should be iteration - 1 not iteration.

(cherry picked from commit 0cbdb01)
Signed-off-by: Michael Armbrust <[email protected]>
@asfgit asfgit closed this in 0cbdb01 Nov 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pzzs @AmplabJenkins @srowen @marmbrus @SparkQA