Skip to content

[Data] Return 'inf' for max object store memory usage#59751

Merged
bveeramani merged 1 commit intomasterfrom
min-max-fix
Dec 30, 2025
Merged

[Data] Return 'inf' for max object store memory usage#59751
bveeramani merged 1 commit intomasterfrom
min-max-fix

Conversation

@bveeramani
Copy link
Copy Markdown
Member

#59412 updated the reservation resource allocator to cap resource allocations based on the resources returned by min_max_resource_usage. The problem is that running tasks can produce any amounts of data, so it doesn't make sense to cap by obj_store_mem_max_pending_output_per_task * concurrency.

This PR fixes that issue by setting the max object store memory usage to 'inf'.

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
@bveeramani bveeramani requested a review from a team as a code owner December 30, 2025 00:05
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 correctly addresses the issue of capping object store memory usage by setting the maximum to infinity for ActorPoolMapOperator and TaskPoolMapOperator. The logic is sound, as the output size of tasks can be unpredictable. The changes are clear and the update to the test for ActorPoolMapOperator is appropriate.

Comment on lines +191 to 194
# Set the max `object_store_memory` requirement to 'inf', because we
# don't know how much data each task can output.
object_store_memory=float("inf"),
)
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.

medium

This change to set object_store_memory to float("inf") is correct. However, the existing tests for min_max_resource_requirements in test_task_pool_map_operator.py do not cover the case where _max_concurrency is set. It would be beneficial to add a test case to ensure this code path is validated, similar to how it's tested for ActorPoolMapOperator.

@bveeramani bveeramani changed the title [Data] Return 'inf` for max object store memory usage [Data] Return 'inf' for max object store memory usage Dec 30, 2025
@bveeramani bveeramani enabled auto-merge (squash) December 30, 2025 00:16
@github-actions github-actions bot added the go add ONLY when ready to merge, run all tests label Dec 30, 2025
@bveeramani bveeramani merged commit 81e0544 into master Dec 30, 2025
6 of 7 checks passed
@bveeramani bveeramani deleted the min-max-fix branch December 30, 2025 00:53
AYou0207 pushed a commit to AYou0207/ray that referenced this pull request Jan 13, 2026
)

ray-project#59412 updated the reservation
resource allocator to cap resource allocations based on the resources
returned by `min_max_resource_usage`. The problem is that running tasks
can produce any amounts of data, so it doesn't make sense to cap by
`obj_store_mem_max_pending_output_per_task * concurrency`.

This PR fixes that issue by setting the max object store memory usage to
'inf'.

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: jasonwrwang <jasonwrwang@tencent.com>
lee1258561 pushed a commit to pinterest/ray that referenced this pull request Feb 3, 2026
)

ray-project#59412 updated the reservation
resource allocator to cap resource allocations based on the resources
returned by `min_max_resource_usage`. The problem is that running tasks
can produce any amounts of data, so it doesn't make sense to cap by
`obj_store_mem_max_pending_output_per_task * concurrency`.

This PR fixes that issue by setting the max object store memory usage to
'inf'.

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
peterxcli pushed a commit to peterxcli/ray that referenced this pull request Feb 25, 2026
)

ray-project#59412 updated the reservation
resource allocator to cap resource allocations based on the resources
returned by `min_max_resource_usage`. The problem is that running tasks
can produce any amounts of data, so it doesn't make sense to cap by
`obj_store_mem_max_pending_output_per_task * concurrency`.

This PR fixes that issue by setting the max object store memory usage to
'inf'.

Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
Signed-off-by: peterxcli <peterxcli@gmail.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