Skip to content

[New scheduler] Deflake test heartbeat #11586

Merged
rkooo567 merged 6 commits intoray-project:masterfrom
wuisawesome:deflek_heartbeat
Oct 30, 2020
Merged

[New scheduler] Deflake test heartbeat #11586
rkooo567 merged 6 commits intoray-project:masterfrom
wuisawesome:deflek_heartbeat

Conversation

@wuisawesome
Copy link
Copy Markdown
Contributor

@wuisawesome wuisawesome commented Oct 24, 2020 •
edited
Loading

Why are these changes needed?

The heartbeat test was relying on the order of entries in the resource_load_by_shape field, but these were coming from a map. Simple fix is to just compare without worrying about the order.

Related issue number

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@wuisawesome
Copy link
Copy Markdown
Contributor Author

wuisawesome commented Oct 24, 2020

cc @stephanie-wang @ericl

@rkooo567
Copy link
Copy Markdown
Contributor

rkooo567 commented Oct 24, 2020

Hmm this failure only happened at Windows build though? I am not sure how this will be related to only Windows failure? But let me approve it first anyway. Let's see the build result.

@wuisawesome
Copy link
Copy Markdown
Contributor Author

wuisawesome commented Oct 24, 2020

I believe this is just coincidental because the iteration order over the map is consistent on linux, and consistent but different on windows.

I won't say that confidently in case the test doesn't pass though :p

@rkooo567
Copy link
Copy Markdown
Contributor

rkooo567 commented Oct 24, 2020

Please resolve issues in the CI (but it at least resolves the Windows issue!)

@wuisawesome
Copy link
Copy Markdown
Contributor Author

wuisawesome commented Oct 24, 2020

remaining tests failures seem unrelated

@wuisawesome wuisawesome added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Oct 24, 2020
@rkooo567 rkooo567 added @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. and removed tests-ok The tagger certifies test failures are unrelated and assumes personal liability. labels Oct 26, 2020
@rkooo567
Copy link
Copy Markdown
Contributor

rkooo567 commented Oct 26, 2020

https://travis-ci.com/github/ray-project/ray/jobs/406407223

This failure looks like it is related to this PR. (build failure from cluster_task_manager tests)

@rkooo567
Copy link
Copy Markdown
Contributor

rkooo567 commented Oct 28, 2020

CAn you merge the latest master? There are lots of serve failures which must've been resolved in the master branch.

@wuisawesome wuisawesome added tests-ok The tagger certifies test failures are unrelated and assumes personal liability. and removed @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. labels Oct 29, 2020
@rkooo567 rkooo567 merged commit e022d12 into ray-project:master Oct 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rkooo567 rkooo567 rkooo567 approved these changes

Assignees

@rkooo567 rkooo567

Labels

tests-ok The tagger certifies test failures are unrelated and assumes personal liability.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wuisawesome @rkooo567