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

[docdb] Make global LogCache usage be a percentage of total memory #3745

Closed
bmatican opened this issue Feb 26, 2020 · 3 comments
Closed

[docdb] Make global LogCache usage be a percentage of total memory #3745

bmatican opened this issue Feb 26, 2020 · 3 comments
Assignees
Labels
area/docdb YugabyteDB core features
Milestone

Comments

@bmatican
Copy link
Contributor

bmatican commented Feb 26, 2020

Currently, the default max LogCache memory usage is 1GB, as opposed to a % based model, like the rocksdb block caches or memstores. We should add a percentage based mechanic to the LogCache as well, so it is more uniform where our memory goes.

@bmatican bmatican added the area/docdb YugabyteDB core features label Feb 26, 2020
@bmatican bmatican assigned jmeehan16 and unassigned bmatican and iSignal Apr 23, 2021
@bmatican
Copy link
Contributor Author

This could be a nice followup to the recent refactor of TSTabletManager components, including LogCache GC

@bmatican
Copy link
Contributor Author

bmatican commented Oct 14, 2021

In src/yb/consensus/log_cache.cc, we already have memory limits

  • per each tablet, default 128MB
  • global, across all tablets, default 1024MB

For a more uniform way of accounting for memory, we should add a % of RAM based flag as well. We can start with something small, like 5%, for now.

We have a similar mechanism for rocksdb memstores, with more details in src/yb/tserver/tablet_memory_manager.cc. Individual memstores are capped by memstore_size_mb, default 128MB. But then we have 2 global limits: global_memstore_size_mb_max, default 2048 and global_memstore_size_percentage, default 10. We then just take the minimum of the respective values.

Feel free to reach out to @jmeehan16 or @robertsami for more info & review.

SrivastavaAnubhav pushed a commit that referenced this issue Nov 3, 2021
Summary:
Added the global_log_cache_size_limit_percentage flag for log cache.

Split the existing TestGlobalMemoryLimit test to TestGlobalMemoryLimitAbsolute (which uses the absolute memory limit) and TestGlboalMemoryLimitPercentage (which uses the new percentage-based memory limit).

Removed a flag from tablet_memory_manager (should_count_memory) which was always true according the CHECK statement after it (verified with @jmeehan).

Test Plan: ybd --cxx_test consensus_log_cache-test --gtest-filter LogCacheTest.TestGlobalMemoryLimit*

Reviewers: rsami, jmeehan

Reviewed By: jmeehan

Subscribers: jason, jmeehan, ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D13518
@SrivastavaAnubhav
Copy link
Contributor

Fixed by #3745

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docdb YugabyteDB core features
Projects
None yet
Development

No branches or pull requests

4 participants