Skip to content

Remove hard code mapping in dstat tool, Resolves git issue #2017 #2218

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lmchilton
Copy link
Contributor

Updated pcp dstat tool to not rely on hard
coded mappings between instance filtering
command line options and the specific plugin
they apply to. Updated QA test #1207 and the
pcp-dstat(5) man page.

Linked to PR #2196

Updated pcp dstat tool to not rely on hard
coded mappings between instance filtering
command line options and the specific plugin
they apply to. Updated QA test performancecopilot#1207 and the
pcp-dstat(5) man page.
@lmchilton lmchilton changed the title Resolves git issue #2017 Remove hard code mapping in dstat tool, Resolves git issue #2017 Jun 6, 2025
@lmchilton lmchilton requested a review from wcohen June 6, 2025 16:24
@wcohen
Copy link
Contributor

wcohen commented Jun 12, 2025

The patch is looking really close. One issue I saw when trying out the branch with the tests is that 1187 fails with the patch. It looks like the following line in the test is failing because the -D sda,total isn't being handled correctly:

dstat "Per Disk" --disk -D sda,total --disk-wait

For pcp-dstat.5 it would be good to describe the mapping of cpu, disk, etc. to the -C, -D, etc. like what is done for printtype.

@wcohen
Copy link
Contributor

wcohen commented Jun 20, 2025

One of the things that bothered me was that qa test 1187 failed when this patch was installed. However, it appears to be an issue with the test itself. The old hardcoded dstat would always prints each disk metrics for disk-wait plugin regardless of what was selected with the -D option. The qa1187 test is trying to get data for an disk that isn't in the data, sda. There is a vda. Would suggest an additional patch that correct the qa 1187 to provide more realistic test:

dstat "Per Disk" --disk -D sda,total --disk-wait

And adjust the output to be the expected:

----system---- --dsk/vda----dsk/total- ---vda---
     time     | read  writ: read  writ|rawa wawa
10-02 17:04:49|           :           |         
10-02 17:04:50|   0   235k:   0   235k|   0 0.00
10-02 17:04:51|   0     0 :   0     0 |   0    0
10-02 17:04:52|   0     0 :   0     0 |   0    0

At this point the patch appears to be working as expected. So once there are patches for pcp-dstat.5 documentation and the qa/1187 test to function correctly. This should be merged in.

@wcohen
Copy link
Contributor

wcohen commented Jun 20, 2025

I have put patches for the update to the pcp-dstat.5 and qa/1187 on https://github.com/wcohen/pcp/tree/wcohen/pr2218

@wcohen
Copy link
Contributor

wcohen commented Jul 11, 2025

I am comfortable that the patch is reasonable at this point. When experimenting with the patch for support the GPU plugin I addressed a number of other issues and place the patches on https://github.com/wcohen/pcp/tree/wcohen/

  • A couple of the tests qa/1187 and qa1207 were not quite right
  • dstat would always set the default grouptype to 2 even if the grouptype in the plugin was something else
  • A few of the plugins needs to specify the default grouptype

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

Successfully merging this pull request may close these issues.

2 participants