Re: Broken --dry-run mode in pg_rewind - Mailing list pgsql-hackers

From Vladimir Borodin
Subject Re: Broken --dry-run mode in pg_rewind
Date
Msg-id DAE88CA1-FC41-47A4-B469-4BEAB135FAC2@simply.name
Whole thread Raw
In response to Re: Broken --dry-run mode in pg_rewind  (Stephen Frost <sfrost@snowman.net>)
Responses Re: Broken --dry-run mode in pg_rewind  (Vladimir Borodin <root@simply.name>)
List pgsql-hackers

8 мая 2015 г., в 16:11, Stephen Frost <sfrost@snowman.net> написал(а):

* Heikki Linnakangas (hlinnaka@iki.fi) wrote:
On 05/08/2015 03:39 PM, Michael Paquier wrote:
On Fri, May 8, 2015 at 9:34 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 05/08/2015 03:25 PM, Vladimir Borodin wrote:
Seems, that pg_rewind does not account --dry-run option properly. A simple
fix
for that is attached.


No, the --dry-run takes effect later. It performs all the actions it
normally would, including reading files from the source, except for actually
writing anything in the target. See the dry-run checks in file_ops.c

Urgh, my test script had an error and I made grep only in pg_rewind.c. Sorry for noise.


Even if the patch sent is incorrect, shouldn't there be some process
bypass in updateControlFile() and createBackupLabel() in case of a
--dry-run?

They both use open_target_file() and write_target_file(), which
check for --dry-run and do nothing if it's set.

Hmm, I wonder it we should print something else than "Done!" at the
end, if run in --dry-run mode. Or give some indication around the
time it says "Rewinding from last common checkpoint at ...", that
it's running in dry-run mode and won't actually modify anything. The
progress messages are a bit alarming if you don't realize that it's
skipping all the writes.

That would be really nice.


Wouldn't hurt to also augment that rather doom-looking "point of no
return" comment with a note that says writes won't happen if in
dry-run. :)

For my 2c anyway.

Thanks!

Stephen


--
May the force be with you…

pgsql-hackers by date:

Previous
From: Simon Riggs
Date:
Subject: Re: subxcnt defined as signed integer in SnapshotData and SerializeSnapshotData
Next
From: Amit Langote
Date:
Subject: Remove obsolete mention of src/tools/backend