Re: Offline enabling/disabling of data checksums

From: Sergei Kornilov <sk(at)zsrv(dot)org>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Michael Banck <michael(dot)banck(at)credativ(dot)de>
Subject: Re: Offline enabling/disabling of data checksums
Date: 2019-03-11 14:11:11
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: tested, passed


I review latest patchset. I have one big question: Is pg_checksums safe for cross-versions operations? Even with update_controlfile call? Currently i am able to enable checksums in pg11 cluster with pg_checksums compiled on HEAD. Is this expected? I didn't notice any version-specific check in code.

And few small notes:

> <command>pg_checksums</command> checks, enables or disables data checksums

maybe better is <application>, not <command>?

> + printf(_("%s enables, disables or verifies data checksums in a PostgreSQL\n"), progname);
> + printf(_("database cluster.\n\n"));

I doubt this is good line formatting for translation purposes.

> + printf(_(" -c, --check check data checksums. This is the default\n"));
> + printf(_(" mode if nothing is specified.\n"));

same. For example pg_basebackup uses different multiline style:

> printf(_(" -r, --max-rate=RATE maximum transfer rate to transfer data directory\n"
> " (in kB/s, or use suffix \"k\" or \"M\")\n"));

> +command_fails(['pg_checksums', '--enable', '-r', '1234', '-D', $pgdata],
> + "fails when relfilnodes are requested and action is --disable");

action is "--enable" here ;-)

> if (badblocks > 0)
> return 1;

Small question: why return 1 instead of exit(1)?

> <refentry id="pgchecksums">
> <indexterm zone="pgchecksums">

How about use "app-pgchecksums" similar to other applications?

regards, Sergei

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-03-11 14:29:53 Re: [Patch] Log10 and hyperbolic functions for SQL:2016 compliance
Previous Message Andy Fan 2019-03-11 13:37:32 Re: Suggestions on message transfer among backends