pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Michael Banck <michael.banck@credativ.de>
To: Michael Paquier <michael@paquier.xyz>
Cc: Magnus Hagander <magnus@hagander.net>
Cc: Fabien COELHO <coelho@cri.ensmp.fr>
Cc: Sergei Kornilov <sk@zsrv.org>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Offline enabling/disabling of data checksums
Date: Fri, 15 Mar 2019 12:54:01 +0100
Message-ID: <1552650841.9697.22.camel@credativ.de> (raw)
In-Reply-To: <20190315025027.GN3493@paquier.xyz>
References: <20190313021621.GP13812@paquier.xyz>
	<alpine.DEB.2.21.1903130805330.4059@lancre>
	<20190313093150.GE2988@paquier.xyz>
	<1552473675.4947.62.camel@credativ.de>
	<CABUevExWFGMZktS2UEYfKbgsEKLVnrQxv5M8GMYFgiBmQy+cNA@mail.gmail.com>
	<1552475364.4947.64.camel@credativ.de>
	<20190313115633.GA1878@paquier.xyz>
	<20190314043936.GH3493@paquier.xyz>
	<CABUevExjCOKDRX=B6fBkoYs7Q2jYJLZS4Bxh1pi0b2_BV-7DEA@mail.gmail.com>
	<1552577180.9697.4.camel@credativ.de>
	<20190315025027.GN3493@paquier.xyz>

Hi,

Am Freitag, den 15.03.2019, 11:50 +0900 schrieb Michael Paquier:
> I have been able to grab some time to incorporate the feedback gathered
> on this thread, and please find attached a new version of the patch to
> add --enable/--disable. 

Some more feedback:

1. There's a typo in line 578 which makes it fail to compile:

|src/bin/pg_checksums/pg_checksums.c:578:4: error: ‘y’ undeclared (first use in this function)
|   }y

2. Should the pqsignal() stuff only be setup in PG_MODE_ENABLE? Same
with the controlfile_path?

3. There's (I think) leftover debug output in the following places:

|+		printf(_("Renaming \"%s\" to \"%s\"\n"), controlfile_path,
|+				controlfile_path_temp);

|+		printf(_("Renaming \"%s\" to \"%s\"\n"), controlfile_path_temp,
|+				controlfile_path);

|+		printf(_("Syncing data folder\n"));

(that one is debatable, we are mentioning this only in verbose mode in
pg_basebackup but pg_checksums is more chatty anyway, so probably fine).

|+		printf(_("Updating control file\n"));

Besides to the syncing message (which is user-relevant cause they might
wonder what is taking so long), the others seem to be implementation
details we don't need to tell the user about.


Michael

-- 
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax:  +49 2166 9901-100
Email: michael.banck@credativ.de

credativ GmbH, HRB Mönchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer

Unser Umgang mit personenbezogenen Daten unterliegt
folgenden Bestimmungen: https://www.credativ.de/datenschutz




view thread (149+ messages)  latest in thread

Message-ID: <1552650841.9697.22.camel@credativ.de>
Permalink:  ../1552650841.9697.22.camel@credativ.de/
Also on:    postgresql.org/message-id/1552650841.9697.22.camel@credativ.de

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: michael.banck@credativ.de, michael@paquier.xyz, magnus@hagander.net, coelho@cri.ensmp.fr, sk@zsrv.org, pgsql-hackers@lists.postgresql.org
  Subject: Re: Offline enabling/disabling of data checksums
  In-Reply-To: <1552650841.9697.22.camel@credativ.de>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox