Skip to content

Conversation

@SeungjinYang
Copy link
Collaborator

@SeungjinYang SeungjinYang commented Aug 26, 2025

The purpose of this PR is to reduce the number of global_user_state.get_cluster_yaml_dict calls during handling of sky status.

Currently, global_user_state.get_cluster_yaml_dict is called once per backend_utils.ssh_credential_from_yaml, which is called in _update_record_with_credentials_and_resources_str. This function is called once per cluster. When an API server has many clusters, the number of DB calls increase linearly.

By renaming and refactoring _update_record_with_credentials_and_resources_str into _update_records_with_credentials_and_resources_str, we can allow the function to make a single db call to get all ssh credentials for the clusters. Appropriate changes are made in backend_utils.py and global_user_state.py to facilitate this change.

Testing: Testing is done entirely with smoke tests. Since sky status is one of the most comprehensively tested calls in smoke tests, smoke test should be enough to ensure lack of regression.

Tested (run the relevant ones):

  • Code formatting: install pre-commit (auto-check on commit) or bash format.sh
  • All smoke tests: /smoke-test (CI) or pytest tests/test_smoke.py (local)
  • Backward compatibility: /quicktest-core (CI) or pytest tests/smoke_tests/test_backward_compat.py (local)

@SeungjinYang SeungjinYang force-pushed the sky-status-optimizations-3 branch from 2315c9f to 4e8dfdb Compare August 26, 2025 23:04
@SeungjinYang
Copy link
Collaborator Author

SeungjinYang commented Aug 27, 2025

/quicktest-core (PASSING)

@SeungjinYang
Copy link
Collaborator Author

SeungjinYang commented Aug 27, 2025

/smoke-test --aws https://buildkite.com/skypilot-1/smoke-tests/builds/2831 (PASSING)

@SeungjinYang SeungjinYang marked this pull request as ready for review August 27, 2025 01:00
@SeungjinYang
Copy link
Collaborator Author

SeungjinYang commented Aug 27, 2025

/smoke-test --aws --postgres https://buildkite.com/skypilot-1/smoke-tests/builds/2833

@SeungjinYang SeungjinYang force-pushed the sky-status-optimizations-3 branch from bc84702 to 9739433 Compare August 27, 2025 07:50
@SeungjinYang SeungjinYang force-pushed the sky-status-optimizations-3 branch from 9739433 to 4b4ac2a Compare August 27, 2025 07:51
Copy link
Collaborator

@lloyd-brown lloyd-brown left a comment

Choose a reason for hiding this comment

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

Looks good!

@SeungjinYang SeungjinYang merged commit faa8c9a into master Aug 28, 2025
16 checks passed
@SeungjinYang SeungjinYang deleted the sky-status-optimizations-3 branch August 28, 2025 20:46
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