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

state: document memory_cache_bytes, reduce default #1233

Merged
merged 1 commit into from
Oct 29, 2020

Conversation

hdevalence
Copy link
Contributor

Closes #1026

Because of the way that sled uses this parameter, the actual in-memory
size may be much larger. Dialing this down should help avoid high
memory usage.

Closes #1026

Because of the way that sled uses this parameter, the actual in-memory
size may be much larger.  Dialing this down should help avoid high
memory usage.
@@ -87,7 +96,7 @@ impl Default for Config {

Self {
cache_dir,
memory_cache_bytes: 512 * 1024 * 1024,
memory_cache_bytes: 50_000_000,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't correspond to an actual in-memory size, so there's no reason to have an exact power of two size.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's also much easier for users to understand and tweak a default that ends with lots of zeroes.

Comment on lines +32 to +36
/// This corresponds to `sled`'s [`cache_capacity`][cc] parameter.
/// Note that the behavior of this parameter is [somewhat
/// unintuitive][gh], measuring the on-disk size of the cached data,
/// not the in-memory size, which may be much larger, especially for
/// smaller keys and values.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

🗜

@dconnolly dconnolly merged commit e2c5b71 into main Oct 29, 2020
@dconnolly dconnolly deleted the dial-down-cache-capacity branch October 29, 2020 18:31
teor2345 added a commit that referenced this pull request Oct 30, 2020
PR #1233 changed the default `memory_cache_bytes`, but left the
acceptance tests with their old value.

To avoid similar issues in future, we simply divide the default by 2.
teor2345 added a commit that referenced this pull request Nov 9, 2020
* Use the default memory limit in the acceptance tests

PR #1233 changed the default `memory_cache_bytes`, but left the
acceptance tests with their old value.
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.

Consider tuning Sled's cache_capacity knob after we change database structure.
3 participants