Skip to content

Conversation

@punya
Copy link
Contributor

@punya punya commented Apr 7, 2015

Version 3.1.1 is two years old and the newer version includes
approximate percentile statistics (among other things).

Version 3.1.1 is two years old and the newer version includes
approximate percentile statistics (among other things).
@ash211
Copy link
Contributor

ash211 commented Apr 7, 2015

@punya can you create a Jira ticket for this and put it in the PR subject like "[SPARK-12345] Bump version of apache commons-math3" ?

@punya punya changed the title Bump version of apache commons-math3 [SPARK-6731] Bump version of apache commons-math3 Apr 7, 2015
@SparkQA
Copy link

SparkQA commented Apr 7, 2015

Test build #29765 has finished for PR 5380 at commit 226622b.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch adds the following new dependencies:
    • commons-math3-3.4.1.jar
  • This patch removes the following dependencies:
    • commons-math3-3.1.1.jar

@srowen
Copy link
Member

srowen commented Apr 7, 2015

It's not entirely that simple, since this is meant to match the version used in other dependencies like Hadoop. The Commons Math3 dependencies aren't 100% compatible across minor releases. I generally favor this update, but you would first need to run tests with a number of different Hadoop profiles, for example, to verify it still works. Give that a go first?

@punya
Copy link
Contributor Author

punya commented Apr 7, 2015

Is there an automated way to do that?

@punya
Copy link
Contributor Author

punya commented Apr 7, 2015

@srowen based on http://central.maven.org/maven2/org/apache/hadoop/hadoop-core/0.20.2/hadoop-core-0.20.2.pom it looks like hadoop (at least this version) doesn't depend on commons-math3 at all. Perhaps we should annotate dependencies that are pinned based on Hadoop profiles to know what needs to be checked when we want to upgrade something?

@punya
Copy link
Contributor Author

punya commented Apr 7, 2015

I looked at https://repo1.maven.org/maven2/org/apache/hadoop/hadoop-project/2.6.0/hadoop-project-2.6.0.pom and did find that they depend on commons-math3 version 3.1.1. Do we aim for strict compatibility with the versions in the Hadoop parent POM, or do we only consider a subset of Hadoop projects that might be on the classpath with Spark?

@srowen
Copy link
Member

srowen commented Apr 8, 2015

My guess is that this is probably OK -- I have actually been using commons math 3.4.1 in a project on Hadoop 2.6 without issues, though I recall a subtle and nasty incompatibility problem from 3.2 to 3.3 (that is very unlikely to be relevant here or manifest here). Clearly the intent has been backwards compatibility: http://commons.apache.org/proper/commons-math/changes-report.html

Although it's always safe to try to harmonize dependencies, it's not 100% possible. If there's a compelling reason to update, and we see that tests pass in a few permutations of Hadoop builds, then I'd support it. For example can you try mvn [opts] -DskipTests clean package; mvn [opts] test for options like -Phadoop-2.2 and -Phadoop-2.4 -Dhadoop.version=2.6.0? if those pass that gives more confidence that there's no issue.

@punya
Copy link
Contributor Author

punya commented Apr 9, 2015

Thanks for the feedback, I'll let you know how the tests go.

@srowen
Copy link
Member

srowen commented Apr 12, 2015

I've tried Hadoop 1.2.1, 2.2.0 and 2.6.0 and tests passed. I am fairly sure this is an OK update. Any other experience on your end?

@punya
Copy link
Contributor Author

punya commented Apr 13, 2015

Unfortunately, I got random flaky tests when I ran it with 2.6.0 (and I didn't get a chance to determine whether the flakes were associated to the version update). My hunch is that they weren't (because they had nothing to do with commons-math3 and seemed related to timeouts). Based on your experience I'd favor merging the PR.

@srowen
Copy link
Member

srowen commented Apr 14, 2015

I think we'll want to upgrade this at some point since there are a number of bug fixes and small new features we want to use. I think I've convinced myself that this is empirically compatible, intended to be compatible, and the particular issue I got hit by is not relevant here.

@asfgit asfgit closed this in 628a72f Apr 14, 2015
@mengxr
Copy link
Contributor

mengxr commented Apr 14, 2015

@punya @srowen You can use japi-compliance-checker to check binary/source compatibility. I compared 3.1.1 and 3.4.1, and there are some break changes in the geometry package. I'm okay with this change, but I don't really know how many users are using the geometry package.

@srowen
Copy link
Member

srowen commented Apr 14, 2015

@mengxr Hm! that's good to know. I agree that I doubt users are affected by this -- Spark isn't -- and users should probably be packaging their own copy of commons-math3 if it matters.

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.

5 participants