[core][taskEvents out of GCS][9/n] Reroute ray.timeline to task events head by karticam · Pull Request #65218 · ray-project/ray

karticam · 2026-08-04T22:48:43Z

Part of the effort to move task events out of GCS.
This PR builds on top of #65160

Similar to how we route ray list tasks to task events head based on the flag RAY_ENABLE_TASK_EVENTS_TO_DASHBOARD_HEAD, we also route ray.timeline to task events head based on the flag. If the flag is not set, GCS is queried.

gemini-code-assist

Code Review

This pull request introduces TaskEventsHeadClient to fetch task events from the dashboard head instead of the GCS when the migration flag RAY_ENABLE_TASK_EVENTS_TO_DASHBOARD_HEAD is enabled, caching the endpoint and reusing the HTTP session. Unit tests have been added to verify both migration-enabled and default behaviors. The review feedback suggests broadening exception handling to requests.RequestException to catch all request-related failures, improving authentication error messages using format_authentication_http_error, and ensuring thread-safety and accessor freshness during the lazy initialization of the client.

gemini-code-assist · 2026-08-04T22:50:05Z

+        except (requests.ConnectionError, requests.Timeout):
+            # The dashboard may have restarted at a new address; re-resolve next call.
+            self._endpoint = None
+            raise


Catching only requests.ConnectionError and requests.Timeout might miss other request-related exceptions (such as DNS resolution failures or other RequestException subclasses) that also indicate the dashboard head is unreachable or restarted. Broadening the exception handling to requests.RequestException ensures the cached endpoint is correctly invalidated in all failure scenarios.

Suggested change

except (requests.ConnectionError, requests.Timeout):

# The dashboard may have restarted at a new address; re-resolve next call.

self._endpoint = None

raise

except requests.RequestException:

# The dashboard may have restarted at a new address; re-resolve next call.

self._endpoint = None

raise

Seems to be a valid comment

gemini-code-assist · 2026-08-04T22:50:05Z

+        if response.status_code != 200:
+            raise RuntimeError(
+                f"Failed to read task events from the dashboard head at {endpoint}: "
+                f"HTTP {response.status_code} {response.reason}."
+            )


If token authentication is enabled, requests to the dashboard head might fail with HTTP 401 (Unauthorized) or 403 (Forbidden). Using format_authentication_http_error allows us to raise a much more user-friendly and informative error message explaining how to resolve the authentication issue.

Suggested change

if response.status_code != 200:

raise RuntimeError(

f"Failed to read task events from the dashboard head at {endpoint}: "

f"HTTP {response.status_code} {response.reason}."

)

if response.status_code != 200:

from ray._private.authentication.http_token_authentication import (

format_authentication_http_error,

)

auth_err = format_authentication_http_error(

response.status_code, response.text

)

if auth_err:

raise RuntimeError(auth_err)

raise RuntimeError(

f"Failed to read task events from the dashboard head at {endpoint}: "

f"HTTP {response.status_code} {response.reason}."

)

Seems like a good comment for better observability in error handling

gemini-code-assist · 2026-08-04T22:50:05Z

+    def _get_task_events_head_client(self) -> "TaskEventsHeadClient":
+        """Lazily build and cache the reusable dashboard-head task-events client."""
+        if self._task_events_head_client is None:
+            self._task_events_head_client = TaskEventsHeadClient(
+                self._connect_and_get_accessor()
+            )
+        return self._task_events_head_client


The lazy initialization of _task_events_head_client is not thread-safe and can lead to race conditions if multiple threads call _get_task_events_head_client concurrently. Additionally, if GlobalState is disconnected and reconnected, the cached client will hold a reference to the old, stale accessor. Protecting the initialization with self._init_lock and checking if the accessor has changed ensures thread-safety and robustness under reconnects.

Suggested change

def _get_task_events_head_client(self) -> "TaskEventsHeadClient":

"""Lazily build and cache the reusable dashboard-head task-events client."""

if self._task_events_head_client is None:

self._task_events_head_client = TaskEventsHeadClient(

self._connect_and_get_accessor()

)

return self._task_events_head_client

def _get_task_events_head_client(self) -> "TaskEventsHeadClient":

"""Lazily build and cache the reusable dashboard-head task-events client."""

accessor = self._connect_and_get_accessor()

with self._init_lock:

if (

self._task_events_head_client is None

or self._task_events_head_client._accessor is not accessor

):

self._task_events_head_client = TaskEventsHeadClient(accessor)

return self._task_events_head_client

Seemed to be a valid comment

done. though not done in the suggested way since it has a race. the accessor we get initially might change later on inside the lock since and we might build the client with a stale accessor.
therefore use the accessor that you grab inside the lock/

MengjinYan · 2026-08-07T01:27:08Z

        )
+
+    if _READ_TASK_EVENTS_FROM_DASHBOARD_HEAD:
+        logger.warning(


Nit: we should probably only output the warning if the dashboard is not started with the cluster. Or we should probably be clear that if there is no information shown or receive certain error, the user should check for whether the dashboard is enabled.

removed it from here now. now we throw only in error cases which is:

when it tries to get the dashboard endpoint, but GCS internal KV returns no endpoint.

if it gets the endpoint, but the request fails. .

so we dont proactively throw the error, but show it only when there is some error

MengjinYan · 2026-08-07T05:49:02Z


        return dict(result)

+    def _get_profiling_task_events(self):


The function name seems a bit misleading. We just get all the task events in this function without filtering for profile events so probably should update the function name here.

MengjinYan · 2026-08-07T06:32:39Z

+    def _get_task_events_head_client(self) -> "TaskEventsHeadClient":
+        """Lazily build and cache the reusable dashboard-head task-events client."""
+        if self._task_events_head_client is None:
+            self._task_events_head_client = TaskEventsHeadClient(
+                self._connect_and_get_accessor()
+            )
+        return self._task_events_head_client


Seemed to be a valid comment

MengjinYan · 2026-08-07T06:55:26Z

+            address = address.decode()
+            if not address.startswith(("http://", "https://")):
+                address = f"http://{address}"
+            self._endpoint = f"{address}/api/task_events/query"


Nit: The endpoint should probably be a constant string so that the same can be referenced using same constant.

MengjinYan · 2026-08-07T07:00:12Z

+            self._endpoint = f"{address}/api/task_events/query"
+        return self._endpoint
+
+    def get_task_events(self, timeout: int):


Nit: The return type annotation is missing

MengjinYan · 2026-08-07T07:10:11Z

+            # The dashboard may have restarted at a new address; re-resolve next call.
+            self._endpoint = None
+            raise
+        if response.status_code != 200:


Wondering if we should check 2xx instead of 200? Also, wondering if we should use something like response.raise_for_status()?

Checked for 2xx manually.
Dint' use response.raise_for_status() since first, it raises on only >= 400 (lets 3xx pass by) and second we want to throw a RuntimeError with our custom message. Doing a raise_for_status() , we would have to wrap it around try catch and then rethrow RuntimeError.

MengjinYan · 2026-08-07T07:12:23Z

+        if response.status_code != 200:
+            raise RuntimeError(
+                f"Failed to read task events from the dashboard head at {endpoint}: "
+                f"HTTP {response.status_code} {response.reason}."
+            )


Seems like a good comment for better observability in error handling

MengjinYan · 2026-08-07T07:23:54Z

I'm wondering if we should add an e2e test to validate the same information can be returned between the GCS path and the event_head path.

added the tests - both unit tests and integration tests. premerge is still running to see if they work

MengjinYan · 2026-08-07T07:27:16Z

+            )
+        reply = gcs_service_pb2.GetTaskEventsReply()
+        reply.ParseFromString(response.content)
+        if reply.status.code != 0:


nit: it's better to use a constant like StatusCode.OK instead of the actual code number.

made a constant in this file. HTTP status code constant OK resolves to 200, so just using our own variable for this

cursor

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit f521f5b. Configure here.}

Signed-off-by: Kartica Modi <karticamodi@gmail.com>

MengjinYan

All nits and things that can be addressed in followup PRs.

MengjinYan · 2026-08-08T23:09:27Z

+# GcsStatus.code value for a successful reply.
+_GCS_STATUS_CODE_OK = 0


Could resolved in a followup PR. Looks like the GCS status code is the same set of code used in status.h. To avoid duplicate definition, it would be better to use the cython binding here. For now, it's better to add a comment about where the code is defined.

MengjinYan · 2026-08-08T23:16:26Z

+# Timeout for the synchronous task-events query to the dashboard head.
+_DASHBOARD_HEAD_QUERY_TIMEOUT_S = 30
+# Dashboard-head route that answers task-events queries.
+_TASK_EVENTS_QUERY_PATH = "/api/task_events/query"


Could be resolved in a followup PR. Looks like the constant could be used in other places as well. It's better to move it to a more general place and use the constant in other places.

MengjinYan · 2026-08-08T23:27:47Z

        raise RuntimeError(
            "Ray has not been started yet. Timeline requires Ray to be initialized first."
        )
+


I might not be clear in my previous comment. But regarding the warning, let's just add one in all cases saying that the timeline feature will be served from the API server in the near future, and if you don't have the API server enabled, please do so to avoid interruption to the usage.

MengjinYan · 2026-08-08T23:54:47Z

+
+    def __init__(self, accessor: GlobalStateAccessor):
+        self._accessor = accessor
+        self._session = requests.Session()


It seems that the session is never closed. We should probably add a close function for the TaskEventHeadLClient and close it in the disconnect function in the GlobalState.

fixed. thanks for pointing out

Signed-off-by: Kartica Modi <karticamodi@gmail.com>

karticam requested a review from a team as a code owner August 4, 2026 22:48

karticam added core Issues that should be addressed in Ray Core go add ONLY when ready to merge, run all tests labels Aug 4, 2026

karticam requested review from MengjinYan and sampan-s-nayak August 4, 2026 22:49

gemini-code-assist Bot reviewed Aug 4, 2026

View reviewed changes

cursor Bot reviewed Aug 4, 2026

View reviewed changes

Comment thread python/ray/_private/state.py

karticam force-pushed the karticam/reroute-state-head branch from f9bd399 to ca17504 Compare August 5, 2026 10:23

karticam force-pushed the karticam/reroute-ray-timeline branch from 7000109 to 16c84bd Compare August 5, 2026 10:31

karticam force-pushed the karticam/reroute-state-head branch from ca17504 to f84c2ce Compare August 6, 2026 00:03

karticam force-pushed the karticam/reroute-ray-timeline branch from 16c84bd to c1012be Compare August 6, 2026 00:03

karticam mentioned this pull request Aug 6, 2026

[core][taskEvents out of GCS][10/n] Fix bugs found after migration #65247

Open

karticam force-pushed the karticam/reroute-state-head branch from f84c2ce to 0045511 Compare August 6, 2026 22:58

karticam force-pushed the karticam/reroute-ray-timeline branch from c1012be to 031279b Compare August 6, 2026 22:59

MengjinYan reviewed Aug 7, 2026

View reviewed changes

karticam force-pushed the karticam/reroute-state-head branch 2 times, most recently from 2a899f1 to 1847097 Compare August 7, 2026 20:32

karticam force-pushed the karticam/reroute-ray-timeline branch from 031279b to 30afc48 Compare August 7, 2026 21:04

karticam force-pushed the karticam/reroute-state-head branch from 1847097 to 1117ea1 Compare August 7, 2026 22:57

karticam force-pushed the karticam/reroute-ray-timeline branch from 9387c98 to 20138e3 Compare August 7, 2026 22:59

karticam force-pushed the karticam/reroute-state-head branch from 1117ea1 to 19b9fca Compare August 8, 2026 01:08

karticam force-pushed the karticam/reroute-ray-timeline branch from 20138e3 to e477ec8 Compare August 8, 2026 01:08

karticam force-pushed the karticam/reroute-state-head branch from 5ceb88a to 6d7f58d Compare August 8, 2026 07:08

karticam force-pushed the karticam/reroute-ray-timeline branch from 8a6583a to f170291 Compare August 8, 2026 07:09

cursor Bot reviewed Aug 8, 2026

View reviewed changes

Comment thread python/ray/tests/test_state_api_2.py Outdated

karticam force-pushed the karticam/reroute-ray-timeline branch from f170291 to f521f5b Compare August 8, 2026 08:06

cursor Bot reviewed Aug 8, 2026

View reviewed changes

Comment thread python/ray/tests/test_state_api_2.py

karticam added 3 commits August 8, 2026 14:46

Routing ray.timeline to task events head

9f089c4

Signed-off-by: Kartica Modi <karticamodi@gmail.com>

Adding warning for ray.timneline

e86a62e

Signed-off-by: Kartica Modi <karticamodi@gmail.com>

Using _config for using ray_configs in py

5343e2d

Signed-off-by: Kartica Modi <karticamodi@gmail.com>

karticam added 5 commits August 8, 2026 14:46

Resolving cmt (pt1)

7f2fd41

Signed-off-by: Kartica Modi <karticamodi@gmail.com>

Resolving cmts (p2)

3c383d3

Signed-off-by: Kartica Modi <karticamodi@gmail.com>

resolving cmt: fix concurrency issue

f1bb446

Signed-off-by: Kartica Modi <karticamodi@gmail.com>

Resolve cmt: import issue, test fix

a3325e6

Signed-off-by: Kartica Modi <karticamodi@gmail.com>

fix test after client formation change

6e8068b

Signed-off-by: Kartica Modi <karticamodi@gmail.com>

karticam force-pushed the karticam/reroute-ray-timeline branch from d30bc16 to 97f0d17 Compare August 8, 2026 21:49

karticam requested review from a team as code owners August 8, 2026 21:49

karticam changed the base branch from karticam/reroute-state-head to master August 8, 2026 21:49

Resolving cmt: adding gcs v/s head tests

f5c37be

Signed-off-by: Kartica Modi <karticamodi@gmail.com>

karticam force-pushed the karticam/reroute-ray-timeline branch from 97f0d17 to f5c37be Compare August 8, 2026 21:50

MengjinYan reviewed Aug 8, 2026

View reviewed changes

Resolving cmt: closing session

cade4ae

Signed-off-by: Kartica Modi <karticamodi@gmail.com>

		# GcsStatus.code value for a successful reply.
		_GCS_STATUS_CODE_OK = 0

Uh oh!

Conversation

karticam commented Aug 4, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MengjinYan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment