Skip to content

Conversation

@mgummelt
Copy link

Overrode the start() method, which was previously starting a thread causing a race condition. I believe this should fix the flaky test.

@rxin
Copy link
Contributor

rxin commented Feb 11, 2016

Can you explain a bit why it is flaky? If the test case is useful, we should just change "test" to "ignore" rather than removing it.

@SparkQA
Copy link

SparkQA commented Feb 11, 2016

Test build #51076 has finished for PR 11164 at commit 3af190b.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, let's make this ignore("mesos kills ...") { ... } instead

@andrewor14
Copy link
Contributor

This test was added recently in #10993 so this is just reverting the test instead of reverting the whole patch. I do think we should use ignore since that's what we do elsewhere in Spark.

@andrewor14
Copy link
Contributor

Also @mgummelt can you either file a JIRA or add the same JIRA to the title of this patch?

@mgummelt
Copy link
Author

Updated the title.

Kept the test, and overrode the start() method, which was previously starting a thread causing a race condition. I believe this should fix the problem.

@mgummelt mgummelt changed the title remove flaky test [SPARK-5095] remove flaky test Feb 12, 2016
@SparkQA
Copy link

SparkQA commented Feb 12, 2016

Test build #51196 has finished for PR 11164 at commit 4e6b4db.

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

@andrewor14
Copy link
Contributor

Merged into master. Let's see whether this fixes it.

@asfgit asfgit closed this in 62b1c07 Feb 12, 2016
@mgummelt mgummelt deleted the fix_mesos_tests branch February 19, 2016 00:11
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.

4 participants