Skip to content

Bisect 2 datasets multitenancy test: https://github.com/ray-project/ray/pull/63961 - #64194

Closed
TimothySeah wants to merge 13 commits into
ray-project:masterfrom
TimothySeah:tseah/2-datasets-multitenancy-bisect
Closed

Bisect 2 datasets multitenancy test: https://github.com/ray-project/ray/pull/63961#64194
TimothySeah wants to merge 13 commits into
ray-project:masterfrom
TimothySeah:tseah/2-datasets-multitenancy-bisect

Conversation

@TimothySeah

Copy link
Copy Markdown
Contributor

Claude thinks that #63961 might have caused a regression. I am rebasing #63737 on the commit before that to see if that's the case.

Signed-off-by: Timothy Seah <tseah@anyscale.com>
Signed-off-by: Timothy Seah <tseah@anyscale.com>
Signed-off-by: Timothy Seah <tseah@anyscale.com>
Signed-off-by: Timothy Seah <tseah@anyscale.com>
Signed-off-by: Timothy Seah <tseah@anyscale.com>
Signed-off-by: Timothy Seah <tseah@anyscale.com>
Signed-off-by: Timothy Seah <tseah@anyscale.com>
Signed-off-by: Timothy Seah <tseah@anyscale.com>
Signed-off-by: Timothy Seah <tseah@anyscale.com>
Signed-off-by: Timothy Seah <tseah@anyscale.com>
Signed-off-by: Timothy Seah <tseah@anyscale.com>
Signed-off-by: Timothy Seah <tseah@anyscale.com>
Signed-off-by: Timothy Seah <tseah@anyscale.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a multitenancy variant of the heterogeneous memory batch inference nightly test, running two concurrent pipelines pinned to separate subclusters to verify performance isolation and task placement. Feedback on the changes highlights potential race conditions from using threading.Thread with the process-global DataContext singleton, suggesting separate processes instead. Additionally, the reviewer recommends restoring the global label_selector state after dataset creation to prevent pollution, and refining the placement verification logic to avoid false positives on unlabeled system tasks.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +153 to +156
if want != node_sc:
bad_on_labeled.append(
(t.task_id, t.name, t.func_or_class_name, want, node_sc)
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The current placement verification logic flags any task running on a labeled node as a mismatch if its label_selector does not match the node's subcluster label.

However, if a task has no ray-subcluster label at all (i.e., want is None), it is likely a Ray system task (such as runtime environment setup, actor replication, or dashboard metrics collection). Since Ray system tasks can be scheduled on worker nodes and do not carry tenant labels, this check will trigger false positive mismatches.

To avoid false positives, we should only assert a mismatch if the task explicitly requests a subcluster (want is not None) but is scheduled on a node with a different label.

Suggested change
if want != node_sc:
bad_on_labeled.append(
(t.task_id, t.name, t.func_or_class_name, want, node_sc)
)
if want is not None and want != node_sc:
bad_on_labeled.append(
(t.task_id, t.name, t.func_or_class_name, want, node_sc)
)

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.

1 participant