Skip to content
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.

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

[ml-release][no-ci] fix torch tune serve test. #30095

Merged
merged 1 commit into from
Nov 9, 2022

Conversation

xwjiang2010
Copy link
Contributor

Signed-off-by: xwjiang2010 xwjiang2010@gmail.com

Why are these changes needed?

The test currently misses wait_for_nodes, resulting in timeout.
https://console.anyscale-staging.com/o/anyscale-internal/projects/prj_qC3ZfndQWYYjx2cz8KWGNUL4/clusters/ses_Q4mHYcB1xS93WatGP7i9jSsy

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • 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 :(

The test currently misses wait_for_nodes, resulting in timeout.

Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com>
Copy link
Contributor

@krfricke krfricke left a comment

Choose a reason for hiding this comment

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

I think the idea behind the golden notebook test is explicitly to test autoscaling. If this doesn't work, we should investigate why this is the case - is it node availability, a problem with the autoscaler, the product, or do we just have to increase the timeout?

cc @matthewdeng for context on golden notebook tests

@krfricke
Copy link
Contributor

krfricke commented Nov 8, 2022

Hm, though it does seem that the cluster compute specifies min_nodes:

worker_node_types:
    - name: worker_node
      instance_type: g3.8xlarge
      min_workers: 2
      max_workers: 2
      use_spot: true

@matthewdeng
Copy link
Contributor

I don't recall this explicitly intending to test autoscaling.

  1. This was originally added with timeout 1800 in [release] add golden notebook release test for torch/tune/serve #16619.
    1. The wait_for_nodes utility may not have existed at this point in time.
  2. Timeout was decreased to 600 in [CI] Reduce cost of golden notebook release test #29609.

Copy link
Contributor

@krfricke krfricke left a comment

Choose a reason for hiding this comment

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

Thanks @matthewdeng for the context.

@krfricke krfricke merged commit c7d696e into ray-project:master Nov 9, 2022
WeichenXu123 pushed a commit to WeichenXu123/ray that referenced this pull request Dec 19, 2022
The test currently misses wait_for_nodes, resulting in timeout.

Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com>
Signed-off-by: Weichen Xu <weichen.xu@databricks.com>
@xwjiang2010 xwjiang2010 deleted the fix_torch_tune_serve_test branch July 26, 2023 19:51
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.

None yet

3 participants