[SPARK-22594][CORE] Handling spark-submit and master version mismatch #19802

jkremser · 2017-11-23T16:55:02Z

What changes were proposed in this pull request?

If the InvalidClassException exception is thrown in the TransportRequestHandler.processOneWayMessage (as a consequence of the different serialVersionUIDs) let's send the connected client a message that there is probably a version mismatch between his spark-submit and the remote spark master she is trying to contact.

How was this patch tested?

TODO:

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
( I've added test in 1b0fd3a )
Please review http://spark.apache.org/contributing.html before opening a pull request.

jerryshao · 2017-11-24T03:25:21Z

Can you please explain more, and how to reproduce this issue? Spark's RPC is not designed for version compatible.

jkremser · 2017-11-24T12:19:29Z

Can you please explain more, and how to reproduce this issue?

Sure,

Start master and worker (in version 2.2.0 for instance):

cat ./python/pyspark/version.py
__version__='2.2.0'

./bin/spark-class org.apache.spark.deploy.worker.Worker spark://`hostname -I | cut -d' ' -f1`:7077 -c 1 -m 1G &> /dev/null &
./bin/spark-class org.apache.spark.deploy.master.Master

Then in another terminal run spark-submit from Spark in different version (be it 2.3.0-SNAPSHOT)

cat ./python/pyspark/version.py
...
__version__ = "2.3.0.dev0"

./bin/spark-submit --class org.apache.spark.examples.SparkPi \
                   --master spark://`hostname -I | cut -d' ' -f1`:7077 \
                   --executor-memory 512M \
                   $PWD/examples/target/scala-2.11/jars/spark-examples_2.11-2.3.0-SNAPSHOT.jar \
                   10

result: in the Spark Master log there is the following exception:

java.io.InvalidClassException: org.apache.spark.rpc.RpcEndpointRef; local class incompatible: stream classdesc serialVersionUID = -1329125091869941550, local class serialVersionUID = 1835832137613908542

but on the spark-submit terminal there is nothing about the possibility of running a different version, actually it tries couple of times to connect and then fails on 17/11/24 13:13:58 ERROR StandaloneSchedulerBackend: Application has been killed. Reason: All masters are unresponsive! Giving up. , here is the complete log: https://pastebin.com/Wzs8vjBd

Spark's RPC is not designed for version compatible.

I hear you, on the other hand PR doesn't even try to make it compatible, all it does is to translate a cryptic error to more understandable one. I think, it may be quite common to run older spark-submit against the updated spark master or at least I've hit the issue couple of times. And I had to google the exception and then on stackoverflow or elsewhere figure out that it actually means that there was a version discrepancy.

If this PR is merged the "client" (the invoker of the spark-submit) will get this message in the log:

7/11/23 16:28:31 WARN TransportResponseHandler: Ignoring response for RPC 0 from /10.43.17.5:7077
 (There is probably a version mismatch between client and server: java.io.InvalidClassException: org.apache.spark.rpc.RpcEndpointRef; local class incompatible: stream classdesc serialVersionUID = 1835832137613908542, local class serialVersionUID = -1329125091869941550
	at java.io.ObjectStreamClass.initNonProxy(ObjectStreamClass.java:616)
	at java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1843)
...

srowen · 2017-11-24T12:24:45Z

...on/network-common/src/main/java/org/apache/spark/network/server/TransportRequestHandler.java

The problem is that this won't catch all or even most errors resulting from version incompatibility. Spark has never supported or contemplated mis-matching versions internally. I don't think we should try to handle this, because it's fundamentally piecemeal and hacky.

Perhaps I should have picked different name for the PR than Handling spark-submit and master version mismatch. It doesn't try to solve the issue in a complex way that two different version could be able to talk to each other, all it does is saying the user. Hey, you have probably different version than spark master. I agree, it's little bit hacky, on the other hand I see no other option than to catch the InvalidClassException, if the version is not part of the message. Perhaps some initial handshake in which the version is sent would be cleaner.

~~What about re-throwing the exception. This way it wouldn't change the semantics of the code, but the client would be informed. wdyt?~~

jkremser · 2017-11-24T14:11:15Z

To explain better the intentions, it doesn't try to solve of somehow provide a compatibility layer between old and new versions of Spark, all it does is slightly improving the UX, because people are hitting the issue all the time (including me)

couple of instances of this issue:

more here:
https://www.google.com/search?ei=BxMYWt3xLJDdwQKLwr9w&q=java.io.InvalidClassException%3A+org.apache.spark.rpc.RpcEndpointRef%3B+local+class+incompatible

btw. it's not only issue with spark-submit, spark-shell connecting to a remote master has the same flaw and with this change it would get the message that something is wrong.. Otherwise, currently it just hangs without any additional error.
I'd bet it also happens with any other kind of Spark driver, be it Jupyter notebook or JVM process trying to connect to a remote master.

felixcheung · 2017-11-24T20:22:18Z

I think there are some values in having a better experience with mismatch client/server versions. We discussed it might be even more common when the client was a R package (or Python perhaps down the line) @shivaram

I agree though focusing on one particular line is likely not sufficient. Perhaps we should design this out? Have this version check formally be part of the protocol handshake?

jkremser · 2017-12-21T16:35:01Z

I've also added tests and removed "WIP" from the label? Do you guys want the change or would you like to have something more elaborated, like an initial handshake?

One note about the "handshake approach". If there is new initial message that would be sent from client to server in which the client sends its version and server decides if it's backward compatible or not with this particular client version, this approach itself wouldn't solve the issue when old client (that doesn't know anything about it) tries to talk with new server. This PR solves this scenario.

Merry Christmas 🎄

jkremser · 2018-01-10T13:33:34Z

@srowen is there anything I can improve in the PR?

srowen

I think it's probably OK if you also re-throw that exception for now. You're just giving more info then.

srowen · 2018-01-10T13:47:40Z

common/network-common/src/test/java/org/apache/spark/network/TransportRequestHandlerSuite.java

These are JVM asserts, not JUnit asserts.

👍
ah, long time no Java 🤦‍♂️

srowen · 2018-01-10T13:48:00Z

common/network-common/src/main/java/org/apache/spark/network/protocol/RpcFailure.java

Nit: put static members first and separated by whitespace.

srowen · 2018-01-10T13:48:11Z

common/network-common/src/test/java/org/apache/spark/network/TransportRequestHandlerSuite.java

2 space, not 4 space indent

srowen · 2018-01-10T13:48:29Z

common/network-common/src/test/java/org/apache/spark/network/TransportRequestHandlerSuite.java

Space after casts

srowen · 2018-01-10T13:48:41Z

common/network-common/src/test/java/org/apache/spark/network/TransportRequestHandlerSuite.java

I don't think it's necessary to assert about the text because it could change

…om different versions

jkremser · 2018-01-10T15:14:10Z

I think it's probably OK if you also re-throw that exception for now. You're just giving more info then.

Hmm, In the original code the exception wasn't re-thrown. Do you really think it is a good idea? The InvalidClassException is a checked exception so the signature of processOneWayMessage() method would need to be changed as well.

this is the original 'catch-them-all' code :)

} catch (Exception e) {
      logger.error("Error while invoking RpcHandler#receive() for one-way message.", e);
}

I think, I addressed all your inline other comments, thanks for the review.

srowen · 2018-01-10T15:15:41Z

I see, yeah that's a fine point. Don't rethrow then.

jkremser · 2018-01-30T10:27:00Z

@srowen was Jenkins ok with the change? I can't see the results.

SparkQA · 2018-01-30T19:14:49Z

Test build #4086 has finished for PR 19802 at commit 4f79632.

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

srowen · 2018-01-31T15:16:26Z

@vanzin do you have thoughts on this one?

vanzin · 2018-01-31T20:33:27Z

I think this is the wrong place for the fix, if one is desired. The transport library doesn't know anything about serialization, that's all handled by the code using it. In this case, the 'RpcEnv' stuff in core. So I don't think it's correct to handle those errors in the transport library.

miacobv · 2018-02-01T15:57:09Z

I get this when I start the master and worker, without running spark-submit on both 2.2.0 and 2.2.1

AmplabJenkins · 2018-04-06T05:46:30Z

Can one of the admins verify this patch?

vanzin · 2018-05-11T19:56:01Z

@Jiri-Kremser if you're not planning to address the feedback you should probably close this PR.

jkremser force-pushed the SPARK-22594 branch from 6c48392 to 2d2e996 Compare November 23, 2017 17:15

srowen reviewed Nov 24, 2017

View reviewed changes

jkremser force-pushed the SPARK-22594 branch from 2d2e996 to 1b0fd3a Compare December 21, 2017 15:41

jkremser changed the title ~~[WIP][SPARK-22594][CORE] Handling spark-submit and master version mismatch~~ [SPARK-22594][CORE] Handling spark-submit and master version mismatch Dec 21, 2017

srowen reviewed Jan 10, 2018

View reviewed changes

[SPARK-22594][CORE] Handling spark-submit and master version mismatch

6991297

jkremser force-pushed the SPARK-22594 branch 2 times, most recently from 868e650 to ff0b7fb Compare January 10, 2018 15:05

[SPARK-22594][CORE] Adding tests for RPC communication when called fr…

4f79632

…om different versions

jkremser force-pushed the SPARK-22594 branch from ff0b7fb to 4f79632 Compare January 10, 2018 15:06

srowen approved these changes Jan 30, 2018

View reviewed changes

vanzin mentioned this pull request May 11, 2018

[BUILD] Close stale PRs #21303

Closed

jkremser closed this May 11, 2018

[SPARK-22594][CORE] Handling spark-submit and master version mismatch #19802

[SPARK-22594][CORE] Handling spark-submit and master version mismatch #19802

Uh oh!

Conversation

jkremser commented Nov 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

jerryshao commented Nov 24, 2017

Uh oh!

jkremser commented Nov 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srowen Nov 24, 2017

Choose a reason for hiding this comment

Uh oh!

jkremser Nov 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkremser commented Nov 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

felixcheung commented Nov 24, 2017

Uh oh!

jkremser commented Dec 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkremser commented Jan 10, 2018

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

srowen Jan 10, 2018

Choose a reason for hiding this comment

Uh oh!

jkremser Jan 10, 2018

Choose a reason for hiding this comment

Uh oh!

srowen Jan 10, 2018

Choose a reason for hiding this comment

Uh oh!

srowen Jan 10, 2018

Choose a reason for hiding this comment

Uh oh!

srowen Jan 10, 2018

Choose a reason for hiding this comment

Uh oh!

srowen Jan 10, 2018

Choose a reason for hiding this comment

Uh oh!

jkremser commented Jan 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srowen commented Jan 10, 2018

Uh oh!

jkremser commented Jan 30, 2018

Uh oh!

SparkQA commented Jan 30, 2018

Uh oh!

srowen commented Jan 31, 2018

Uh oh!

vanzin commented Jan 31, 2018

Uh oh!

miacobv commented Feb 1, 2018

Uh oh!

AmplabJenkins commented Apr 6, 2018

Uh oh!

vanzin commented May 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jkremser commented Nov 23, 2017 •

edited

Loading

jkremser commented Nov 24, 2017 •

edited

Loading

jkremser Nov 24, 2017 •

edited

Loading

jkremser commented Nov 24, 2017 •

edited

Loading

jkremser commented Dec 21, 2017 •

edited

Loading

jkremser commented Jan 10, 2018 •

edited

Loading