From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: TipTop Labs <office@tiptop-labs.com>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: PostgreSQL mailing lists <pgsql-bugs@postgresql.org>
Subject: Re: BUG #14999: pg_rewind corrupts control file global/pg_control
Date: Mon, 9 Apr 2018 13:58:26 +0900
Message-ID: <20180409045826.GA1740@paquier.xyz> (raw)
In-Reply-To: <20180407222258.GA3345@paquier.xyz>
References: <30255.1522711675@sss.pgh.pa.us>
<20180403024110.GB1621@paquier.xyz>
<A89569FD-1CCB-4060-932B-B61DD0F756BF@tiptop-labs.com>
<20180404072625.GB2208@paquier.xyz>
<22961.1522867812@sss.pgh.pa.us>
<20180405013818.GC1732@paquier.xyz>
<E1979209-A39E-4229-BBE0-59BC43FB4691@tiptop-labs.com>
<20180405065840.GH1732@paquier.xyz>
<15971.1523116292@sss.pgh.pa.us>
<20180407222258.GA3345@paquier.xyz>
On Sun, Apr 08, 2018 at 07:22:58AM +0900, Michael Paquier wrote:
> Yes, that's one of the methods I mentioned in my previous email. Now,
> do we want to fail the run immediately if such a file is found? Or do
> we want to report at once all such files so as the user does not need to
> run multiple times pg_rewind.
So I have been playing with that part...
> The latter would be much more useful.
While that would be nice, the result is rather ugly and this needs to
generate the same error message in two code paths. So I have chewed
things in such a way that one failure found makes the whole rewind to
just fail, and allows retries to work. The attached has a test as well
as documentation.
Thoughts?
> A downside here is that the file from the source would still be fetched
> and copied on the target. So a file which was in read-only would become
> basically writable. Don't think that it is a big deal as the has
> superuser rights at this point, but that's worth mentioning.
That's mentioned in the docs of the attached.
--
Michael
Attachments:rewind-readonly-v4.patch (text/x-diff, 5.4K) downloaddiff --git a/doc/src/sgml/ref/pg_rewind.sgml b/doc/src/sgml/ref/pg_rewind.sgmlindex 520d843f0e..0952f3903e 100644--- a/doc/src/sgml/ref/pg_rewind.sgml+++ b/doc/src/sgml/ref/pg_rewind.sgml@@ -95,6 +95,25 @@ PostgreSQL documentation
are currently on by default. <xref linkend="guc-full-page-writes"/>
must also be set to <literal>on</literal>, but is enabled by default.
</para>
++ <para>+ Before rewriting any files on the target data directory,+ <application>pg_rewind</application> checks if any files are writable,+ failing immediately if that is not the case. It is possible to+ execute again a rewind after fixing the permission issues. This can+ happen for example for read-only SSL-related configuration files.+ Note that if the file is copied from the source to the target, then+ it may be necessary to rebuild the links properly as a rewind would+ have caused those files to be created with newer permission sets.+ </para>++ <warning>+ <para>+ If <application>pg_rewind</application> fails while processing, then+ the data folder of the target is likely not in a state that can be+ recovered. In such a case, taking a new fresh backup is recommended.+ </para>+ </warning>
</refsect1>
<refsect1>
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.cindex c3fc519895..c0b58c76ce 100644--- a/src/bin/pg_rewind/filemap.c+++ b/src/bin/pg_rewind/filemap.c@@ -189,7 +189,56 @@ process_source_file(const char *path, file_type_t type, size_t newsize,
exists = false;
}
else
- exists = true;+ {+ int fd;++ /*+ * The file exists on the source and the target. Now check if+ * the file is writable.+ */+ fd = open(localpath, O_WRONLY);++ if (fd < 0)+ {+ if (errno == EISDIR)+ {+ /*+ * No need to worry about directories, those are not+ * valid targets for writes.+ */+ exists = true;+ }+ else if (errno == EACCES)+ {+ /*+ * The file exists on the source and the target but it is+ * not writable on target. In order to avoid failures+ * mid-flight when processing the data directory, fail+ * immediately and let the user know. This way, if anything+ * like a read-only file is found, then pg_rewind complains+ * and a successive run can be completed after doing some+ * cleanup on this target data directory.+ */+ pg_fatal("file \"%s\" exists on both target and source server, but is not writable on target.\n"+ "Please check the permissions on \"%s\" before executing again pg_rewind.\n",+ path, localpath);+ }+ else+ {+ /*+ * Complain about anything else, that should not happen,+ * even ENOENT which has already been checked above.+ */+ pg_fatal("could not open file \"%s\": %s\n",+ localpath, strerror(errno));+ }+ }+ else+ {+ exists = true;+ close(fd);+ }+ }
switch (type)
{
diff --git a/src/bin/pg_rewind/t/006_readonly.pl b/src/bin/pg_rewind/t/006_readonly.plnew file mode 100644index 0000000000..489e1f4593--- /dev/null+++ b/src/bin/pg_rewind/t/006_readonly.pl@@ -0,0 +1,77 @@+# Test how pg_rewind reacts to read-only files in the data dirs.+# All such files should be ignored in the process.++use strict;+use warnings;+use TestLib;+use Test::More tests => 2;++use File::Copy;+use File::Find;++use RewindTest;++my $test_mode = "remote";++RewindTest::setup_cluster($test_mode);+RewindTest::start_master();+RewindTest::create_standby($test_mode);++# Create the same read-only file in standby and master+my $test_master_datadir = $node_master->data_dir;+my $test_standby_datadir = $node_standby->data_dir;+my $readonly_master = "$test_master_datadir/readonly_file";+my $readonly_standby = "$test_standby_datadir/readonly_file";++append_to_file($readonly_master, "in master");+append_to_file($readonly_standby, "in standby");+chmod 0400, $readonly_master, $readonly_standby;++RewindTest::promote_standby();++# Stop the master and run pg_rewind.+$node_master->stop;++my $master_pgdata = $node_master->data_dir;+my $standby_pgdata = $node_standby->data_dir;+my $standby_connstr = $node_standby->connstr('postgres');+my $tmp_folder = TestLib::tempdir;++# Keep a temporary postgresql.conf for master node or it would be+# overwritten during the rewind.+copy(+ "$master_pgdata/postgresql.conf",+ "$tmp_folder/master-postgresql.conf.tmp");++# Including read-only data in the source and the target will+# cause pg_rewind to fail.+my $rewind_command = [ 'pg_rewind', "--debug",+ "--source-server", $standby_connstr,+ "--target-pgdata=$master_pgdata" ];++command_fails($rewind_command, 'pg_rewind fails with read-only');++# Now remove the read-only data on both sides, the data folder+# from the previous attempt should still be able to work.+unlink($readonly_master);+unlink($readonly_standby);+command_ok($rewind_command, 'pg_rewind passes without read-only');++# Now move back postgresql.conf with old settings+move("$tmp_folder/master-postgresql.conf.tmp",+ "$master_pgdata/postgresql.conf");++# Plug-in rewound node to the now-promoted standby node+my $port_standby = $node_standby->port;+$node_master->append_conf('recovery.conf', qq(+primary_conninfo='port=$port_standby'+standby_mode=on+recovery_target_timeline='latest'+));++# Restart the master to check that rewind went correctly.+$node_master->start;++RewindTest::clean_rewind_test();++exit(0);
signature.asc (application/pgp-signature, 832B) download
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-bugs@postgresql.org
Cc: michael@paquier.xyz, tgl@sss.pgh.pa.us, office@tiptop-labs.com, 9erthalion6@gmail.com, sawada.mshk@gmail.com
Subject: Re: BUG #14999: pg_rewind corrupts control file global/pg_control
In-Reply-To: <20180409045826.GA1740@paquier.xyz>
* 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