Skip to content
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

osd: add dump filter for tracked ops #16561

Merged
merged 3 commits into from Jul 27, 2017

Conversation

Projects
None yet
4 participants
@Yan-waller
Copy link
Contributor

Yan-waller commented Jul 25, 2017

add filters to dump the tracked ops just what we need.

While diagnosing a problem about client's op, we tried to dump the history tracked ops, but there were too many unrelated ops (set osd_op_history_size=1000), this change help us to focus on a certain client.

additional, extend this filter to other tracked op dump commands, using this as following:

  1. ceph daemon osd.# dump_historic_ops , filter nothing, dump all things as before;
  2. ceph daemon osd.# dump_historic_ops ip1:port1/nonce1 ... , filter ip, port, nonce, port and nonce are optional;

Signed-off-by: Yan Jun yan.jun8@zte.com.cn

common: fix indent
Signed-off-by: Yan Jun <yan.jun8@zte.com.cn>

@Yan-waller Yan-waller force-pushed the Yan-waller:wip-walle-0725osddumpclinfo branch from e317ce6 to 573bd43 Jul 25, 2017

@gregsfortytwo
Copy link
Member

gregsfortytwo left a comment

I can definitely see how this is useful, but I'm a bit worried about the interface breaking going on.

  1. The MDS and monitor both have their own ways of associating TrackedOp structures with clients and IP addresses that this is incompatible with.

  2. The filters have very generic interface names, but only actually work with IP addresses specifying clients.

I think this would be great if instead of moving the source_inst into TrackedOp, each TrackedOp subclass could implement a filter(TrackedOp* this, const set<string>& filters) function that is invoked and can do whatever is useful for that particular implementation. The OpRequest implementation can just be what is written here, and the MDS and monitor can specify their own at their convenience!

@Yan-waller Yan-waller force-pushed the Yan-waller:wip-walle-0725osddumpclinfo branch from 573bd43 to 814de29 Jul 26, 2017

osd: filter the trackedop that we needed.
Signed-off-by: Yan Jun <yan.jun8@zte.com.cn>
@Yan-waller

This comment has been minimized.

Copy link
Contributor Author

Yan-waller commented Jul 26, 2017

@gregsfortytwo, thanks for your advice, that's really valuable. I have fixed it, could you help to review it again ?

common: drop useless semicolon
Signed-off-by: Yan Jun <yan.jun8@zte.com.cn>
@gregsfortytwo

This comment has been minimized.

Copy link
Member

gregsfortytwo commented Jul 26, 2017

This looks great. Only other thing would be a test for this functionality, but I don't re,e,bear if we have a good setup for testing the admin socket commands.

@xiexingguo xiexingguo added this to the luminous milestone Jul 26, 2017

@yuriw yuriw merged commit 2eaa086 into ceph:master Jul 27, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.