[core][dashboard] Update nodes on delta. #47367
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
rynewang
changed the title
jjyao
reviewed
Aug 28, 2024
| subscriber = GcsAioNodeInfoSubscriber(address=gcs_addr) | ||
| await subscriber.subscribe() | ||
|
|
||
| # Get all node info from GCS. For TOCTOU, it happens after the subscription. |
There was a problem hiding this comment.
if you do:
- get all
- subscribe
- for each update, ...
between 1 and 2 you can miss some nodes. so we do
- subscribe
- get all
- for each update, ...
There was a problem hiding this comment.
Let's expand the acronym as it's not a commonly used one
There was a problem hiding this comment.
I think actor_head is not doing that, it's doing get all and then subscribe so it has the TOCTOU problem?
Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
|
Fixed all comments, reverted state_aggregator.py TPE changes (will be in another PR). PTAL! @jjyao @alexeykudinkin |
jjyao
reviewed
Aug 28, 2024
| subscriber = GcsAioNodeInfoSubscriber(address=gcs_addr) | ||
| await subscriber.subscribe() | ||
|
|
||
| # Get all node info from GCS. For TOCTOU, it happens after the subscription. |
There was a problem hiding this comment.
I think actor_head is not doing that, it's doing get all and then subscribe so it has the TOCTOU problem?
Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
| for actor_id, actor_table_data in actors.items() | ||
| } | ||
|
|
||
| def convert(actors) -> Dict[str, dict]: |
There was a problem hiding this comment.
Why are we duplicating the changes already made in https://github.com/anyscale/runtime/pull/928?
There was a problem hiding this comment.
OK I can leave this to you.
| subscriber = GcsAioNodeInfoSubscriber(address=gcs_addr) | ||
| await subscriber.subscribe() | ||
|
|
||
| # Get all node info from GCS. For TOCTOU, it happens after the subscription. |
There was a problem hiding this comment.
Let's expand the acronym as it's not a commonly used one
rynewang
changed the title
|
Thanks for the quick turn around! Let's do another round of reviews @jjyao @alexeykudinkin |
jjyao
reviewed
Aug 28, 2024
| @@ -136,6 +136,20 @@ def __init__(self, dashboard_head): | |||
| self.accumulative_event_processing_s = 0 | |||
|
|
|||
| async def _update_actors(self): | |||
| """ | |||
| Yields actor info. First yields all actors from GCS, then subscribes to | |||
There was a problem hiding this comment.
this method doesn't yield anything?
There was a problem hiding this comment.
we should make the code consistent for actor_head and node_head: e.g. both use AsyncGenerator
There was a problem hiding this comment.
yes but let's make it another PR.
There was a problem hiding this comment.
we should at least fix the comment to match the current PR
jjyao
approved these changes
Aug 28, 2024
| @@ -136,6 +136,20 @@ def __init__(self, dashboard_head): | |||
| self.accumulative_event_processing_s = 0 | |||
|
|
|||
| async def _update_actors(self): | |||
| """ | |||
| Yields actor info. First yields all actors from GCS, then subscribes to | |||
There was a problem hiding this comment.
we should at least fix the comment to match the current PR
Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
Signed-off-by: Ruiyang Wang <rywang014@gmail.com>
alexeykudinkin
approved these changes
Aug 29, 2024
This was referenced Sep 17, 2024
ujjawal-khare
pushed a commit
to ujjawal-khare-27/ray
that referenced
this pull request
Oct 12, 2024
Like actor_head.py, we now update DataSource.nodes on delta. It first queries all node infos, then subscribes node deltas. Each delta updates: 1. DataSource.nodes[node_id] 2. DataSource.agents[node_id] 3. a warning generated after RAY_DASHBOARD_HEAD_NODE_REGISTRATION_TIMEOUT = 10s Note on (2) agents: it's read from internal kv, and is not readily available until the agent.py is spawned and writes its own port to internal kv. So we make an async task for each node to poll this port every 1s. It occurs that the get-all-then-subscribe code has a TOCTOU problem, so also updated actor_head.py to first subscribe then get all actors. Signed-off-by: Ruiyang Wang <rywang014@gmail.com> Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
ujjawal-khare
pushed a commit
to ujjawal-khare-27/ray
that referenced
this pull request
Oct 15, 2024
Like actor_head.py, we now update DataSource.nodes on delta. It first queries all node infos, then subscribes node deltas. Each delta updates: 1. DataSource.nodes[node_id] 2. DataSource.agents[node_id] 3. a warning generated after RAY_DASHBOARD_HEAD_NODE_REGISTRATION_TIMEOUT = 10s Note on (2) agents: it's read from internal kv, and is not readily available until the agent.py is spawned and writes its own port to internal kv. So we make an async task for each node to poll this port every 1s. It occurs that the get-all-then-subscribe code has a TOCTOU problem, so also updated actor_head.py to first subscribe then get all actors. Signed-off-by: Ruiyang Wang <rywang014@gmail.com> Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
ujjawal-khare
pushed a commit
to ujjawal-khare-27/ray
that referenced
this pull request
Oct 15, 2024
Like actor_head.py, we now update DataSource.nodes on delta. It first queries all node infos, then subscribes node deltas. Each delta updates: 1. DataSource.nodes[node_id] 2. DataSource.agents[node_id] 3. a warning generated after RAY_DASHBOARD_HEAD_NODE_REGISTRATION_TIMEOUT = 10s Note on (2) agents: it's read from internal kv, and is not readily available until the agent.py is spawned and writes its own port to internal kv. So we make an async task for each node to poll this port every 1s. It occurs that the get-all-then-subscribe code has a TOCTOU problem, so also updated actor_head.py to first subscribe then get all actors. Signed-off-by: Ruiyang Wang <rywang014@gmail.com> Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
ujjawal-khare
pushed a commit
to ujjawal-khare-27/ray
that referenced
this pull request
Oct 15, 2024
Like actor_head.py, we now update DataSource.nodes on delta. It first queries all node infos, then subscribes node deltas. Each delta updates: 1. DataSource.nodes[node_id] 2. DataSource.agents[node_id] 3. a warning generated after RAY_DASHBOARD_HEAD_NODE_REGISTRATION_TIMEOUT = 10s Note on (2) agents: it's read from internal kv, and is not readily available until the agent.py is spawned and writes its own port to internal kv. So we make an async task for each node to poll this port every 1s. It occurs that the get-all-then-subscribe code has a TOCTOU problem, so also updated actor_head.py to first subscribe then get all actors. Signed-off-by: Ruiyang Wang <rywang014@gmail.com> Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
ujjawal-khare
pushed a commit
to ujjawal-khare-27/ray
that referenced
this pull request
Oct 15, 2024
Like actor_head.py, we now update DataSource.nodes on delta. It first queries all node infos, then subscribes node deltas. Each delta updates: 1. DataSource.nodes[node_id] 2. DataSource.agents[node_id] 3. a warning generated after RAY_DASHBOARD_HEAD_NODE_REGISTRATION_TIMEOUT = 10s Note on (2) agents: it's read from internal kv, and is not readily available until the agent.py is spawned and writes its own port to internal kv. So we make an async task for each node to poll this port every 1s. It occurs that the get-all-then-subscribe code has a TOCTOU problem, so also updated actor_head.py to first subscribe then get all actors. Signed-off-by: Ruiyang Wang <rywang014@gmail.com> Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Like actor_head.py, we now update DataSource.nodes on delta. It first queries all node infos, then subscribes node deltas. Each delta updates:
Note on (2) agents: it's read from internal kv, and is not readily available until the agent.py is spawned and writes its own port to internal kv. So we make an async task for each node to poll this port every 1s.
It occurs that the get-all-then-subscribe code has a TOCTOU problem, so also updated actor_head.py to first subscribe then get all actors.