Horton

Re: new heapcheck contrib module

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: Thomas Munro <thomas.munro@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Peter Geoghegan <pg@bowt.ie>, "Andrey M. Borodin" <x4mmm@yandex-team.ru>, Stephen Frost <sfrost@snowman.net>, Michael Paquier <michael@paquier.xyz>, Amul Sul <sulamul@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Andres Freund <andres@anarazel.de>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2021-02-17T22:09:19Z
Lists: pgsql-hackers 
On Wed, Feb 17, 2021 at 1:46 PM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
> Reworking the code took a while.  Version 39 patches attached.

Regarding the documentation, I think the Usage section at the top is
far too extensive and duplicates the option description section to far
too great an extent. You have 21 usage examples for a command with 34
options. Even if we think it's a good idea to give a brief summary of
usage, it's got to be brief; we certainly don't need examples of
obscure special-purpose options like --maintenance-db here. Looking
through the commands in "PostgreSQL Client Applications" and
"Additional Supplied Programs," most of them just have a synopsis
section and nothing like this Usage section. Those that do have a
Usage section typically use it for a narrative description of what to
do with the tool (e.g. see pg_test_timing), not a long list of
examples. I'm inclined to think you should nuke all the examples and
incorporate the descriptive text, to the extent that it's needed,
either into the descriptions of the individual options or, if the
behavior spans many options, into the Description section.

A few of these examples could move down into an Examples section at
the bottom, perhaps, but I think 21 is still too many. I'd try to
limit it to 5-7. Just hit the highlights.

I also think that perhaps it's not best to break up the list of
options into so many different categories the way you have. Notice
that for example pg_dump and psql don't do this, instead putting
everything into one ordered list, despite also having a lot of
options. This is arguably worse if you want to understand which
options are related to each other, but it's better if you are just
looking for something based on alphabetical order.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Commits

  1. Add pg_amcheck, a CLI for contrib/amcheck.

  2. Refactor and generalize the ParallelSlot machinery.

  3. Generalize parallel slot result handling.

  4. Move some code from src/bin/scripts to src/fe_utils to permit reuse.

  5. Factor pattern-construction logic out of processSQLNamePattern.

  6. Doc: clean up verify_heapam() documentation.

  7. Fix more portability issues in new amcheck code.

  8. Fix portability issues in new amcheck test.

  9. Try to avoid a compiler warning about using fxid uninitialized.

  10. Extend amcheck to check heap pages.

  11. Adjust walsender usage of xlogreader, simplify APIs

  12. Improve checking of child pages in contrib/amcheck.

  13. Sanitize line pointers within contrib/amcheck.

  14. Fix possible sorting error when aborting use of abbreviated keys.