[GitHub] spark pull request #19802: [WIP][SPARK-22594][CORE] Handling spark-submit an...

2017-11-24 Thread Jiri-Kremser
Github user Jiri-Kremser commented on a diff in the pull request:

https://github.com/apache/spark/pull/19802#discussion_r152960380
  
--- Diff: 
common/network-common/src/main/java/org/apache/spark/network/server/TransportRequestHandler.java
 ---
@@ -206,6 +207,11 @@ public void onFailure(Throwable e) {
   private void processOneWayMessage(OneWayMessage req) {
 try {
   rpcHandler.receive(reverseClient, req.body().nioByteBuffer());
+} catch (InvalidClassException ice) {
--- End diff --

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?


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #19802: [WIP][SPARK-22594][CORE] Handling spark-submit an...

2017-11-24 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/19802#discussion_r152959040
  
--- Diff: 
common/network-common/src/main/java/org/apache/spark/network/server/TransportRequestHandler.java
 ---
@@ -206,6 +207,11 @@ public void onFailure(Throwable e) {
   private void processOneWayMessage(OneWayMessage req) {
 try {
   rpcHandler.receive(reverseClient, req.body().nioByteBuffer());
+} catch (InvalidClassException ice) {
--- End diff --

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.


---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #19802: [WIP][SPARK-22594][CORE] Handling spark-submit an...

2017-11-23 Thread Jiri-Kremser
GitHub user Jiri-Kremser opened a pull request:

https://github.com/apache/spark/pull/19802

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

## What changes were proposed in this pull request?
If the `InvalidClassException` exception is thrown in the 
`TransportRequestHandler.processOneWayMessage` 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)
- [x] Please review http://spark.apache.org/contributing.html before 
opening a pull request.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Jiri-Kremser/spark SPARK-22594

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/19802.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #19802


commit 6c48392035d6a7300a50a620e8980657304443c6
Author: Jirka Kremser 
Date:   2017-11-23T16:49:55Z

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




---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org