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:
> >
> >> Hi,
> >>
> >> 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).
> >>
> >> Then -d could (in the future, I guess that is too late for v11) be used
> >> for -d/--dbname (or make that only a long option, if the above does not
> >> work).
> >
> > I realized after sending the previous post that we can not specify a
> > database
> > by name because pg_verify_checksum is run in offline and this can not get
> > the
> > OID from the database name. Also, there are global and pg_tblspc
> > directories
> > not only base/<database OID>. So, it seems to me good to specify a
> > directories
> > to scan which is under PGDATA. We would be able to use -d ( or --directory
> > ?)
> > for this purpose.
>
> Changing functionality to the above discussed is obviously 12 material, but
> since we are discussing changing the command line API of the tool by
> repurposing -d; do we want to rename the current use of -d to -v (with the
> accompanying ―-verbose) before 11 ships? It’s clearly way way too late in the
> cycle but it seems worth to at least bring up since 11 will be the first
> version pg_verify_checksums ship in. I’m happy to do the work asap if so.
I agree with this. Almost other commands doesn't use -d as debug mode
although there a few exception, and instead -v is used for verbose mode.
If we can change the command line of pg_veriry_checksums, before release of
PG 11 is best. Attached is the patch to do this.
Regards,
--
Yugo Nagata <nag...@sraoss.co.jp>
commit 452f981c30c9d9c7263e6006dc4cda51278ff376
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
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;
