Skip to content

[Data] Fix unclear metadata warning and incorrect operator name logging#61380

Merged
bveeramani merged 5 commits intoray-project:masterfrom
yuhuan130:fix/operator_logging
Mar 3, 2026
Merged

[Data] Fix unclear metadata warning and incorrect operator name logging#61380
bveeramani merged 5 commits intoray-project:masterfrom
yuhuan130:fix/operator_logging

Conversation

@yuhuan130
Copy link
Copy Markdown
Contributor

@yuhuan130 yuhuan130 commented Feb 27, 2026 •
edited
Loading

Why are these changes needed?

The metadata timeout warning always shows operator=DataOpTask instead of the actual operator name, making it impossible to identify which operator is stuck.

What's included?

Add operator_name parameter to DataOpTask

  • New optional parameter in DataOpTask.__init__ stores the actual operator name

Rewrite warning message in plain English

Add unit test

  • test_operator_name_parameter verifies operator_name parameter is stored correctly

Screenshots

Screenshot 2026-02-27 at 00 05 56 Screenshot 2026-02-27 at 01 57 34

Benefits

  • Users can identify which specific operator is stuck in their pipeline
  • Warning message uses plain English

Note

Closes #61334

@yuhuan130
fix: improve operator name logging in DataOpTask
2d0a96c 
Signed-off-by: “Alex <alexchien130@gmail.com>
@yuhuan130
fix: add test & warning rework
f0af00d 
Signed-off-by: “Alex <alexchien130@gmail.com>
@yuhuan130 yuhuan130 requested a review from a team as a code owner February 27, 2026 08:06
gemini-code-assist[bot]
gemini-code-assist bot reviewed Feb 27, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

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

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 effectively addresses the issue of unclear operator names in metadata timeout warnings. By adding an operator_name parameter to DataOpTask and updating the warning message, the logging becomes much more informative and user-friendly. The changes are well-implemented across physical_operator.py, hash_shuffle.py, and map_operator.py. The inclusion of a unit test is also good. I have one suggestion to further improve the test coverage to ensure the new warning message is verified.

Comment thread python/ray/data/tests/test_streaming_executor.py Outdated
@yuhuan130
fix: add test
582aa74 
Signed-off-by: “Alex <alexchien130@gmail.com>
@yuhuan130
fix: merge conflict
4c5a664 
Signed-off-by: “Alex <alexchien130@gmail.com>
@yuhuan130
Copy link
Copy Markdown
Contributor Author

yuhuan130 commented Feb 27, 2026

@bveeramani PTAL~ Thank you ☺️

machichima
machichima reviewed Feb 27, 2026
View reviewed changes
Comment thread python/ray/data/_internal/execution/interfaces/physical_operator.py
machichima
machichima reviewed Feb 27, 2026
View reviewed changes
Comment thread python/ray/data/tests/test_streaming_executor.py Outdated
@ray-gardener ray-gardener bot added data Ray Data-related issues community-contribution Contributed by the community labels Feb 27, 2026
bveeramani
bveeramani reviewed Feb 27, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@bveeramani bveeramani left a comment

Choose a reason for hiding this comment

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

Left comments.

ty @machichima for doing the first pass

Comment thread python/ray/data/_internal/execution/interfaces/physical_operator.py
Comment thread python/ray/data/_internal/execution/interfaces/physical_operator.py
Comment thread python/ray/data/_internal/execution/interfaces/physical_operator.py Outdated
Comment thread python/ray/data/_internal/execution/interfaces/physical_operator.py Outdated
Comment thread python/ray/data/_internal/execution/interfaces/physical_operator.py Outdated
bveeramani
bveeramani reviewed Feb 27, 2026
View reviewed changes
Comment thread python/ray/data/tests/test_streaming_executor.py Outdated
Comment thread python/ray/data/tests/test_streaming_executor.py Outdated
slfan1989
slfan1989 reviewed Mar 1, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@slfan1989 slfan1989 left a comment

Choose a reason for hiding this comment

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

Thanks to @machichima and @bveeramani for the detailed comments! I don’t have additional suggestions, but I do have one point: The current log warning says 'waiting for operator,' but it’s actually waiting for metadata from the operator. It might be clearer to change it to 'waiting for metadata from operator …' to avoid any confusion."

Comment thread python/ray/data/_internal/execution/interfaces/physical_operator.py Outdated
@yuhuan130
fix: warning wording & rm warning test & op_name fallback
bbd79a1 
Signed-off-by: “Alex <alexchien130@gmail.com>
@yuhuan130
Copy link
Copy Markdown
Contributor Author

yuhuan130 commented Mar 3, 2026

bbd79a1 - Updated based on the review comments and Slack discussion

bveeramani
bveeramani approved these changes Mar 3, 2026
View reviewed changes
Comment thread python/ray/data/tests/test_streaming_executor.py
def test_operator_name_parameter(self, ray_start_regular_shared):
streaming_gen = create_stub_streaming_gen(block_nbytes=[1])
task = DataOpTask(0, streaming_gen, operator_name="MapBatches(fn)")
assert task._operator_name == "MapBatches(fn)"
Copy link
Copy Markdown
Member

@bveeramani bveeramani Mar 3, 2026

Choose a reason for hiding this comment

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

In general, I think we should avoid testing against internal attributes, but won't block since I know this has been in-flight for a while

Copy link
Copy Markdown
Contributor Author

@yuhuan130 yuhuan130 Mar 3, 2026

Choose a reason for hiding this comment

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

Thanks for noting. Will keep this in mind for future contributions.

@bveeramani bveeramani enabled auto-merge (squash) March 3, 2026 19:46
@github-actions github-actions bot added the go add ONLY when ready to merge, run all tests label Mar 3, 2026
@bveeramani bveeramani merged commit 80cc4bd into ray-project:master Mar 3, 2026
7 of 8 checks passed
ayushk7102 pushed a commit to ayushk7102/ray that referenced this pull request Mar 6, 2026
@yuhuan130 @ayushk7102
[Data] Fix unclear metadata warning and incorrect operator name loggi…
67b943e 
…ng (ray-project#61380)

## Why are these changes needed?
The metadata timeout warning always shows `operator=DataOpTask` instead
of the actual operator name, making it impossible to identify which
operator is stuck.

## What's included?

**Add operator_name parameter to DataOpTask**
- New optional parameter in `DataOpTask.__init__` stores the actual
operator name

**Rewrite warning message in plain English**

**Add unit test**
- `test_operator_name_parameter` verifies operator_name parameter is
stored correctly

## Screenshots

<img width="710" height="437" alt="Screenshot 2026-02-27 at 00 05 56"
src="https://github.com/user-attachments/assets/5f168fe3-bf4a-4268-b471-1126ddcc4cee"
/>
<img width="710" height="334" alt="Screenshot 2026-02-27 at 01 57 34"
src="https://github.com/user-attachments/assets/6e9648b0-0427-4667-b6e4-2beed9f9edab"
/>

## Benefits

- Users can identify which specific operator is stuck in their pipeline
- Warning message uses plain English

## Note

Closes ray-project#61334

---------

Signed-off-by: “Alex <alexchien130@gmail.com>
Signed-off-by: Ayush Kumar <ayushk7102@gmail.com>
ryanaoleary pushed a commit to ryanaoleary/ray that referenced this pull request Mar 13, 2026
@yuhuan130 @ryanaoleary
[Data] Fix unclear metadata warning and incorrect operator name loggi…
e244691 
…ng (ray-project#61380)

## Why are these changes needed?
The metadata timeout warning always shows `operator=DataOpTask` instead
of the actual operator name, making it impossible to identify which
operator is stuck.

## What's included?

**Add operator_name parameter to DataOpTask**
- New optional parameter in `DataOpTask.__init__` stores the actual
operator name

**Rewrite warning message in plain English**

**Add unit test**
- `test_operator_name_parameter` verifies operator_name parameter is
stored correctly

## Screenshots

<img width="710" height="437" alt="Screenshot 2026-02-27 at 00 05 56"
src="https://github.com/user-attachments/assets/5f168fe3-bf4a-4268-b471-1126ddcc4cee"
/>
<img width="710" height="334" alt="Screenshot 2026-02-27 at 01 57 34"
src="https://github.com/user-attachments/assets/6e9648b0-0427-4667-b6e4-2beed9f9edab"
/>


## Benefits

- Users can identify which specific operator is stuck in their pipeline
- Warning message uses plain English

## Note

Closes ray-project#61334

---------

Signed-off-by: “Alex <alexchien130@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slfan1989 slfan1989 slfan1989 left review comments

@machichima machichima machichima left review comments

@bveeramani bveeramani bveeramani approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

community-contribution Contributed by the community data Ray Data-related issues go add ONLY when ready to merge, run all tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Data] Fix unclear metadata warning and incorrect operator name logging

4 participants

@yuhuan130 @bveeramani @slfan1989 @machichima