On 10/26/21 11:31 PM, Luís Henriques wrote:
On Tue, Oct 26, 2021 at 07:40:51AM -0400, Jeff Layton wrote:
On Tue, 2021-10-26 at 11:05 +0800, Xiubo Li wrote:
On 10/22/21 1:30 AM, Patrick Donnelly wrote:
On Thu, Oct 21, 2021 at 12:35 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
On Thu, 2021-10-21 at 12:18 -0400, Patrick Donnelly wrote:
On Thu, Oct 21, 2021 at 11:44 AM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
On Thu, 2021-10-21 at 09:52 -0400, Patrick Donnelly wrote:
On Wed, Oct 20, 2021 at 12:27 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
On Wed, 2021-10-20 at 15:37 +0100, Luís Henriques wrote:
This counter will keep track of the number of remote object copies done on
copy_file_range syscalls. This counter will be filesystem per-client, and
can be accessed from the client debugfs directory.
Cc: Patrick Donnelly <pdonnell@xxxxxxxxxx>
Signed-off-by: Luís Henriques <lhenriques@xxxxxxx>
---
This is an RFC to reply to Patrick's request in [0]. Note that I'm not
100% sure about the usefulness of this patch, or if this is the best way
to provide the functionality Patrick requested. Anyway, this is just to
get some feedback, hence the RFC.
Cheers,
--
Luís
[0] https://github.com/ceph/ceph/pull/42720
I think this would be better integrated into the stats infrastructure.
Maybe you could add a new set of "copy" stats to struct
ceph_client_metric that tracks the total copy operations done, their
size and latency (similar to read and write ops)?
I think it's a good idea to integrate this into "stats" but I think a
local debugfs file for some counters is still useful. The "stats"
module is immature at this time and I'd rather not build any qa tests
(yet) that rely on it.
Can we generalize this patch-set to a file named "op_counters" or
similar and additionally add other OSD ops performed by the kclient?
Tracking this sort of thing is the main purpose of the stats code. I'm
really not keen on adding a whole separate set of files for reporting
this.
Maybe I'm confused. Is there some "file" which is already used for
this type of debugging information? Or do you mean the code for
sending stats to the MDS to support cephfs-top?
What's the specific problem with relying on the data in debugfs
"metrics" file?
Maybe no problem? I wasn't aware of a "metrics" file.
Yes. For instance:
# cat /sys/kernel/debug/ceph/*/metrics
item total
------------------------------------------
opened files / total inodes 0 / 4
pinned i_caps / total inodes 5 / 4
opened inodes / total inodes 0 / 4
item total avg_lat(us) min_lat(us) max_lat(us) stdev(us)
-----------------------------------------------------------------------------------
read 0 0 0 0 0
write 5 914013 824797 1092343 103476
metadata 79 12856 1572 114572 13262
item total avg_sz(bytes) min_sz(bytes) max_sz(bytes) total_sz(bytes)
----------------------------------------------------------------------------------------
read 0 0 0 0 0
write 5 4194304 4194304 4194304 20971520
item total miss hit
-------------------------------------------------
d_lease 11 0 29
caps 5 68 10702
I'm proposing that Luis add new lines for "copy" to go along with the
"read" and "write" ones. The "total" counter should give you a count of
the number of operations.
Okay that makes more sense!
Side note: I am a bit horrified by how computer-unfriendly that
table-formatted data is.
Any suggestion to improve this ?
How about just make the "metric" file writable like a switch ? And as
default it will show the data as above and if tools want the
computer-friendly format, just write none-zero to it, then show raw data
just like:
# cat /sys/kernel/debug/ceph/*/metrics
opened_files:0
pinned_i_caps:5
opened_inodes:0
total_inodes:4
read_latency:0,0,0,0,0
write_latency:5,914013,824797,1092343,103476
metadata_latency:79,12856,1572,114572,13262
read_size:0,0,0,0,0
write_size:5,4194304,4194304,4194304,20971520
d_lease:11,0,29
caps:5,68,10702
I'd rather not multiplex the output of this file based on some input.
That would also be rather hard to do -- write() and read() are two
different syscalls, so you'd need to track a bool (or something) across
them somehow.
Currently, I doubt there are many scripts in the field that scrape this
info and debugfs is specifically excluded from ABI concerns. If we want
to make it more machine-readable (which sounds like a good thing), then
I suggest we just change the output to something like what you have
above and not worry about preserving the "legacy" output.
Ok, before submitting any new revision of this patch I should probably
clean this up. I can submit a patch to change the format to what Xiubo is
proposing. Obviously, that patch will also need to document what all
those fields actually mean.
Alternatively, the metrics file could be changed into a directory and have
4 different files, one per each section:
metrics/
|- files <-- not sure how to name the 1st section
|- latency
|- size
\- caps
Each of these files would then have the header but, since it's a single
header, parsing it in a script would be pretty easy. The advantage is
that this would be self-documented (with filenames and headers).
This sounds good to me.
Cheers,
--
Luís
