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

Add Kryo registration for two classes required for Spark 2.3.0. #1897

Merged
merged 1 commit into from Feb 7, 2018

Conversation

7 participants
@jpdna
Member

jpdna commented Feb 1, 2018

Adds Kryo registration for two classes which is are required for Spark 2.3.0.rc2
With these changes ADAM compiles and passes all tests for Spark-2.3.0.rc2

These changes fail to compile with Spark-2.2.1, so must be accompanied by Spark version bump.

If there is a better way to note these changes than a PR against master, let me know.

@AmplabJenkins

This comment has been minimized.

AmplabJenkins commented Feb 1, 2018

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

Build result: FAILURE

[...truncated 7 lines...] > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > /home/jenkins/git2/bin/git rev-parse origin/pr/1897/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains 66a6896 # timeout=10Checking out Revision 66a6896 (origin/pr/1897/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 66a6896f799ee9e6185dfdbb2cbe87bde76d5761First time build. Skipping changelog.Triggering ADAM-prb ? 2.6.2,2.10,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.11,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.10,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.11,2.2.1,centosADAM-prb ? 2.6.2,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.11,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.11,2.2.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@heuermh

This comment has been minimized.

Member

heuermh commented Feb 1, 2018

What if you registered those classes using class names, and then put them in a try catch block?

@heuermh

This comment has been minimized.

Member

heuermh commented Feb 1, 2018

Paschall Paschall
Kryo registered org.apache.spark.sql.execution.datasources.BasicWrite…
…TaskStats and org.apache.spark.sql.execution.datasources.ExecutedWriteSummary as required for Spark 2.3.0, within try clause for backward compatibility

Added try clause for kryo.register changes for backward Spark 2.2.1 compatibility

fixed typo

@jpdna jpdna changed the title from Update for Spark version 2.3.0, breaks 2.2.1 to Update for Spark version 2.3.0 Feb 1, 2018

@jpdna

This comment has been minimized.

Member

jpdna commented Feb 1, 2018

Now this works for Spark 2.3.0rc2 and also works for Spark 2.2.1
Thanks @heuermh - I put in try as you suggested

@coveralls

This comment has been minimized.

coveralls commented Feb 1, 2018

Coverage Status

Coverage decreased (-0.06%) to 82.646% when pulling b7552d0 on jpdna:spark-2.3.0rc2 into adff336 on bigdatagenomics:master.

2 similar comments
@coveralls

This comment has been minimized.

coveralls commented Feb 1, 2018

Coverage Status

Coverage decreased (-0.06%) to 82.646% when pulling b7552d0 on jpdna:spark-2.3.0rc2 into adff336 on bigdatagenomics:master.

@coveralls

This comment has been minimized.

coveralls commented Feb 1, 2018

Coverage Status

Coverage decreased (-0.06%) to 82.646% when pulling b7552d0 on jpdna:spark-2.3.0rc2 into adff336 on bigdatagenomics:master.

@coveralls

This comment has been minimized.

coveralls commented Feb 1, 2018

Coverage Status

Coverage decreased (-0.06%) to 82.646% when pulling b7552d0 on jpdna:spark-2.3.0rc2 into adff336 on bigdatagenomics:master.

@AmplabJenkins

This comment has been minimized.

AmplabJenkins commented Feb 1, 2018

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

@AmplabJenkins

This comment has been minimized.

AmplabJenkins commented Feb 1, 2018

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

Build result: FAILURE

[...truncated 7 lines...] > /home/jenkins/git2/bin/git init /home/jenkins/workspace/ADAM-prb # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git --version # timeout=10 > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/heads/:refs/remotes/origin/ # timeout=15 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10 > /home/jenkins/git2/bin/git config --add remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > /home/jenkins/git2/bin/git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > /home/jenkins/git2/bin/git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ # timeout=15 > /home/jenkins/git2/bin/git rev-parse origin/pr/1897/merge^{commit} # timeout=10 > /home/jenkins/git2/bin/git branch -a -v --no-abbrev --contains 0c34d26 # timeout=10Checking out Revision 0c34d26 (origin/pr/1897/merge) > /home/jenkins/git2/bin/git config core.sparsecheckout # timeout=10 > /home/jenkins/git2/bin/git checkout -f 0c34d2676c5b2fc1bf543a82978bd07a2ed5e498First time build. Skipping changelog.Triggering ADAM-prb ? 2.6.2,2.10,2.2.1,centosTriggering ADAM-prb ? 2.6.2,2.11,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.10,2.2.1,centosTriggering ADAM-prb ? 2.7.3,2.11,2.2.1,centosADAM-prb ? 2.6.2,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.6.2,2.11,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.10,2.2.1,centos completed with result FAILUREADAM-prb ? 2.7.3,2.11,2.2.1,centos completed with result FAILURENotifying endpoint 'HTTP:https://webhooks.gitter.im/e/ac8bb6e9f53357bc8aa8'
Test FAILed.

@jpdna

This comment has been minimized.

Member

jpdna commented Feb 1, 2018

I think there is a Jenkins test configuration error of some kind causing build to fail above, but I don't know how to fix it.
I've confirmed again that all tests pass when I run locally, with the Spark 2.2.1 dependency.

@fnothaft

Thanks @jpdna! LGTM. I'll take a lool at Jenkins soon.

@fnothaft

This comment has been minimized.

Member

fnothaft commented Feb 1, 2018

@shaneknapp can you take a look at these builds in @AmplabJenkins? We're getting errors checking out the branch.

@heuermh

This comment has been minimized.

Member

heuermh commented Feb 1, 2018

Note this can't be merged until 2.3.0 is released; voting is still taking place on RC2, which will fail. RC3 may be available next week.

} catch {
case cnfe: java.lang.ClassNotFoundException => {
log.info("Did not find Spark internal class. This is expected for Spark 1.")
log.info("Did not find Spark internal class. This is expected for earlier Spark versions.")

This comment has been minimized.

@akmorrow13

akmorrow13 Feb 2, 2018

Contributor

Can you say spark < 2.3.0 or is that incorrect?

This comment has been minimized.

@jpdna

jpdna Feb 2, 2018

Member

So the first two kryo.register in this try were already there from a Spark1 to Spark2 incompatibility and the two I add i this PR are for a <2.3 to a 2.3 incompatibility.
But I think just saying <2.3 as you suggest is probably fine, will do unless @heuermh comments otherwise.

This comment has been minimized.

@heuermh

heuermh Feb 2, 2018

Member

TBH the errors with Spark 1 won't happen any more, since we don't support it. There are a few other places in this file that could be updated as well, but that is a low priority.

@fnothaft fnothaft added this to the 0.24.0 milestone Feb 2, 2018

@shaneknapp

This comment has been minimized.

Contributor

shaneknapp commented Feb 2, 2018

@heuermh heuermh modified the milestones: 0.24.0, 0.25.0 Feb 2, 2018

@heuermh

This comment has been minimized.

Member

heuermh commented Feb 2, 2018

Moved to 0.25.0 Milestone, can't have 0.24.0 wait for the Spark 2.3.0 release.

@heuermh

This comment has been minimized.

Member

heuermh commented Feb 2, 2018

Sorry for the noise, I see that the pom.xml change is no longer in this pull request.

@heuermh heuermh modified the milestones: 0.25.0, 0.24.0 Feb 2, 2018

@heuermh

This comment has been minimized.

Member

heuermh commented Feb 7, 2018

Jenkins, retest this please.

@AmplabJenkins

This comment has been minimized.

AmplabJenkins commented Feb 7, 2018

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

@heuermh heuermh merged commit 3be86b0 into bigdatagenomics:master Feb 7, 2018

2 checks passed

Codacy/PR Quality Review Good work! A positive pull request.
Details
default Merged build finished.
Details
@heuermh

This comment has been minimized.

Member

heuermh commented Feb 7, 2018

Thank you, @jpdna!

@heuermh heuermh changed the title from Update for Spark version 2.3.0 to Add Kryo registration for two classes required for Spark 2.3.0. Feb 7, 2018

@heuermh heuermh added this to Completed in Release 0.24.0 Feb 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment