Skip to content

Conversation

@squito
Copy link
Contributor

@squito squito commented Feb 14, 2017

Adds a test case that ensures that Executors gracefully handle a task that fails to deserialize, by sending back a reasonable failure message. This does not change any behavior (the prior behavior was already correct), it just adds a test case to prevent regression.

@SparkQA
Copy link

SparkQA commented Feb 14, 2017

Test build #72880 has finished for PR 16930 at commit f1e7e6f.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@squito
Copy link
Contributor Author

squito commented Feb 14, 2017

Jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Feb 14, 2017

Test build #72891 has finished for PR 16930 at commit f1e7e6f.

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

Copy link
Contributor

@kayousterhout kayousterhout left a comment

Choose a reason for hiding this comment

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

One small comment -- otherwise looks good

val conf = new SparkConf
val serializer = new JavaSerializer(conf)
val env = createMockEnv(conf, serializer)
val serializedTask = serializer.newInstance().serialize(new NonDeserializableTask)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you take the error message as a param to NonDeserializableTask, so the magic string can be defined once and used below on line 137? From a quick read I thought the one on line 137 was coming from the Spark core code (not from the test code)

@kayousterhout
Copy link
Contributor

LGTM assuming tests pass

@SparkQA
Copy link

SparkQA commented Feb 24, 2017

Test build #73437 has finished for PR 16930 at commit ce6bf9a.

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

@kayousterhout
Copy link
Contributor

I merged this into master

@asfgit asfgit closed this in 5f74148 Feb 24, 2017
@squito
Copy link
Contributor Author

squito commented Feb 24, 2017

thanks @kayousterhout !

@squito squito deleted the executor_task_deserialization branch February 24, 2017 22:59
Yunni pushed a commit to Yunni/spark that referenced this pull request Feb 27, 2017
Adds a test case that ensures that Executors gracefully handle a task that fails to deserialize, by sending back a reasonable failure message.  This does not change any behavior (the prior behavior was already correct), it just adds a test case to prevent regression.

Author: Imran Rashid <[email protected]>

Closes apache#16930 from squito/executor_task_deserialization.
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.

3 participants