Skip to content

[core] Fix test_object_spilling.py on Windows#53851

Merged
edoakes merged 2 commits intoray-project:masterfrom
edoakes:eoakes/fix-spill-win
Jun 16, 2025
Merged

[core] Fix test_object_spilling.py on Windows#53851
edoakes merged 2 commits intoray-project:masterfrom
edoakes:eoakes/fix-spill-win

Conversation

@edoakes
Copy link
Copy Markdown
Collaborator

@edoakes edoakes commented Jun 16, 2025

I made the workload more stressful in #53803 by fetching all of the results concurrently. That seems to have caused Windows to time out: https://buildkite.com/ray-project/postmerge/builds/10876#01977755-755b-485e-bd13-f0ea3e33cc36/158-818

I won't pretend to fully understand why, but reverting to the old pattern in an attempt to fix it.

Also added an explicit wait for the dir to drain because there was an error during cleanup caused by it.

edoakes added 2 commits June 16, 2025 09:02
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
@edoakes edoakes added the go add ONLY when ready to merge, run all tests label Jun 16, 2025
@edoakes edoakes requested a review from a team June 16, 2025 16:15
@edoakes edoakes enabled auto-merge (squash) June 16, 2025 16:15
Comment on lines +755 to +756
for obj_ref in [f.remote() for _ in range(5)]:
ray.get(obj_ref)
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.

This is bizarre. I would expect

ray.get([f.remote() for _ in range(5)])

to have better performance since it's fetching a batch instead of blocking on each individual call.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

in this case I think it's more stressful because not all 5 objs fit in the object store

@edoakes edoakes merged commit 7766b52 into ray-project:master Jun 16, 2025
5 of 6 checks passed
elliot-barn pushed a commit that referenced this pull request Jun 18, 2025
I made the workload more stressful in
#53803 by fetching all of the
results concurrently. That seems to have caused Windows to time out:
https://buildkite.com/ray-project/postmerge/builds/10876#01977755-755b-485e-bd13-f0ea3e33cc36/158-818

I won't pretend to fully understand why, but reverting to the old
pattern in an attempt to fix it.

Also added an explicit wait for the dir to drain because there was an
error during cleanup caused by it.

---------

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
minerharry pushed a commit to minerharry/ray that referenced this pull request Jun 27, 2025
I made the workload more stressful in
ray-project#53803 by fetching all of the
results concurrently. That seems to have caused Windows to time out:
https://buildkite.com/ray-project/postmerge/builds/10876#01977755-755b-485e-bd13-f0ea3e33cc36/158-818

I won't pretend to fully understand why, but reverting to the old
pattern in an attempt to fix it.

Also added an explicit wait for the dir to drain because there was an
error during cleanup caused by it.

---------

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
elliot-barn pushed a commit that referenced this pull request Jul 2, 2025
I made the workload more stressful in
#53803 by fetching all of the
results concurrently. That seems to have caused Windows to time out:
https://buildkite.com/ray-project/postmerge/builds/10876#01977755-755b-485e-bd13-f0ea3e33cc36/158-818

I won't pretend to fully understand why, but reverting to the old
pattern in an attempt to fix it.

Also added an explicit wait for the dir to drain because there was an
error during cleanup caused by it.

---------

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants