Bug #21402
closedmds: move remaining containers in CDentry/CDir/CInode to mempool
0%
Description
This commit:
https://github.com/ceph/ceph/commit/e035b64fcb0482c3318656e1680d683814f494fe
does only part of the work. There are still containers which need to be moved to the mempool so the cache memory usage is more accurate.
Files
Updated by Patrick Donnelly over 6 years ago
- Related to Backport #21384: luminous: mds: cache limits should be expressed in memory usage, not inode count added
Updated by Patrick Donnelly over 6 years ago
- Related to deleted (Backport #21384: luminous: mds: cache limits should be expressed in memory usage, not inode count)
Updated by Patrick Donnelly over 6 years ago
- Related to Bug #20594: mds: cache limits should be expressed in memory usage, not inode count added
Updated by Patrick Donnelly over 6 years ago
- Copied to Documentation #22599: doc: mds memory tracking of cache is imprecise by a constant factor added
Updated by Patrick Donnelly over 6 years ago
- Related to Documentation #22599: doc: mds memory tracking of cache is imprecise by a constant factor added
Updated by Patrick Donnelly over 6 years ago
- Status changed from New to In Progress
Updated by Nathan Cutler over 6 years ago
- Copied to deleted (Documentation #22599: doc: mds memory tracking of cache is imprecise by a constant factor)
Updated by Patrick Donnelly about 6 years ago
- File patched-memcache.png patched-memcache.png added
- File master-memcache.png master-memcache.png added
- Status changed from In Progress to Fix Under Review
https://github.com/ceph/ceph/pull/19954
Also ran two 64-client kernel build tests (one patched, one master) with a single active MDS with 24GB of RAM and `mds cache memory limit = 8GB`. Both graphs attached.
The patched version is 33% closer to the true RSS (30% smaller than RSS to 20%). Of course, it'll never be exactly the RSS because the MDS uses RAM for other purposes.
Other good news: apparently this also reduces RAM use by the MDS by 10% at an 8GB cache. I'm thinking about doing another test with a much larger cache to see the benefits there. Edit: the reason for this would be that many member containers no longer use indirect references (where possible) to other structures. i.e. instead of map<foo, bar*>, we have map<foo, bar>.
Updated by Patrick Donnelly about 6 years ago
- File master-memcache.png master-memcache.png added
- File patched-memcache.png patched-memcache.png added
64GB cache size limit experiment attached.
The master branch was tested with 64 kernel clients each building the kernel 4 times (so 256 parallel kernel builds). Afterwards, I realized it wasn't going to fill the cache so I killed it but the larger cache usage is still useful for some comparison.
The patched-memcache.png shows the memory usage for the patched version. This has 64 kernel clients and 512 parallel kernel builds.
Conclusions: The % of RSS remains stable for the cache size as its grows around ~20% for the patched case and ~35% for the master branch. That would explain the ~50% overuse of RAM ((1-.35)^-1 = 1.53) reported on the mailing list. With the patched version, we should see about ~25% instead ((1-.20)^-1 = 1.25).
Also worth noting, again, the memory usage of the MDS goes down. For comparative purposes, I selected a time when the cache in master was at its max and compared it to (nearly) the same cache size in the patched version. For a 35.8GB cache, the master branch uses a RSS of 54.3GB. For the patched branch at the same cache size, the RSS is 45.7GB. That gives us a reduction of about 15.8%!
There are still a small number of containers in the cache that need mvoed to the mempool. I've noted them in the PR with FIXMEs. We should revisit this when there is more time available.
Updated by Patrick Donnelly about 6 years ago
It occurred to me I wasn't comparing apples to apples when doing the memory reduction comparisons. I looked at the same data using inode count instead and the memory usage is about the same. No real change.
Updated by Patrick Donnelly about 6 years ago
- Status changed from Fix Under Review to Pending Backport
Updated by Patrick Donnelly about 6 years ago
- Copied to Bug #22962: mds: move remaining containers in CDentry/CDir/CInode to mempool (cont.) added
Updated by Patrick Donnelly about 6 years ago
- Related to Bug #22962: mds: move remaining containers in CDentry/CDir/CInode to mempool (cont.) added
Updated by Nathan Cutler about 6 years ago
- Copied to Backport #22972: luminous: mds: move remaining containers in CDentry/CDir/CInode to mempool added
Updated by Patrick Donnelly about 6 years ago
- Status changed from Pending Backport to Resolved