Skip to content

[SPARK-10430][Spark Core]Added hashCode methods in AccumulableInfo and RDDOperationScope #8581

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

Closed
wants to merge 3 commits into from
Closed

[SPARK-10430][Spark Core]Added hashCode methods in AccumulableInfo and RDDOperationScope #8581

wants to merge 3 commits into from

Conversation

vinodkc
Copy link
Contributor

@vinodkc vinodkc commented Sep 3, 2015

No description provided.

@vinodkc vinodkc changed the title [SPARK-10430][SPARK-10430]Added hashCode methods in AccumulableInfo and RDDOperationScope [SPARK-10430][Spark Core]Added hashCode methods in AccumulableInfo and RDDOperationScope Sep 3, 2015
@andrewor14
Copy link
Contributor

ok to test

test("equals and hashCode") {
val opScope1 = new RDDOperationScope("scope1", id = "1")
val opScope2 = new RDDOperationScope("scope1", id = "1")
assert(opScope1 == opScope2)
Copy link
Contributor

Choose a reason for hiding this comment

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

===

@andrewor14
Copy link
Contributor

Looks good, thanks for fixing this.

@SparkQA
Copy link

SparkQA commented Sep 3, 2015

Test build #41961 has finished for PR 8581 at commit 25c0f7b.

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

@SparkQA
Copy link

SparkQA commented Sep 3, 2015

Test build #41962 has finished for PR 8581 at commit 56be40b.

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


override def hashCode(): Int = {
val state = Seq(id, name, update, value, internal)
state.map(_.hashCode).reduce(31 * _ + _)
Copy link
Member

Choose a reason for hiding this comment

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

Ah, something just occurred to me: you've been using reduce here but the operation isn't associative or commutative. I think it has to be reduceLeft to guarantee the output you want. Maybe update other instances of this in the code (from other PRs) here too?

@SparkQA
Copy link

SparkQA commented Sep 3, 2015

Test build #41964 has finished for PR 8581 at commit 3822bd3.

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

@andrewor14
Copy link
Contributor

LGTM merging into master thanks.

@asfgit asfgit closed this in 11ef32c Sep 3, 2015
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.

4 participants