On Wed, 29 Aug 2018 13:46:38 +0200 Magnus Hagander <mag...@hagander.net> wrote:
> On Wed, Aug 29, 2018 at 1:37 PM, Michael Banck <michael.ba...@credativ.de> > wrote: > > > Hi, > > > > On Wed, Aug 29, 2018 at 08:33:43PM +0900, Yugo Nagata wrote: > > > On Wed, 29 Aug 2018 10:28:33 +0200 > > > Daniel Gustafsson <dan...@yesql.se> wrote: > > > > > On 27 Aug 2018, at 14:05, Yugo Nagata <nag...@sraoss.co.jp> wrote: > > > > > On Mon, 27 Aug 2018 13:34:12 +0200 > > > > > Michael Banck <michael.ba...@credativ.de> wrote: > > > > >> On Mon, Aug 27, 2018 at 07:53:36PM +0900, Yugo Nagata wrote: > > > > >>> On Fri, 24 Aug 2018 18:01:09 +0200 > > > > >>> Peter Eisentraut <peter.eisentr...@2ndquadrant.com> wrote: > > > > >>>> I'm curious about this option: > > > > >>>> > > > > >>>> -r RELFILENODE check only relation with specified > > relfilenode > > > > >>>> > > > > >>>> but there is no facility to specify a database. > > > > >>>> > > > > >>>> Also, referring to the relfilenode of a mapped relation seems a > > bit > > > > >>>> inaccurate. > > > > >>>> > > > > >>>> Maybe reframing this in terms of the file name of the file you > > want > > > > >>>> checked would be better? > > > > >>> > > > > >>> If we specified 1234 to -r option, pg_verify_shceksums checks not > > only 1234 > > > > >>> but also 1234_vm, 1234_fsm, and 1234.1, 1234.2, ... and so on, so > > I think > > > > >>> it makes senses to allow to specify a relfilenode instead of a > > file name. > > > > >>> > > > > >>> I think it is reasonable to add a option to specify a database, > > although > > > > >>> I don't know which character is good because both -d and -D are > > already used.... > > > > >> > > > > >> Maybe the -d (debug) option should be revisited as well. Mentioning > > > > >> every scanned block generates a huge amount of output which might be > > > > >> useful during development but does not seem very useful for a stable > > > > >> release. AFAICT there is no other debug output for now. > > > > >> > > > > >> So it could be renamed to -v (verbose) and only mention each scanned > > > > >> file, e.g. (errors/checksum mismatches are still reported of > > course). > > > > I still think this should be changed as well, i.e. -v should not report > > every block scanned, as that really is debug output and IMO not useful > > in general? AFAICT your page does not change the output at all, just > > renames the option. > > > > > I agree with this (though it's my fault initially :P). Per-page output is > going to be useless in pretty much all production cases. It makes sense to > also change it to be per-file. I updated the patch to output only per-file information in the verbose mode. Does this behavior match you expect? Regards, -- Yugo Nagata <nag...@sraoss.co.jp>
commit ec21c608cd78f65747916487b56a197c685649c8 Author: Yugo Nagata <nag...@sraoss.co.jp> Date: Wed Aug 29 19:37:13 2018 +0900 Raname pg_verity_checksums debug option -d to -v / verbose Also, change to only mention each scanced file not every block. diff --git a/src/bin/pg_verify_checksums/pg_verify_checksums.c b/src/bin/pg_verify_checksums/pg_verify_checksums.c index 938b92282a..0f931b7579 100644 --- a/src/bin/pg_verify_checksums/pg_verify_checksums.c +++ b/src/bin/pg_verify_checksums/pg_verify_checksums.c @@ -31,7 +31,7 @@ static int64 badblocks = 0; static ControlFileData *ControlFile; static char *only_relfilenode = NULL; -static bool debug = false; +static bool verbose = false; static const char *progname; @@ -43,7 +43,7 @@ usage() printf(_(" %s [OPTION]... [DATADIR]\n"), progname); printf(_("\nOptions:\n")); printf(_(" [-D, --pgdata=]DATADIR data directory\n")); - printf(_(" -d debug output, list all checked blocks\n")); + printf(_(" -v, --verbose output verbose messages, list all checked blocks\n")); printf(_(" -r RELFILENODE check only relation with specified relfilenode\n")); printf(_(" -V, --version output version information, then exit\n")); printf(_(" -?, --help show this help, then exit\n")); @@ -120,7 +120,7 @@ scan_file(char *fn, int segmentno) progname, fn, blockno, csum, header->pd_checksum); badblocks++; } - else if (debug) + else if (verbose) fprintf(stderr, _("%s: checksum verified in file \"%s\", block %d: %X\n"), progname, fn, blockno, csum); } @@ -208,6 +208,7 @@ main(int argc, char *argv[]) { static struct option long_options[] = { {"pgdata", required_argument, NULL, 'D'}, + {"verbose", no_argument, NULL, 'v'}, {NULL, 0, NULL, 0} }; @@ -234,12 +235,12 @@ main(int argc, char *argv[]) } } - while ((c = getopt_long(argc, argv, "D:r:d", long_options, &option_index)) != -1) + while ((c = getopt_long(argc, argv, "D:r:v", long_options, &option_index)) != -1) { switch (c) { - case 'd': - debug = true; + case 'v': + verbose = true; break; case 'D': DataDir = optarg;