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

mds/server: skip unwanted dn in handle_client_readdir #12870

Merged
merged 1 commit into from Jan 13, 2017

Conversation

Projects
None yet
6 participants
@xiaoxichen
Contributor

xiaoxichen commented Jan 11, 2017

We can skip unwanted dn which < (offset_key, snap) via map.lower_bound, rather than
iterate across them.

Previously we iterate and skip dn which < (offset_key, dn->last), as dn->last >= snap
means (offset_key, dn->last) >= (offset_key, snap), and such iterate_and_skip logic
still keep, so this commit doesnt change code logic but an optimization.

Signed-off-by: Xiaoxi Chen xiaoxchen@ebay.com

@xiexingguo

This comment has been minimized.

Member

xiexingguo commented Jan 11, 2017

Nit: typo in the title.

s/msd/mds/

mds/server: skip unwanted dn in handle_client_readdir
We can skip unwanted dn which  < (offset_key, snap) via map.lower_bound, rather than
iterate across them.

Previously we iterate and skip dn which < (offset_key, dn->last), as dn->last >= snap
 means (offset_key, dn->last) >= (offset_key, snap), and such iterate_and_skip logic
still keep, so this commit doesnt change code logic but an optimization.

Signed-off-by: Xiaoxi Chen <xiaoxchen@ebay.com>

@xiaoxichen xiaoxichen changed the title from msd/server: skip unwanted dn in handle_client_readdir to mds/server: skip unwanted dn in handle_client_readdir Jan 11, 2017

@xiaoxichen

This comment has been minimized.

Contributor

xiaoxichen commented Jan 11, 2017

@xiexingguo , fixed, thanks for reminding.

@xiaoxichen

This comment has been minimized.

Contributor

xiaoxichen commented Jan 11, 2017

The initiative for this commit is, we hit MDS CPU bottleneck (100% on one core as it is single thread) in our cephFS production enviroment.

Troubleshooting showing one applications are calling readdir often, what is worth, the dir has 130K files. Profiling showing on average each readdir call take ~20ms to finish in this scale(#files) and waste a significant time(and CPU!) on skipping unwanted dentries.

Take this request as example: 10 ms was spent on skipping the dentries and 7 ms was spent on encoding the wanted 1024 dentries. This patch addressed the 10ms and attempt to minimize it.

2017-01-09 19:49:03.023878 7ff8d74ce700 10 mds.0.server snapid head

[Iterating and skipping all dentry < offset]
https://github.com/ceph/ceph/blob/v10.2.2/src/mds/Server.cc#L3379

2017-01-09 19:49:03.033836 7ff8d74ce700 10 mds.0.cache.ino(100000edea3) encode_inodestat issuing pAsLsXsFscr seq 1867
[Encoding inodes to reply message, 1024 inodes in total , +10.836ms]
2017-01-09 19:49:03.040745 7ff8d74ce700 10 mds.0.cache.ino(100000eb3e7) encode_inodestat issuing pAsLsXsFscr seq 1867
……

[Finished encoding, reply to client,total size ~300KB + 17.752ms]
2017-01-09 19:49:03.040752 7ff8d74ce700 10 mds.0.server reply to client_request(client.26741659:858764 readdir #100000cbe8d lvs3b02c-cb62.stratus.lvs.ebay.com.pem 2017-01-09 19:49:02.991586) v2 readdir num=1024 bytes=310510 end=0 complete=0

@xiaoxichen xiaoxichen requested review from jcsp and ukernel Jan 11, 2017

@gregsfortytwo

I think this is fine. I was worried at first about how skipping interacts with dirfrag splitting but I think it's okay since we're ordered all nicely now...?

@xiaoxichen

This comment has been minimized.

Contributor

xiaoxichen commented Jan 11, 2017

Yes, and as the hash order change is backported to latest jewel, this change can also safely be backported. Do we need to create a ticket to track?

@jcsp

jcsp approved these changes Jan 12, 2017

@jcsp

This comment has been minimized.

Contributor

jcsp commented Jan 12, 2017

Looks good. I included this on a test run last night but there were too many (unrelated) failures, so it'll go through again today/tonight.

@jcsp jcsp merged commit b5262c2 into ceph:master Jan 13, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
@xiaoxichen

This comment has been minimized.

Contributor

xiaoxichen commented Jan 13, 2017

@jcsp , could we backport this to jewel?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment