[UX] Add in-progress spot jobs to `sky status` #1691

Michaelvll · 2023-02-10T19:57:39Z

Fixes #1665.

In this PR, we also show in-progress spot jobs in the sky status output.

We also changed the implementation of fetch_aws.py/fetch_azure.py to get rid of ray, because:

ray has larger overhead
ray will add sigterm handler when being imported (here), which will cause the following error when we use multiprocessing.

TODO:

we show sky status after sky launch, and the current change will add overheads to a normal sky launch. We may need to add an additional option to sky status to avoid querying the spot jobs.

Overhead comparison:

python -c 'import time; from sky.clouds.service_catalog.data_fetchers import fetch_aws; start=time.time(); fetch_aws.fetch_availability_zone_mappings(); end = time.time(); print(end-start)' (used in aws_catalog.py)
new: 2.44 seconds, master: 6.50 seconds
time python -m sky.clouds.service_catalog.data_fetchers.fetch_aws --no-az-mappings --check-all-regions-enabled-for-account
new: 43s, master: 59s
time python -m sky.clouds.service_catalog.data_fetchers.fetch_azure
new: 1m9s, master: 1m28s

Tested (run the relevant ones):

Any manual or new tests for this PR (please specify below)
- sky status when the controller is stopped. (6.4s)
- sky status -r when the controller is stopped. (37s and spot job query does not introduce more cost)
- sky status when the controller is INIT.
- sky status when the controller is UP.
- python -m sky.clouds.service_catalog.data_fetchers.fetch_aws --no-az-mappings --check-all-regions-enabled-for-account
- python -m sky.clouds.service_catalog.data_fetchers.fetch_azure

concretevitamin · 2023-02-13T18:48:17Z

Thanks @Michaelvll for this UX improvement! I will try to get to this today or tomorrow. Quick question: many users will have tens of jobs - mix of in progress and finished - in the spot queue. What would the output looks like (will it put normal clusters out of the current terminal page)?

Michaelvll · 2023-02-13T18:51:30Z

Thanks @Michaelvll for this UX improvement! I will try to get to this today or tomorrow. Quick question: many users will have tens of jobs - mix of in progress and finished - in the spot queue. What would the output looks like (will it put normal clusters out of the current terminal page)?

Good question! It currently shows all the in-progress spot jobs without the finished ones. I can make it only show the first 5 in-progress spot jobs, and show all the spot jobs when -a is specified. Wdyt?

concretevitamin · 2023-02-13T18:57:18Z

Another idea is to not show any jobs, but show the total number of in-progress jobs + a hint asking users to check "spot queue". Wdyt?

…

On Mon, Feb 13, 2023 at 10:51 AM Zhanghao Wu ***@***.***> wrote: Thanks @Michaelvll <https://github.com/Michaelvll> for this UX improvement! I will try to get to this today or tomorrow. Quick question: many users will have tens of jobs - mix of in progress and finished - in the spot queue. What would the output looks like (will it put normal clusters out of the current terminal page)? Good question! It currently shows all the in-progress spot jobs without the finished ones. I can make it only show the first 5 in-progress spot jobs, and show all the spot jobs when -a is specified. Wdyt? — Reply to this email directly, view it on GitHub <#1691 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEQWHXYWXLNJXFPJZKKUFLWXJ7DZANCNFSM6AAAAAAUYFTVGI> . You are receiving this because your review was requested.Message ID: ***@***.***>

Michaelvll · 2023-02-13T21:13:20Z

Another idea is to not show any jobs, but show the total number of in-progress jobs + a hint asking users to check "spot queue". Wdyt?

I think it might be better to show the actual spot jobs for the user's convenience. We can still show the spot queue hint in it. : )

Michaelvll · 2023-02-15T20:58:37Z

According to the feedback, I reordered hints for autostopped clusters to the end. Also, I removed the managed spot queue printing when the spot controller is STOPPED or DOWN. PTAL @concretevitamin

…nto spot-queue-in-status

concretevitamin

Thanks @Michaelvll, let's go with this design thanks to user feedback. Left some UX comments.

» sst
Clusters
NAME                          LAUNCHED    RESOURCES                            STATUS  AUTOSTOP  COMMAND
sky-spot-controller-8a3968f2  16 hrs ago  1x GCP(n2-standard-4, disk_size=50)  UP      10m       sky spot queue -r

Managed Spot Jobs

* 1 cluster has auto{stop,down} scheduled. Refresh statuses with: sky status --refresh

or with a refresh

» sst -r
Clusters
NAME                          LAUNCHED    RESOURCES                            STATUS   AUTOSTOP  COMMAND
sky-spot-controller-8a3968f2  16 hrs ago  1x GCP(n2-standard-4, disk_size=50)  STOPPED  10m       sky spot queue -r

Managed Spot Jobs

* 1 cluster has auto{stop,down} scheduled. Refresh statuses with: sky status --refresh

Showing the header alone "Managed Spot Jobs" seems surprising (leaving it hanging). Shall we print something like "No in-progress jobs found." immediately under the header?

sky/cli.py

concretevitamin

Tried it out, works well! Some comments, mostly UX nits.

sky/cli.py

sky/clouds/service_catalog/aws_catalog.py

sky/clouds/service_catalog/data_fetchers/fetch_aws.py

sky/clouds/service_catalog/data_fetchers/fetch_azure.py

sky/utils/cli_utils/status_utils.py

sky/cli.py

Michaelvll · 2023-02-25T09:07:30Z

~~Blocked by #1727~~

…nto spot-queue-in-status

concretevitamin

Thanks @Michaelvll for this. Huge long-awaited UX improvement. LGTM.

Running a spot launch with a stopped controller, there seems to be a blank line:

 » sst
Clusters
NAME                          LAUNCHED        RESOURCES                            STATUS  AUTOSTOP  COMMAND
sky-spot-controller-8a3968f2  a few secs ago  1x GCP(n2-standard-4, disk_size=50)  INIT    -         sky spot launch -dy --cloud...

Managed spot jobs
Controller's latest status is detected to be INIT; jobs will not be shown until it becomes UP.

sky/backends/backend_utils.py

sky/cli.py

concretevitamin · 2023-02-25T23:40:39Z

sky/cli.py

+                    f'* {job_info}To see all jobs: {colorama.Style.BRIGHT}'
+                    f'sky spot queue{colorama.Style.RESET_ALL}')


Yes, not feeling strongly.

sky/cli.py

Co-authored-by: Zongheng Yang <zongheng.y@gmail.com>

…-experiments into spot-queue-in-status

concretevitamin

LGTM

concretevitamin · 2023-02-26T05:01:16Z

sky/backends/backend_utils.py

@@ -1958,11 +1958,10 @@ def _refresh_cluster_record(

    Args:
        cluster_name: The name of the cluster.
-        force_refresh: refresh the cluster status as long as the cluster exists.
-            If False, the cluster status will only be refreshed if the cluster:
+        force_refresh: if True, refresh the cluster status even if it may be skipped. Otherwise (the default),


format (line too long)?

Hmm, good catch! Weirdly, our format script and the GitHub CI failed to catch it. Fixed. Thanks!

concretevitamin · 2023-02-26T05:06:38Z

sky/spot/spot_utils.py

+        # controller is INIT/UP. This optimization avoids unnecessary costly
+        # refresh when the controller is already stopped. This optimization is
+        # based on the assumption that the user will not start the controller
+        # manually from the cloud console.


Seems safe to me after reading all usage in core and cli. Even if users manually start the controller in the console, this indicates it was previously autostopped, which means job have finished, so those usage seem safe.

* Show spot queue in sky status * format * Handle missed error * Do not force refresh for spot controller * use multiprocesssing * fix deprecation error * fix * fix * Fix spot queue problem * Add --no-show-spot-queue * format * Only show first 5 in-progress jobs * fix * reorder * Do not show the spot queue if the controller is stopped * format * fix * Rearange * partially address comments * address comments * fix * longer wait for spot job * fix * Fix message * format * fix message * revert changes for catalog fetcher * revert * partially address * Use the new fetchers * address comments * force_refresh * Adopt threading instead * no need to wait * address comments * Update sky/backends/backend_utils.py Co-authored-by: Zongheng Yang <zongheng.y@gmail.com> * Update sky/cli.py Co-authored-by: Zongheng Yang <zongheng.y@gmail.com> * Update sky/cli.py Co-authored-by: Zongheng Yang <zongheng.y@gmail.com> * rename option * fix argument * format * add comment --------- Co-authored-by: Zongheng Yang <zongheng.y@gmail.com>

Michaelvll added 10 commits February 10, 2023 10:51

Show spot queue in sky status

0153c45

format

49ebc97

Handle missed error

404fa78

Do not force refresh for spot controller

7a26b5c

use multiprocesssing

60735ea

fix deprecation error

1dd254c

fix

ff73ac1

fix

8902dd3

Fix spot queue problem

a7b8e39

Add --no-show-spot-queue

7f1c785

Michaelvll marked this pull request as ready for review February 12, 2023 00:46

format

fa5fd00

Michaelvll requested a review from concretevitamin February 13, 2023 16:12

Michaelvll added 2 commits February 13, 2023 11:27

Only show first 5 in-progress jobs

d26ab16

fix

83e9bea

Michaelvll added 4 commits February 15, 2023 12:46

reorder

824a0dd

Do not show the spot queue if the controller is stopped

f9e13ef

format

b36660d

fix

242c3de

Michaelvll added 2 commits February 15, 2023 16:21

Rearange

d17a635

Merge branch 'master' of github.com:concretevitamin/sky-experiments i…

a74e7f3

…nto spot-queue-in-status

concretevitamin reviewed Feb 23, 2023

View reviewed changes

sky/cli.py Show resolved Hide resolved

sky/cli.py Outdated Show resolved Hide resolved

sky/cli.py Outdated Show resolved Hide resolved

sky/cli.py Outdated Show resolved Hide resolved

Michaelvll added 3 commits February 23, 2023 22:43

partially address comments

1ef697e

address comments

5ad2476

fix

c11410d

fix message

ea2b15b

concretevitamin reviewed Feb 25, 2023

View reviewed changes

Michaelvll added 6 commits February 24, 2023 23:28

revert changes for catalog fetcher

c1c498d

revert

ba1870a

partially address

d8ec9ae

Use the new fetchers

466e479

address comments

70e6255

force_refresh

37e14da

Michaelvll mentioned this pull request Feb 25, 2023

[Catalog] Get rid of ray for catalog fetching #1727

Merged

2 tasks

Michaelvll added 2 commits February 25, 2023 01:03

Adopt threading instead

2420372

no need to wait

8abcf9e

Merge branch 'master' of github.com:concretevitamin/sky-experiments i…

a40beec

…nto spot-queue-in-status

Michaelvll added duplicate This issue or pull request already exists and removed duplicate This issue or pull request already exists labels Feb 25, 2023

address comments

311e9ea

concretevitamin approved these changes Feb 25, 2023

View reviewed changes

Michaelvll and others added 8 commits February 25, 2023 16:25

Update sky/backends/backend_utils.py

e9b553b

Co-authored-by: Zongheng Yang <zongheng.y@gmail.com>

Update sky/cli.py

d5fc5be

Co-authored-by: Zongheng Yang <zongheng.y@gmail.com>

Update sky/cli.py

dc34db3

Co-authored-by: Zongheng Yang <zongheng.y@gmail.com>

rename option

918b00a

Merge branch 'spot-queue-in-status' of github.com:concretevitamin/sky…

f15ccd1

…-experiments into spot-queue-in-status

fix argument

cc8d1c8

format

5a6fe94

add comment

c97c47a

concretevitamin approved these changes Feb 26, 2023

View reviewed changes

format

f2af937

Michaelvll merged commit 1ac582a into master Feb 26, 2023

Michaelvll deleted the spot-queue-in-status branch February 26, 2023 05:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UX] Add in-progress spot jobs to `sky status` #1691

[UX] Add in-progress spot jobs to `sky status` #1691

Michaelvll commented Feb 10, 2023 •

edited

Loading

concretevitamin commented Feb 13, 2023

Michaelvll commented Feb 13, 2023

concretevitamin commented Feb 13, 2023 via email

Michaelvll commented Feb 13, 2023

Michaelvll commented Feb 15, 2023 •

edited

Loading

concretevitamin left a comment

concretevitamin left a comment

Michaelvll commented Feb 25, 2023 •

edited

Loading

concretevitamin left a comment

concretevitamin Feb 25, 2023

concretevitamin left a comment

concretevitamin Feb 26, 2023

Michaelvll Feb 26, 2023

concretevitamin Feb 26, 2023

		f'* {job_info}To see all jobs: {colorama.Style.BRIGHT}'
		f'sky spot queue{colorama.Style.RESET_ALL}')

[UX] Add in-progress spot jobs to sky status #1691

[UX] Add in-progress spot jobs to sky status #1691

Conversation

Michaelvll commented Feb 10, 2023 • edited Loading

concretevitamin commented Feb 13, 2023

Michaelvll commented Feb 13, 2023

concretevitamin commented Feb 13, 2023 via email

Michaelvll commented Feb 13, 2023

Michaelvll commented Feb 15, 2023 • edited Loading

concretevitamin left a comment

Choose a reason for hiding this comment

concretevitamin left a comment

Choose a reason for hiding this comment

Michaelvll commented Feb 25, 2023 • edited Loading

concretevitamin left a comment

Choose a reason for hiding this comment

concretevitamin Feb 25, 2023

Choose a reason for hiding this comment

concretevitamin left a comment

Choose a reason for hiding this comment

concretevitamin Feb 26, 2023

Choose a reason for hiding this comment

Michaelvll Feb 26, 2023

Choose a reason for hiding this comment

concretevitamin Feb 26, 2023

Choose a reason for hiding this comment

[UX] Add in-progress spot jobs to `sky status` #1691

[UX] Add in-progress spot jobs to `sky status` #1691

Michaelvll commented Feb 10, 2023 •

edited

Loading

Michaelvll commented Feb 15, 2023 •

edited

Loading

Michaelvll commented Feb 25, 2023 •

edited

Loading