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

quincy: Revert "exporter: user only counter dump/schema commands for extacting counters" #54169

Merged
merged 1 commit into from Oct 25, 2023

Conversation

cbodley
Copy link
Contributor

@cbodley cbodley commented Oct 24, 2023

This reverts commit 6cc7c06.

the 'counter' commands are not present on quincy

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

…g counters"

This reverts commit 6cc7c06.

the 'counter' commands are not present on quincy

Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley cbodley added this to the quincy milestone Oct 24, 2023
@cbodley cbodley mentioned this pull request Oct 24, 2023
14 tasks
@idryomov
Copy link
Contributor

We have a lasting issue with exporter in that there are no automated tests. @avanthakkar who I intended to assess exporter use in quincy and put up a revert PR after manual testing is probably on holiday.

@alimaredia Having played with exporter recently, would you be able to "qualify" this PR?

@idryomov
Copy link
Contributor

jenkins test api

@alimaredia
Copy link
Contributor

@cbodley @idryomov I noticed missing from this PR is that PendingReleaseNotes is not reverted or changed. In the section >=17.2.4 there is note that mentions the counter dump and counter schema commands being added.

There is another that says all rbd-mirror daemon perf counters will be labeled and can be viewed with the counter dump/schema commands. Not sure if that needs to be additionally reverted just wanted to point it out

@idryomov
Copy link
Contributor

@cbodley @idryomov I noticed missing from this PR is that PendingReleaseNotes is not reverted or changed. In the section >=17.2.4 there is note that mentions the counter dump and counter schema commands being added.

@alimaredia That was a botched docs backport, where git cherry-pick was used blindly, without checking the end result. The original commit that added 5 lines morphed into something that added 71 lines: 535b8db -> a2c08e6.

I have already highlighted that in #51653. I don't think we need to worry about it here.

@alimaredia
Copy link
Contributor

@idryomov @cbodley I was having trouble building this branch today but here is how I would test it.

  1. Start up a vstart cluster. Ex: MON=1 OSD=1 MDS=0 RGW=1 MGR=0 ../src/vstart.sh -n -d
  2. Do a perf dump. Ex: ./bin/ceph -c ceph.conf daemon client.rgw.8000 perf dump
  3. Start the exporter. Ex: ./bin/ceph-exporter -c ceph.conf -f --stats-period=5 --sock-dir /path/to/ceph/build/out
  4. Go to http://localhost:9926/metrics on that machine in a web browser. You should see metrics there from the perf dump.

If I can get the build to work tomorrow I'll update this PR with the results from the above steps but if you can build the code quickly go ahead and run those steps.

@avanthakkar
Copy link
Contributor

I don't see counter dump/schema commands working anymore. so LGTM!

**$ ceph daemon /var/run/ceph/ceph-osd.2.asok counter dump**
no valid command found; 10 closest matches:
0
1
2
abort
assert
bench [<count:int>] [<size:int>] [<object_size:int>] [<object_num:int>]
bluefs debug_inject_read_zeros
bluefs files list
bluefs stats
bluestore allocator dump block
admin_socket: invalid command

@avanthakkar
Copy link
Contributor

Also ceph-exporter perfeclty working exposing the metrics at *:9926/metrics
https://paste.sh/a6HSJVJl#xf4DcwaYifZjXDkpTiGiuUr4

@idryomov
Copy link
Contributor

I don't see counter dump/schema commands working anymore.

@avanthakkar What do you mean by "anymore"? counter dump/schema commands never worked (were never present) in quincy.

@idryomov idryomov merged commit 2abad54 into ceph:quincy Oct 25, 2023
11 checks passed
@cbodley
Copy link
Contributor Author

cbodley commented Oct 25, 2023

thanks @idryomov @alimaredia @avanthakkar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants