DNM: backport chain_xattr thing for xfs to hammer #4973
Conversation
If we wrote an xattr that was a multiple of a chunk, we will try to read the next chunk and get ENODATA. If that happens bail out of the loop and assume we've read the whole thing. Backport: hammer, firefly Signed-off-by: Sage Weil <sage@redhat.com> (cherry picked from commit 8614dce)
XFS has a hard limit of 255 (or 254?) bytes for xattrs to be inlined in the inode due to a single byte for the length in the on-disk format. If we have an xattr that is a bit bigger than that it will get kicked out into a separate extent, but if we stripe it across a few shorter xattrs it can be inlined when the xfs inode is e.g. 2K. Adjust the chain_xattr logic to capture this case. Note that we are doing this unconditionally regardless of fs type, but that is probably okay given that most users use XFS and the cost isn't huge. Reported-by: Ning Yao <yaoning@ruijie.com.cn> Signed-off-by: Sage Weil <sage@redhat.com> (cherry picked from commit c6cdb40)
ENOATTR == ENODATA on Linux; other systems return ENOATTR. Signed-off-by: Sage Weil <sage@redhat.com> (cherry picked from commit b2cd80c)
...if it isn't already defined. Haomai reported seeing this somewhere. Signed-off-by: Sage Weil <sage@redhat.com> (cherry picked from commit 886f5a9)
|
Is there an issue associated with this ? |
|
bug was 11969, backport issue 11981 On Tue, 16 Jun 2015, Loic Dachary wrote:
|
|
Hi Sage, do we actually do performance benchmark for this improvement? |
|
I mean is it possible it won't behavior better as we expect ? |
|
Just tried this patch on Giant with radosgw, all xattrs can get inline after applying this patch. I will try to find an environment to test, and hopefully get some results to share. |
|
It has not been benchmarked. We have high confidence that XFS will inline
the smaller attrs in the inode, though, so it is hard to see how it could
be slower. How much faster it is depends on how cold the inode cache
gets, or rather what the inode/xattr cache miss rate is.
|
|
I was intending to test this but am bogged down in other things at the moment. Might not happen until after RHS. |
liewegas
changed the title
ghost
assigned
|
@athanatos as soon as you think this is ready for testing, we can add it to the backport integration branch |
|
We're choosing not to backport this to hammer. |
http://tracker.ceph.com/issues/11981
We should be sure that this is worth the backport because it is a one-way street: once you stripe xattrs over small values you can't downgrade to a previous hammer point release.