[Feat][Core/Dashboard] Add SubprocessModules to the Dashboard routes, and convert HealthzHead #51282

MortalHappiness · 2025-03-11T23:17:00Z

Why are these changes needed?

Based on #51172 and #49864 to convert HealthzHead to subprocess dashboard module.

Integrate SubprocessModule into Dashboard. Now it can load SubprocessModules.
The support is only in non-minimal. For a minimal Ray, no SubprocessModule are loaded, but others minimal DashboardHeadModules can still work.
Converted HealthzHead to non-minimal. It does not do anything in minimal anyway.
Optimized usage_stats.py to use multi_get to save number of rpcs.

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 added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
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 :(

python/ray/_private/ray_constants.py

python/ray/dashboard/subprocesses/utils.py

python/ray/_private/usage/usage_lib.py

python/ray/dashboard/dashboard.py

python/ray/dashboard/head.py

python/ray/dashboard/modules/healthz/healthz_head.py

python/ray/dashboard/subprocesses/handle.py

python/ray/dashboard/subprocesses/module.py

python/ray/dashboard/subprocesses/utils.py

python/ray/train/BUILD

python/ray/dashboard/subprocesses/module.py

python/ray/dashboard/subprocesses/utils.py

Signed-off-by: Chi-Sheng Liu <[email protected]>

MortalHappiness · 2025-03-15T02:39:29Z

Added back _parent_process_death_detection_task in the subprocess because I found it hangs when calling run_string_as_driver in tests.

This reverts commit bbed411. Signed-off-by: Chi-Sheng Liu <[email protected]>

jjyao · 2025-03-17T05:09:10Z

python/ray/_private/node.py

-                raise OSError(
-                    f"AF_UNIX path length cannot exceed {maxlen} bytes: {result!r}"
-                )
+            validate_socket_filepath(result.split("://", 1)[-1])


Do we need to keep .encode("utf-8")?

Do we want to use non-ASCII chracters for filename? If we only use ASCII characters for filename, then the length of string and the length of the encoded bytes will be the same.

I don't know the history, I think it's safer to keep it.

Added back .encode("utf-8") inside validate_socket_filepath.

jjyao · 2025-03-17T05:33:33Z

python/ray/dashboard/subprocesses/module.py

+        Detect parent process death by checking if ppid is still the same.
+        """
+        while True:
+            ppid = os.getppid()


Return the parent’s process id. When the parent process has exited, on Unix the id returned is the one of the init process (1), on Windows it is still the same id, which may be already reused by another process.

This approach won't work on Windows.

Try multiprocessing.parent_process().is_alive() or pipe.

Use psutil instead. It can detect PID reuse.

https://psutil.readthedocs.io/en/latest/index.html#psutil.Process.is_running

Signed-off-by: Chi-Sheng Liu <[email protected]>

jjyao · 2025-03-17T15:56:35Z

python/ray/dashboard/subprocesses/module.py

        :param dashboard_head: The DashboardHead instance.
        """
        self._config = config
+        self._parent_process = psutil.Process(parent_process_pid)


What if parent is already dead and the pid is reused?

This is in the constructor of the child process. So do you mean that the parent process dies immediately even before the child process constructor has run?

I've changed the implementation to use multiprocessing.parent_process().is_alive() because I found that the parent_process() call returns a variable maintained by Python, so it should be able to identify the parent process calling multiprocessing.

https://github.com/python/cpython/blob/a936af924efc6e2fb59e27990dcd905b7819470a/Lib/multiprocessing/process.py#L51-L55

Signed-off-by: Chi-Sheng Liu <[email protected]>

jjyao · 2025-03-18T08:13:01Z

python/ray/dashboard/subprocesses/module.py

+
+    async def _detect_parent_process_death(self):
+        """
+        Detect parent process death by checking if ppid is still the same.


Needs to update the comment

Signed-off-by: Chi-Sheng Liu <[email protected]>

… routes, and convert HealthzHead (ray-project#51282)" This reverts commit ae7340d. Signed-off-by: Rui Qiao <[email protected]>

… routes, and convert HealthzHead (#51282)" (#51512) Signed-off-by: Rui Qiao <[email protected]>

…d routes, and convert HealthzHead (ray-project#51282)" (ray-project#51512) This reverts commit 8773682. Signed-off-by: Chi-Sheng Liu <[email protected]>

…d routes, and convert HealthzHead (#51282)" (#51512) (#51523) Signed-off-by: Chi-Sheng Liu <[email protected]>

… and convert HealthzHead (ray-project#51282) Signed-off-by: Chi-Sheng Liu <[email protected]> Signed-off-by: Dhakshin Suriakannu <[email protected]>

… routes, and convert HealthzHead (ray-project#51282)" (ray-project#51512) Signed-off-by: Rui Qiao <[email protected]> Signed-off-by: Dhakshin Suriakannu <[email protected]>

…d routes, and convert HealthzHead (ray-project#51282)" (ray-project#51512) (ray-project#51523) Signed-off-by: Chi-Sheng Liu <[email protected]> Signed-off-by: Dhakshin Suriakannu <[email protected]>

MortalHappiness added the go add ONLY when ready to merge, run all tests label Mar 11, 2025

MortalHappiness force-pushed the feature/healthzhead-dashboard-subprocess branch from 2dbfedf to 1da9451 Compare March 11, 2025 23:17

MortalHappiness mentioned this pull request Mar 11, 2025

[Feat][Core/Dashboard] Re-implement dashboard subprocess modules with aiohttp server/client #51172

Merged

8 tasks

MortalHappiness changed the title ~~Feature/healthzhead dashboard subprocess~~ [Feat][Core/Dashboard] Add SubprocessModules to the Dashboard routes, and convert HealthzHead Mar 11, 2025

MortalHappiness force-pushed the feature/healthzhead-dashboard-subprocess branch 4 times, most recently from 30b0061 to 5836482 Compare March 13, 2025 18:19

MortalHappiness marked this pull request as ready for review March 14, 2025 02:10

MortalHappiness requested review from edoakes, richardliaw and jjyao as code owners March 14, 2025 02:10

MortalHappiness assigned jjyao Mar 14, 2025

MortalHappiness requested a review from a team March 14, 2025 02:11

jjyao reviewed Mar 14, 2025

View reviewed changes

python/ray/_private/ray_constants.py Outdated Show resolved Hide resolved

MortalHappiness force-pushed the feature/healthzhead-dashboard-subprocess branch 3 times, most recently from b2f8ac0 to 5c080c5 Compare March 14, 2025 03:25

MortalHappiness commented Mar 14, 2025

View reviewed changes

python/ray/dashboard/subprocesses/utils.py Outdated Show resolved Hide resolved

MortalHappiness requested review from hongpeng-guo, justinvyu, matthewdeng, raulchen and woshiyyya as code owners March 14, 2025 03:53

jjyao reviewed Mar 14, 2025

View reviewed changes

python/ray/train/BUILD Outdated Show resolved Hide resolved

jjyao reviewed Mar 14, 2025

View reviewed changes

python/ray/dashboard/subprocesses/module.py Outdated Show resolved Hide resolved

python/ray/dashboard/subprocesses/utils.py Outdated Show resolved Hide resolved

MortalHappiness marked this pull request as draft March 14, 2025 05:42

MortalHappiness force-pushed the feature/healthzhead-dashboard-subprocess branch 2 times, most recently from 28bc5a8 to 52c166e Compare March 14, 2025 09:52

MortalHappiness added 2 commits March 15, 2025 10:35

Add back parent process death detection task

cb5c5c5

Signed-off-by: Chi-Sheng Liu <[email protected]>

Remove daemon attribute for subprocess

bbed411

Signed-off-by: Chi-Sheng Liu <[email protected]>

Revert "Remove daemon attribute for subprocess"

1186504

This reverts commit bbed411. Signed-off-by: Chi-Sheng Liu <[email protected]>

MortalHappiness force-pushed the feature/healthzhead-dashboard-subprocess branch from 73f9222 to 1186504 Compare March 15, 2025 02:47

MortalHappiness marked this pull request as ready for review March 15, 2025 03:56

jjyao reviewed Mar 17, 2025

View reviewed changes

MortalHappiness added 2 commits March 17, 2025 14:38

Address comments

1a06c77

Signed-off-by: Chi-Sheng Liu <[email protected]>

Move psutil parent process initializing to constructor

718b28b

Signed-off-by: Chi-Sheng Liu <[email protected]>

jjyao reviewed Mar 17, 2025

View reviewed changes

Change to multiprocessing.parent_process().is_alive()

aacd30b

Signed-off-by: Chi-Sheng Liu <[email protected]>

jjyao approved these changes Mar 18, 2025

View reviewed changes

jjyao and others added 3 commits March 18, 2025 01:22

Merge branch 'master' into feature/healthzhead-dashboard-subprocess

90eed6c

Update comment

a664db1

Signed-off-by: Chi-Sheng Liu <[email protected]>

Fix get_or_create_event_loop error

aff6693

Signed-off-by: Chi-Sheng Liu <[email protected]>

jjyao merged commit ae7340d into ray-project:master Mar 18, 2025
5 checks passed

This was referenced Mar 18, 2025

[Chore][Core/Dashboard] Improve log message for subprocess modules #51479

Closed

[dashboard] Add SubprocessModules to the Dashboard routes, and convert HealthzHead. #49864

Closed

jjyao pushed a commit that referenced this pull request Mar 19, 2025

Revert "[Feat][Core/Dashboard] Add SubprocessModules to the Dashboard…

8773682

… routes, and convert HealthzHead (#51282)" (#51512) Signed-off-by: Rui Qiao <[email protected]>

MortalHappiness mentioned this pull request Mar 19, 2025

Reapply "[Feat][Core/Dashboard] Add SubprocessModules to the Dashboard routes, and convert HealthzHead (#51282)" (#51512) #51523

Merged

8 tasks

jjyao pushed a commit that referenced this pull request Mar 19, 2025

Reapply "[Feat][Core/Dashboard] Add SubprocessModules to the Dashboar…

32cf632

…d routes, and convert HealthzHead (#51282)" (#51512) (#51523) Signed-off-by: Chi-Sheng Liu <[email protected]>

hainesmichaelc added the community-backlog label May 22, 2025

[Feat][Core/Dashboard] Add SubprocessModules to the Dashboard routes, and convert HealthzHead #51282

[Feat][Core/Dashboard] Add SubprocessModules to the Dashboard routes, and convert HealthzHead #51282

Uh oh!

Conversation

MortalHappiness commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are these changes needed?

Related issue number

Checks

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MortalHappiness commented Mar 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

MortalHappiness Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MortalHappiness Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

MortalHappiness commented Mar 11, 2025 •

edited

Loading

MortalHappiness commented Mar 15, 2025 •

edited

Loading

MortalHappiness Mar 17, 2025 •

edited

Loading

MortalHappiness Mar 17, 2025 •

edited

Loading