Details
-
Improvement
-
Resolution: Fixed
-
Minor
-
Lustre 2.4.0
-
5826
Description
The stats maintained by the EXP_MD_COUNTER_INCREMENT in the md_ops member wrappers and stored in struct obd_device's md_stats are not used. The classes that implement the the md_ops interface (lmv and mdc) do not allocate the them and the classes that allocate them (mgs and mdt) do not implement md_ops. (This is not a terrible issue but it does add to the internal and external cognitive crud of lustre proc.)
It should be easy to add these stats to lmv and mds but the value of doing so seems low. md_ops contains several groups of low-level function whose counts would be highly correlated and there are already counters in llite for the high level metadata operations.
If they are to be enabled anywhere then EXP_MD_COUNTER_INCREMENT() should check exp->exp_obd->md_stats rather than exp->exp_obd->obd_stats; lprocfs_alloc_md_stats() should check that obd->obd_type->typ_md_ops is not NULL; and exp_md_stats should be used somewhere or removed.
Attachments
Activity
Keith, this data is well used by admins. The big sites support a healthy cottage industry of lustre monitoring tools.
Have we thought about moving part of this stat data to a perf tracepoints sort of implementation with the idea to keep admin friendly items in /proc and developer data elsewhere?
I don't think these stats should be added to MDCs or any other class. Before I say why here's what they will look like:
~# cat /proc/fs/lustre/mdc/lustre-MDT0000-mdc-ffff88020176ec00/md_stats snapshot_time 1359065590.107282 secs.usecs getstatus 1 samples [reqs] change_cbdata 2246 samples [reqs] find_cbdata 2413 samples [reqs] close 157 samples [reqs] create 1667 samples [reqs] enqueue 12 samples [reqs] getattr 42 samples [reqs] getattr_name 5 samples [reqs] intent_lock 13239 samples [reqs] link 1 samples [reqs] rename 17 samples [reqs] setattr 113 samples [reqs] readpage 17 samples [reqs] unlink 537 samples [reqs] getxattr 155 samples [reqs] init_ea_size 4 samples [reqs] get_lustre_md 2857 samples [reqs] free_lustre_md 2752 samples [reqs] set_open_replay_data 157 samples [reqs] clear_open_replay_data 157 samples [reqs] set_lock_data 9897 samples [reqs] lock_match 2306 samples [reqs] intent_getattr_async 33 samples [reqs] revalidate_lock 3375 samples [reqs]
Now imagine someone reading this and saying "Whenever my jobs run the clients show a lot of change_cbdata and revalidate_lock reqs. Is that bad?"
What would you say to them? I would say that I'm sorry I submitted a patch to enable these stats.
The point is that these stats are counts of calls, and not of high-level metadata operations that help admins and users model their filesystem.
> John, with the advent of multiple MDTs, the stats per MDC will no longer be in lockstep with those at the llite/LMV level. It would be nice to have the per-MDC metadata stats available on the client
That seems fine to me. I had planned to do this in three parts:
1) Remove from mgs (see above)
2) Remove from mdt.
3) Remove common functions and members.
I'll go ahead with 1 and 2. Rather than just turning these on for lmv and mdc, I think the calls to EXP_MD_COUNTER_INCREMENT() should first be pruned. I don't believe that knowing the call count of mdc_init_ea_size() assists anyone in mentally modeling the behavior of lustre.
See http://review.whamcloud.com/4813 for a patch to remove md_stats from mgs and add some error handling to mgs proc initialization.
John, with the advent of multiple MDTs, the stats per MDC will no longer be in lockstep with those at the llite/LMV level. It would be nice to have the per-MDC metadata stats available on the client (e.g. for http://review.whamcloud.com/4403) if you think they provide some value.
In any case, it definitely seems like there is some cleanup to do, and as always I'm appreciative of your efforts.
Added 2.5.0 FixVersion