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.

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

[SPARK-1021] Defer the data-driven computation of partition bounds in so... #3079

Closed
wants to merge 2 commits into from

Conversation

erikerlandson
Copy link
Contributor

...rtByKey() until evaluation.

@erikerlandson
Copy link
Contributor Author

Reboot of #1689

@SparkQA
Copy link

SparkQA commented Nov 3, 2014

Test build #22828 has started for PR 3079 at commit 019ac27.

  • This patch merges cleanly.

@erikerlandson
Copy link
Contributor Author

@marmbrus, @scwf, FWIW, the correlationoptimizer14 test appears to be working for me. I ran it using: env _RUN_SQL_TESTS=true _SQL_TESTS_ONLY=true ./dev/run-tests > ~/run-tests-1021.txt 2>&1

Not sure why, but running sbt -Dspark.hive.whitelist=correlationoptimizer14 hive/test was not causing the test to run in my environment

@marmbrus
Copy link
Contributor

marmbrus commented Nov 3, 2014

@erikerlandson I think you also need -Phive for the tests to run. It is possible some other things changed (or even that that test case changed with the upgrade to hive 13). Perhaps you can include a dummy change to the Hive code to make sure those tests are run in this PR so we can see what Jenkins thinks?

@SparkQA
Copy link

SparkQA commented Nov 3, 2014

Test build #22828 has finished for PR 3079 at commit 019ac27.

  • This patch fails Spark unit 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/22828/
Test FAILed.

@SparkQA
Copy link

SparkQA commented Nov 4, 2014

Test build #22880 has started for PR 3079 at commit 0fc30fe.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 4, 2014

Test build #22880 has finished for PR 3079 at commit 0fc30fe.

  • This patch fails RAT 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/22880/
Test FAILed.

@SparkQA
Copy link

SparkQA commented Nov 4, 2014

Test build #22892 has started for PR 3079 at commit 2183325.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 4, 2014

Test build #22892 has finished for PR 3079 at commit 2183325.

  • This patch fails Spark unit 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/22892/
Test FAILed.

@@ -113,8 +117,12 @@ class RangePartitioner[K : Ordering : ClassTag, V](
private var ordering = implicitly[Ordering[K]]

// An array of upper bounds for the first (partitions - 1) partitions
private var rangeBounds: Array[K] = {
if (partitions <= 1) {
@volatile private var valRB: Array[K] = null
Copy link
Contributor

Choose a reason for hiding this comment

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

valRD is a kinda confusing name. I think the convention would be to name it _rangeBounds. Eg.

https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/FutureAction.scala#L111

@erikerlandson
Copy link
Contributor Author

For reference, this other issue has some overlap:
https://issues.apache.org/jira/browse/SPARK-4514

@marmbrus
Copy link
Contributor

hi @erikerlandson, thanks for working on this. It would be great to have a solution to this long running problem. Since it looks like there is still some work to be done, I propose we close this issue for now. We are on a renewed mission to keep the PR queue small and limited to active PRs so that things don't fall through the cracks.

@marmbrus
Copy link
Contributor

Do please reopen though once you having something that is passing tests :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants