Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core] Make Object Store memory default ratio env-configurable #47726

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

alexeykudinkin
Copy link
Contributor

Currently, ratio of available memory allocated to Object Store is hard-coded at 30% which means that for most of the time Ray Data applications are actually not using 70% of memory.

This change

  • Removes static cap of 200Gb on the Object Store size
  • Makes DEFAULT_OBJECT_STORE_MEMORY_PROPORTION env-configurable

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 :(

@alexeykudinkin alexeykudinkin added the go add ONLY when ready to merge, run all tests label Sep 18, 2024
@@ -71,11 +71,10 @@ def env_set_by_user(key):

ID_SIZE = 28

# The default maximum number of bytes to allocate to the object store unless
# overridden by the user.
DEFAULT_OBJECT_STORE_MAX_MEMORY_BYTES = 200 * 10**9
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is a comment for why having this cap: # Cap memory to avoid memory waste and perf issues on large nodes.

Can we leave this change out and just make the ratio configurable? Or also make this cap configurable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And that explanation isn't very useful, right? What's the waste? What are perf issues? No links, no context.

This cap of 200Gb is arbitrary and hard to explain for nodes that carry 1Tb+ memory available

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