Skip to content

[SPARK-8136][YARN] Fix flakiness in YarnClusterSuite. #6680

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.

Sign up for GitHub

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

Jump to bottom
Closed
harishreedharan wants to merge 2 commits into from
Closed

[SPARK-8136][YARN] Fix flakiness in YarnClusterSuite. #6680

harishreedharan wants to merge 2 commits into from

Conversation

@harishreedharan
Copy link
Contributor

@harishreedharan harishreedharan commented Jun 6, 2015

Instead of actually downloading the logs, just verify that the logs link is actually
a URL and is in the expected format.

@harishreedharan
[SPARK-8136][YARN] Fix flakiness in YarnClusterSuite.
50d69a7 
Instead of actually downloading the logs, just verify that the logs link is actually
a URL and is in the expected format.
@sryza
Copy link
Contributor

sryza commented Jun 6, 2015

Does this comment still apply: // If we are running in yarn-cluster mode, verify that driver logs are downloadable.?

@SparkQA
Copy link

SparkQA commented Jun 6, 2015

Test build #34346 has finished for PR 6680 at commit 50d69a7.

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

srowen
srowen reviewed Jun 6, 2015
View reviewed changes
yarn/src/test/scala/org/apache/spark/deploy/yarn/YarnClusterSuite.scala
Copy link
Member

@srowen srowen Jun 6, 2015

Choose a reason for hiding this comment

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

LGTM; Source isn't used now is it?

Copy link
Contributor Author

@harishreedharan harishreedharan Jun 6, 2015

Choose a reason for hiding this comment

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

Yep, I will clean the import up. Also, I think I am going to remove the check for hostname because that can fail if there are multiple hostnames which may not be the one returned by Utils.getLocalHostName() (just rely on the URL format).

@harishreedharan
Copy link
Contributor Author

harishreedharan commented Jun 6, 2015

YarnClusterSuite has passed, running it again to ensure it is not flaky.

Jenkins, retest this please.

@harishreedharan
Copy link
Contributor Author

harishreedharan commented Jun 6, 2015

Jenkins, test this please.

@SparkQA
Copy link

SparkQA commented Jun 6, 2015

Test build #34377 has finished for PR 6680 at commit 3183aeb.

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

@harishreedharan
Copy link
Contributor Author

harishreedharan commented Jun 6, 2015

Jenkins, test this please.

@andrewor14
Copy link
Contributor

andrewor14 commented Jun 6, 2015

looks great, thanks for fixing this. I'll merge this once tests pass.

@harishreedharan
Copy link
Contributor Author

harishreedharan commented Jun 6, 2015

Looks like YarnClusterSuite has passed.

@SparkQA
Copy link

SparkQA commented Jun 6, 2015

Test build #34378 has finished for PR 6680 at commit 3183aeb.

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

@SparkQA
Copy link

SparkQA commented Jun 7, 2015

Test build #34381 has finished for PR 6680 at commit 3183aeb.

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

@harishreedharan
Copy link
Contributor Author

harishreedharan commented Jun 7, 2015

This failed in streaming tests. I think this is good for commit, since the YARN tests all passed

@andrewor14
Copy link
Contributor

andrewor14 commented Jun 7, 2015

I just merged this in master. However it doesn't merge cleanly in 1.4 and 1.3. Could you open PRs against those branches?

harishreedharan added a commit that referenced this pull request Jun 7, 2015
@harishreedharan
[SPARK-8136] [YARN] Fix flakiness in YarnClusterSuite.
ed2cc3e 
Instead of actually downloading the logs, just verify that the logs link is actually
a URL and is in the expected format.

Author: Hari Shreedharan <[email protected]>

Closes #6680 from harishreedharan/simplify-am-log-tests and squashes the following commits:

3183aeb [Hari Shreedharan] Remove check for hostname which can fail on machines with several hostnames. Removed some unused imports.
50d69a7 [Hari Shreedharan] [SPARK-8136][YARN] Fix flakiness in YarnClusterSuite.
@harishreedharan
Copy link
Contributor Author

harishreedharan commented Jun 7, 2015

I will open a PR against 1.4 (if this functionality was committed to that branch). Since this functionality and the test is not in 1.3 branch, it should not be needed in that branch.

@harishreedharan
Copy link
Contributor Author

harishreedharan commented Jun 7, 2015

So this feature was not in 1.3 or 1.4 branches. So we are good.

@andrewor14
Copy link
Contributor

andrewor14 commented Jun 7, 2015

Alright sounds good

@andrewor14
Copy link
Contributor

andrewor14 commented Jun 7, 2015

Could you close this PR then? It seems that github is not closing them after merge for some reason.

@harishreedharan
Copy link
Contributor Author

harishreedharan commented Jun 7, 2015

Closing since this was merged into master.

nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
@harishreedharan @nemccarthy
[SPARK-8136] [YARN] Fix flakiness in YarnClusterSuite.
0da86a7 
Instead of actually downloading the logs, just verify that the logs link is actually
a URL and is in the expected format.

Author: Hari Shreedharan <[email protected]>

Closes apache#6680 from harishreedharan/simplify-am-log-tests and squashes the following commits:

3183aeb [Hari Shreedharan] Remove check for hostname which can fail on machines with several hostnames. Removed some unused imports.
50d69a7 [Hari Shreedharan] [SPARK-8136][YARN] Fix flakiness in YarnClusterSuite.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@harishreedharan @sryza @SparkQA @andrewor14 @srowen