-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Reduce loaded range tree memory usage #9181
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thank you for the detailed PR comment. It was tremendously helpful. Mainly a few comments to head off some future issues.
Several of the bots hit the recent unrelated metaslab ASSERT, I've resubmitted those tests. The style builder also flagged a few minor cstyle warnings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the refactor! Makes it easier to read the (quite complicated) insertion/removal code.
This looks like it will be a godsend for old pools. Unfortunately i can't seem to get this built for testing - failing to build in-tree with
EDIT: the comment here seems to indicate that the struct definition was made for human readability, not so much informing the compiler as to what types are involved in a clear manner:
I'm guessing that this is fixed with a designated initializer detailing the struct members for the compiler up front. |
@sempervictus It looks like you've got an outdated definition of |
@pcd1193182: this also includes the kvmem alloc PR (8384) and the grsec patch, neither of which touch that file. Its also from today's master branch. |
Ok, so this gets me past the first build errors:
which brings me to:
The first issue is likely being caught by the randstruct plugin, which exists upstream as well, so designated initializers will need to be used (anywhere that has more than one struct member which are not VLAs as those go to the end of a randstructed struct, IIUC).
|
@pcd1193182: designated initializers PR created in your repo to deal with the randstruct bit, and ensure that rt_btree_ops is always a range_tree_ops_t. |
@pcd1193182: so the plot thickens a bit, think i found an edge case related to how i'm building:
|
@pcd1193182: the renaming of btree_* to zfs_btree_* seems to work, everything built, zloop is running happily. I've added that commit to the PR as well, which should address all of the issues i've found so far. Hoping to verify stability and check against some rather old and fragmented pools to see how it behaves. |
@pcd1193182: now it looks like i've found (stumbled over thanks to uderef) a real bug. This happens when importing a zpool which has dedup enabled and created in a prior version of ZFS, then upgraded to the current master's feature set, sha512 IIRC (9th gen HK chip, power to spare). |
@sempervictus do you happen to have a crash dump from that kernel panic? |
@pcd1193182: negative, boot-time crash fully locks up the system, and it cant write anything to any media since all volumes are ZFS (except /boot and /boot/efi) - hence the shoddy phone photo |
@sempervictus Ok so I've had time now to catch up with the various points you addressed. I'm happy to pull in the designated initializers part of the PR, that seems like a good change. I'm not as convinced about the rename from As for the kernel panic, I'll see if I can reproduce it. Does this happen consistently when you populate a pool as you described, or is that just the scenario that your pool happened to be in when you encountered this issue? Neither dedup nor pool features should have an effect on the behavior of the btree code, so if that's the consistent factor it would be useful information. |
@pcd1193182: if those functions aren't renamed, they will conflict with the in-kernel (in Linux) btree implementation at least when building ZFS into the kernel as a static binary. Edge-case seems like an odd way to minimize the build failure - isn't in-kernel build what Illumos does too? As you mention, refcount is a good example of where we already do this - the kernel has its own refcount API, so ZFS had to prefix its own to avoid the collision IIRC. |
@sempervictus I hadn't realized to what extent building as part of the kernel was a supported operation; I've prefixed the data types and functions with |
@pcd1193182: i think 65a91b1 might be causing some problems here. Latest naive merge attempt resulted in build failures with incorrect argument numbers for functions and redefinitions of things. |
Signed-off-by: Paul Dagnelie <[email protected]>
Signed-off-by: Paul Dagnelie <[email protected]>
Signed-off-by: Paul Dagnelie <[email protected]>
Signed-off-by: Paul Dagnelie <[email protected]>
Signed-off-by: Paul Dagnelie <[email protected]>
Signed-off-by: Paul Dagnelie <[email protected]>
Signed-off-by: Paul Dagnelie <[email protected]>
Signed-off-by: Paul Dagnelie <[email protected]>
Signed-off-by: Paul Dagnelie <[email protected]>
d435e6e
to
42dfc54
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for update!
LGTM with wrapper at this moment and can update it later to another one if will agreed with proposals.
maybe use define something like:
just for example |
Signed-off-by: Paul Dagnelie <[email protected]>
(1) https://bitbucket.org/dilos/dilos-illumos/commits/7495cf4e17d78eb677e2a3d60c38f61c4e66268b with my updates I can see several options: |
@ikozhukhov I'm going to go ahead and merge this PR as is. It includes the |
ok, i have no issues with |
Additional test cases for the btree implementation, see #9181. Reviewed-by: Paul Dagnelie <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: John Kennedy <[email protected]> Closes #9717
This patch implements a new tree structure for ZFS, and uses it to store range trees more efficiently. The new structure is approximately a B-tree, though there are some small differences from the usual characterizations. The tree has core nodes and leaf nodes; each contain data elements, which the elements in the core nodes acting as separators between its children. The difference between core and leaf nodes is that the core nodes have an array of children, while leaf nodes don't. Every node in the tree may be only partially full; in most cases, they are all at least 50% full (in terms of element count) except for the root node, which can be less full. Underfull nodes will steal from their neighbors or merge to remain full enough, while overfull nodes will split in two. The data elements are contained in tree-controlled buffers; they are copied into these on insertion, and overwritten on deletion. This means that the elements are not independently allocated, which reduces overhead, but also means they can't be shared between trees (and also that pointers to them are only valid until a side-effectful tree operation occurs). The overhead varies based on how dense the tree is, but is usually on the order of about 50% of the element size; the per-node overheads are very small, and so don't make a significant difference. The trees can accept arbitrary records; they accept a size and a comparator to allow them to be used for a variety of purposes. The new trees replace the AVL trees used in the range trees today. Currently, the range_seg_t structure contains three 8 byte integers of payload and two 24 byte avl_tree_node_ts to handle its storage in both an offset-sorted tree and a size-sorted tree (total size: 64 bytes). In the new model, the range seg structures are usually two 4 byte integers, but a separate one needs to exist for the size-sorted and offset-sorted tree. Between the raw size, the 50% overhead, and the double storage, the new btrees are expected to use 8*1.5*2 = 24 bytes per record, or 33.3% as much memory as the AVL trees (this is for the purposes of storing metaslab range trees; for other purposes, like scrubs, they use ~50% as much memory). We reduced the size of the payload in the range segments by teaching range trees about starting offsets and shifts; since metaslabs have a fixed starting offset, and they all operate in terms of disk sectors, we can store the ranges using 4-byte integers as long as the size of the metaslab divided by the sector size is less than 2^32. For 512-byte sectors, this is a 2^41 (or 2TB) metaslab, which with the default settings corresponds to a 256PB disk. 4k sector disks can handle metaslabs up to 2^46 bytes, or 2^63 byte disks. Since we do not anticipate disks of this size in the near future, there should be almost no cases where metaslabs need 64-byte integers to store their ranges. We do still have the capability to store 64-byte integer ranges to account for cases where we are storing per-vdev (or per-dnode) trees, which could reasonably go above the limits discussed. We also do not store fill information in the compact version of the node, since it is only used for sorted scrub. We also optimized the metaslab loading process in various other ways to offset some inefficiencies in the btree model. While individual operations (find, insert, remove_from) are faster for the btree than they are for the avl tree, remove usually requires a find operation, while in the AVL tree model the element itself suffices. Some clever changes actually caused an overall speedup in metaslab loading; we use approximately 40% less cpu to load metaslabs in our tests on Illumos. Another memory and performance optimization was achieved by changing what is stored in the size-sorted trees. When a disk is heavily fragmented, the df algorithm used by default in ZFS will almost always find a number of small regions in its initial cursor-based search; it will usually only fall back to the size-sorted tree to find larger regions. If we increase the size of the cursor-based search slightly, and don't store segments that are smaller than a tunable size floor in the size-sorted tree, we can further cut memory usage down to below 20% of what the AVL trees store. This also results in further reductions in CPU time spent loading metaslabs. The 16KiB size floor was chosen because it results in substantial memory usage reduction while not usually resulting in situations where we can't find an appropriate chunk with the cursor and are forced to use an oversized chunk from the size-sorted tree. In addition, even if we do have to use an oversized chunk from the size-sorted tree, the chunk would be too small to use for ZIL allocations, so it isn't as big of a loss as it might otherwise be. And often, more small allocations will follow the initial one, and the cursor search will now find the remainder of the chunk we didn't use all of and use it for subsequent allocations. Practical testing has shown little or no change in fragmentation as a result of this change. If the size-sorted tree becomes empty while the offset sorted one still has entries, it will load all the entries from the offset sorted tree and disregard the size floor until it is unloaded again. This operation occurs rarely with the default setting, only on incredibly thoroughly fragmented pools. There are some other small changes to zdb to teach it to handle btrees, but nothing major. Reviewed-by: George Wilson <[email protected]> Reviewed-by: Matt Ahrens <[email protected]> Reviewed by: Sebastien Roy [email protected] Reviewed-by: Igor Kozhukhov <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Paul Dagnelie <[email protected]> Closes openzfs#9181 Signed-off-by: Bryant G. Ly <[email protected]> Conflicts: cmd/zdb/zdb.c include/sys/spa.h module/zfs/Makefile.in module/zfs/dmu_send.c module/zfs/dsl_bookmark.c module/zfs/dsl_deadlist.c module/zfs/metaslab.c
Signed-off-by: Paul Dagnelie [email protected]
Reviewed by: Matt Ahrens [email protected]
Reviewed by: Sebastien Roy [email protected]
Reviewed by: George Wilson [email protected]
Motivation and Context
On large systems with high fragmentation, very large amounts of memory can be used to store metaslab space maps in memory in range trees. In addition, if the system is very fragmented, it is often necessary to store many of these in memory in order to be able to efficiently perform allocations. Failure to keep enough metaslabs loaded can cause serious performance problems, especially on sync write workloads.
Part of the reason that so much memory is used is that the structure used to store the range segs is a pair of AVL trees (one is sorted by offset, the other by size). This results in
range_seg_t
s of 72 bytes to store a mere 24 bytes of payload (8 of which is unused for metaslab range trees). While these trees are easy to use and provide good performance, we can significantly reduce the memory usage of the range trees by using a more efficient tree structure.Description
This patch implements a new tree structure for ZFS, and uses it to store range trees more efficiently.
The new structure is approximately a B-tree, though there are some small differences from the usual characterizations. The tree has core nodes and leaf nodes; each contain data elements, which the elements in the core nodes acting as separators between its children. The difference between core and leaf nodes is that the core nodes have an array of children, while leaf nodes don't. Every node in the tree may be only partially full; in most cases, they are all at least 50% full (in terms of element count) except for the root node, which can be less full. Underfull nodes will steal from their neighbors or merge to remain full enough, while overfull nodes will split in two. The data elements are contained in tree-controlled buffers; they are copied into these on insertion, and overwritten on deletion. This means that the elements are not independently allocated, which reduces overhead, but also means they can't be shared between trees (and also that pointers to them are only valid until a side-effectful tree operation occurs). The overhead varies based on how dense the tree is, but is usually on the order of about 50% of the element size; the per-node overheads are very small, and so don't make a significant difference. The trees can accept arbitrary records; they accept a size and a comparator to allow them to be used for a variety of purposes.
The new trees replace the AVL trees used in the range trees today. Currently, the range_seg_t structure contains three 8 byte integers of payload and two 24 byte avl_tree_node_ts to handle its storage in both an offset-sorted tree and a size-sorted tree (total size: 64 bytes). In the new model, the range seg structures are usually two 4 byte integers, but a separate one needs to exist for the size-sorted and offset-sorted tree. Between the raw size, the 50% overhead, and the double storage, the new btrees are expected to use 8*1.5*2 = 24 bytes per record, or 33.3% as much memory as the AVL trees (this is for the purposes of storing metaslab range trees; for other purposes, like scrubs, they use ~50% as much memory).
We reduced the size of the payload in the range segments by teaching range trees about starting offsets and shifts; since metaslabs have a fixed starting offset, and they all operate in terms of disk sectors, we can store the ranges using 4-byte integers as long as the size of the metaslab divided by the sector size is less than 2^32. For 512-byte sectors, this is a 2^41 (or 2TB) metaslab, which with the default settings corresponds to a 256PB disk. 4k sector disks can handle metaslabs up to 2^46 bytes, or 2^63 byte disks. Since we do not anticipate disks of this size in the near future, there should be almost no cases where metaslabs need 64-byte integers to store their ranges. We do still have the capability to store 64-byte integer ranges to account for cases where we are storing per-vdev (or per-dnode) trees, which could reasonably go above the limits discussed. We also do not store fill information in the compact version of the node, since it is only used for sorted scrub.
We also optimized the metaslab loading process in various other ways to offset some inefficiencies in the btree model. While individual operations (
find
,insert
,remove_from
) are faster for the btree than they are for the avl tree,remove
usually requires afind
operation, while in the AVL tree model the element itself suffices. Some clever changes actually caused an overall speedup in metaslab loading; we use approximately 40% less cpu to load metaslabs in our tests on Illumos.Another memory and performance optimization was achieved by changing what is stored in the size-sorted trees. When a disk is heavily fragmented, the
df
algorithm used by default in ZFS will almost always find a number of small regions in its initial cursor-based search; it will usually only fall back to the size-sorted tree to find larger regions. If we increase the size of the cursor-based search slightly, and don't store segments that are smaller than a tunable size floor in the size-sorted tree, we can further cut memory usage down to below 20% of what the AVL trees store. This also results in further reductions in CPU time spent loading metaslabs.The 16KiB size floor was chosen because it results in substantial memory usage reduction while not usually resulting in situations where we can't find an appropriate chunk with the cursor and are forced to use an oversized chunk from the size-sorted tree. In addition, even if we do have to use an oversized chunk from the size-sorted tree, the chunk would be too small to use for ZIL allocations, so it isn't as big of a loss as it might otherwise be. And often, more small allocations will follow the initial one, and the cursor search will now find the remainder of the chunk we didn't use all of and use it for subsequent allocations. Practical testing has shown little or no change in fragmentation as a result of this change.
If the size-sorted tree becomes empty while the offset sorted one still has entries, it will load all the entries from the offset sorted tree and disregard the size floor until it is unloaded again. This operation occurs rarely with the default setting, only on incredibly thoroughly fragmented pools.
There are some other small changes to zdb to teach it to handle btrees, but nothing major.
How Has This Been Tested?
Correctness testing
This change has undergone extensive testing on Illumos: first, a feature was added to zhack that would allocate a btree and manipulate it at random by inserting and removing random integers. An AVL tree was used alongside it to verify its integrity, and internal integrity checks were run frequently to identify any broken invariants. This acted as a fuzzer to catch basic problems. Next, the code was integrated into the kernel for use in range trees, and then run through zfs-test and zloop to identify issues with any workloads that might occur in normal operation that didn't occur regularly with random inputs and outputs (many inserts/deletions in a row, lots of merges and splits, etc). Finally, the code was used in performance tests on a performance VM with a large, heavily fragmented pool. This provided both further correctness testing as well as performance testing.
On Linux, this change has been run through zfs-test and zloop several times, with a focus on the areas that differ between Illumos and Linux (notably the sorted scrub code, which uses the
fill
range_seg
entry that doesn't exist on Illumos).Performance testing
Performance test results from Illumos:
~80% decrease in memory utilization to store loaded metaslabs
~40% decrease in cpu utilization to load metaslabs
specific range tree operations vary, but most operations are slightly faster in btrees or comparable, except for range_tree_vacate (which takes somewhat longer because it needs to free two trees).
Some basic performance tests have been run on Linux to verify that there was no regression due to these changes. The specifics of memory and CPU usage are expected to remain approximately the same between the two platforms.
Types of changes
Checklist:
Signed-off-by
.