Обсуждение: Broken --dry-run mode in pg_rewind

On 05/08/2015 03:25 PM, Vladimir Borodin wrote:
> Hi all.
>
> 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

- Heikki

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

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

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
>
> 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.

- Heikki

* 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
> >
> >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.

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