Simplify final sync in pg_rewind's target folder and add --no-sync

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Postgres hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Simplify final sync in pg_rewind's target folder and add --no-sync
Date: 2018-03-25 12:26:07
Message-ID: 20180325122607.GB3707@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Hi all,

While looking at pg_rewind code, I have been surprised to find that the
final fsync done on the target's data folder is done using initdb -S via
a system() call. This is in my opinion overcomplicated because we have
a dedicated API in fe_utils able to do a fsync on a data folder, called
fsync_pgdata() that I have implemented when working on data durability
for the other backup tools. So I would like to simplify the code as
attached.

One difference that this patch introduces is that a failed sync is not
considered as a failure, still failures are reported to stderr. This
new behavior is actually more consistent with what happens in pg_dump
and pg_basebackup. And we have decided previously to do so, see here
for more details on the discussion:
https://www.postgresql.org/message-id/CAB7nPqQ_B0j3n1t%3D8c1ZLHXF1b8Tf4XsXoUC9bP9t5Hab--SMg%40mail.gmail.com

An extra thing I have noticed, which is I think an oversight, is that
there is no --no-sync option in pg_rewind. Like the other binaries,
this is useful to reduce the I/O effort when running tests.

Both things are implemented as attached. I am of course not pushing for
integrating that patch in v11 even if it is straight-forward, so I'll
park it in the next future commit fest.

Thanks,
--
Michael

Attachment Content-Type Size
rewind-sync-fixes.patch text/x-diff 5.4 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message MauMau 2018-03-25 13:15:52 Re: [bug fix] ECPG: freeing memory for pgtypes crashes on Windows
Previous Message Fabien COELHO 2018-03-25 12:23:21 Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors