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

Documentation feedback from first external reviewer #60

Open
portante opened this issue Aug 14, 2015 · 3 comments
Open

Documentation feedback from first external reviewer #60

portante opened this issue Aug 14, 2015 · 3 comments

Comments

@portante
Copy link
Member

From Doug Williams ([email protected]):

Here's some initial comments and suggested edits to pbench-agent.html.

General Comments:

The documentation flow seems to mix of basic, intermediate and advanced topics. It may make sense to segregate these topics by complexity. An example flow:

  • Running a pre-packaged benchmark on a single node
  • What data is collected, and selection of tools (default vs optional)
  • Running benchmark on multiple nodes
  • Running user-specified benchmark
  • Adding new benchmark to pbench
  • Adding new tool to pbench

Some of the text is in an informal conversational style using personal pronouns, 'You', 'I'. I've included some proposed edits if your decide to go with a more formal 3rd person style. I view this as a secondary issue.

Section 1: WARNING

DDW COMMENT: Use of 'some' may be unnecessary, may want to consider the following ...

This document may describe future capabilities of pbench.
Currently both code and documentation are undergoing
active development, and while we strive for consistency,
if you find something not working as described here, please
let us know.  It may be a bug in the documentation, a bug in
the code or a feature not yet implemented.

Section 2: What is pbench?

DDW COMMENT: Text seemed a bit confusing, may want to consider the following ...

... Pbench includes built-in scripts supporting many common
benchmarks such as cyclictest, debench, fio, linpack, migrate,
iozone, netperf, specjbb2005 and uperf.  Options for use of
Pbench with other (non-built-in) benchmarks include:
  o Running pbench in collector-only mode, separately running
     benchmark
  o Extending pbench by through development a benchmark-specific
     pbench script.
Such contributions are more than welcome!

DDW QUESTION: How does one extend Pbench for additional data collectors?

Section 3: Quick links

DDW COMMENT: Both URLS are currently invalid

- Results Directory - http://pbench.example.com/results/
- pbench RPM repo - http://pbench.example.com/repo

Section 4: TL;DR version

DDW COMMENT: Need to update URL to reflect repo location. Will you be using GitHub as a repo?

wget -O /etc/yum.repos.d/pbench.repo http://repohost.example.com/repo/yum.repos.d/pbench.repo

Section 5: How to install

DDW COMMENT: Similar issue concerning repo URL

http://repohost.example.com/repo/yum.repos.d

and

wget -O /etc/yum.repos.d/pbench.repo http://repohost.example.com/repo/yum.repos.d/pbench.repo

Section 5.1: Updating pbench

DDW COMMENT: Nit, section is written with a conversational style, such as 'I' and 'you'.

Consider:

Since the pbench package and associated benchmark and tools RPMS are
updated frequently, it may be necessary to clean the yum cache in order for
yum to see any new versions.  If during update yum reports no packages to
update, try again after cleaning the cache:

<<Command Sequence>>

If may be necessary to logout and re-login after changes. If the above update
encounters problems, try the following workaround:

<<Command Sequence>>

.....

The workaround should not be necessary if currently installed release is
0.31-95 or later.

.....

When upgrading to a release later than -102, due to changes in label
handling it is necessary to clear out and re-register tools post upgrade.
For example:

<<Command Sequence>>

Section 6: First Steps

DDW COMMENT: Another first-person to 3rd person change ...

...
Built-in benchmarks can be run by invoking the associated pbench_XXX
script
  - pbench will install the benchmark if necessary:
...

Section 6.1: First Steps with user-benchmark

DDW COMMENT: should Section 2 make reference to 6.1 (user-benchmark) in context of 'but the data collection can be run separately as well with a benchmark that is not built-in to pbench ....'

DDW COMMENT: Stylistic change to remove first person

Consider:

A user-benchmark script can be used to run other benchmark in addition
to the benchmarks pre-packaged with pbench.  user-benchmark takes a
command as argument ...

Section 6.2: First Steps with Remote Hosts and user-benchmark

DDW COMMENT: This section appears to be the first treatment of multi-host benchmarks. My recommendation is that you show and example of a multi-host packaged benchmark, then show an example of a multi-host user-benchmark.

Section 8: Available tools

DDW COMMENT: Stylistic nit, consider the following wording

register-tool-set configures the following tools by default:

DDW COMMENT: Error in last command sequence

Current:

unregister --name=perf
register-tool --name=perf -- --record-opts="record -a --freq=200"

Should read:

unregister-tool --name=perf
register-tool --name=perf -- --record-opts="record -a --freq=200"

Section 9: Available Benchmark Scripts

DDW COMMENT: Which of these benchmarks support multi-host operation?

DDW COMMENT: Stylistic nit

Consider:

Note that in many of these scripts the default tool group is hard-wired:

edits to the appropriate script may be required when using different tool group

Section 10: Utility Scripts

DDW COMMENT: Stylistic nit

Consider:

This section provides background for the Second steps section below.

Pbench uses a utility scripts to do common operations.  Many of the
utility scripts support the following options:
  --name to specify a tool
  --group to specify a tool group
  --with-options to list or pass options to a tool
  --remote to operate on a remote host

See entries in the FAQ section below for more details on these options.

DDW COMMENT: Consider headings 'Tool Registration related Utility Scripts', 'Tool Control related Utility Scripts', 'Results and Post Processing related Utility Scripts', and 'Miscellaneous Utility Scripts'

Section 11: Second Steps

DDW COMMENT: The warning is a bit confusing. If you're recommending against user-benchmarks, then move this content later in docs. If it's something else, such as ad-hoc scripts, then I would advise that you move the treatment of extensibility of user creation of benchmark scripts into an advanced section.

Section 12: Running Pbench Collection Tools with an Arbitrary Benchmark

DDW COMMENT: Should the warning in Section 11 be moved here?

@portante
Copy link
Member Author

@arcolife, @atheurer, @ndokos, can one of you consider Doug's suggestions and review feedback?

@ndokos
Copy link
Member

ndokos commented Aug 14, 2015

I'll work on this: I've long wanted to reorganize it along complexity
lines.

@jayunit100
Copy link

thanks for creating this issue ; the repohost.example.com links can be a little misleading, i would remove those. the doc page looks like it good be very useful if the introduction section became a little more simplified for new folks, and then the later parts, even if hard to follow, might be easier to digest if a working basic example was there to begin with.

@ashishkamra ashishkamra added this to the V0.48 milestone Oct 23, 2017
@ashishkamra ashishkamra assigned ashishkamra and unassigned ndokos Jan 18, 2018
@ndokos ndokos removed this from the V0.48 milestone Feb 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants