Hi,
Am Donnerstag, den 14.03.2019, 15:26 +0100 schrieb Magnus Hagander:
> Given that the failure is data corruption, I don't think big fat
> warning is enough. We should really make it impossible to start up the
> postmaster by mistake during the checksum generation. People don't
> read the documentation until it's too late. And it might not even be
> under their control - some automated tool might go in and try to start
> postgres, and boom, corruption.
I guess you're right.
> One big-hammer method could be similar to what pg_upgrade does --
> temporarily rename away the controlfile so postgresql can't start, and
> when done, put it back.
That sounds like a good solution to me. I've made PoC patch for that,
see attached.
The only question is whether pg_checksums should try to move pg_control
back (i) on failure (ii) when interrupted?
Michael
--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax: +49 2166 9901-100
Email: michael.ba...@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
From 9b1471afee485a96c32e393989ca4eb0aed52f88 Mon Sep 17 00:00:00 2001
From: Michael Banck <mba...@debian.org>
Date: Thu, 14 Mar 2019 16:02:49 +0100
Subject: [PATCH] Rename away pg_control while enabling checksums.
In order to prevent that the cluster is accidently started during the
operation, the pg_control file is renamed to
pg_control.pg_checksums_in_progress during the operation and renamed back at
the end.
---
doc/src/sgml/ref/pg_checksums.sgml | 5 +++--
src/bin/pg_checksums/pg_checksums.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/ref/pg_checksums.sgml b/doc/src/sgml/ref/pg_checksums.sgml
index c3ccbf4eb7..91dfd3515a 100644
--- a/doc/src/sgml/ref/pg_checksums.sgml
+++ b/doc/src/sgml/ref/pg_checksums.sgml
@@ -48,8 +48,9 @@ PostgreSQL documentation
<para>
While checking or enabling checksums needs to scan or write every file in
the cluster, disabling will only update the file
- <filename>pg_control</filename>.
- </para>
+ <filename>pg_control</filename>. During enabling,
+ the <filename>pg_control</filename> </para> file is renamed in order to
+ prevent accidentally starting the cluster.
</refsect1>
<refsect1>
diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 852ddafc94..590fb925e8 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -55,6 +55,12 @@ typedef enum
#define PG_TEMP_FILES_DIR "pgsql_tmp"
#define PG_TEMP_FILE_PREFIX "pgsql_tmp"
+#ifndef WIN32
+#define pg_mv_file rename
+#else
+#define pg_mv_file pgrename
+#endif
+
static PgChecksumMode mode = PG_MODE_CHECK;
static const char *progname;
@@ -88,6 +94,7 @@ usage(void)
*/
static const char *const skip[] = {
"pg_control",
+ "pg_control.pg_checksums_in_progress",
"pg_filenode.map",
"pg_internal.init",
"PG_VERSION",
@@ -306,6 +313,8 @@ main(int argc, char *argv[])
};
char *DataDir = NULL;
+ char old_controlfile_path[MAXPGPATH];
+ char new_controlfile_path[MAXPGPATH];
int c;
int option_index;
bool crc_ok;
@@ -440,6 +449,23 @@ main(int argc, char *argv[])
exit(1);
}
+ /*
+ * Disable cluster by renaming pg_control when enabling checksums so
+ * that it cannot be started by accident during the operation
+ */
+ if (mode == PG_MODE_ENABLE)
+ {
+ snprintf(old_controlfile_path, sizeof(old_controlfile_path),
+ "%s/global/pg_control", DataDir);
+ snprintf(new_controlfile_path, sizeof(new_controlfile_path),
+ "%s/global/pg_control.pg_checksums_in_progress", DataDir);
+ if (pg_mv_file(old_controlfile_path, new_controlfile_path) != 0)
+ {
+ fprintf(stderr, _("%s: unable to rename %s to %s.\n"), progname, old_controlfile_path, new_controlfile_path);
+ exit(1);
+ }
+ }
+
/* Operate on all files if checking or enabling checksums */
if (mode == PG_MODE_CHECK || mode == PG_MODE_ENABLE)
{
@@ -466,6 +492,16 @@ main(int argc, char *argv[])
*/
if (mode == PG_MODE_ENABLE || mode == PG_MODE_DISABLE)
{
+
+ /* Move back pg_control */
+ if (mode == PG_MODE_ENABLE)
+ {
+ if (pg_mv_file(new_controlfile_path, old_controlfile_path) != 0)
+ {
+ fprintf(stderr, _("%s: unable to rename %s to %s.\n"), progname, new_controlfile_path, old_controlfile_path);
+ exit(1);
+ }
+ }
/* Update control file */
ControlFile->data_checksum_version =
(mode == PG_MODE_ENABLE) ? PG_DATA_CHECKSUM_VERSION : 0;
--
2.11.0
