Re: Incorrect fsync handling in pg_basebackup's tar_finish

Magnus Hagander <magnus@hagander.net>

From: Magnus Hagander <magnus@hagander.net>
To: Michael Paquier <michael@paquier.xyz>
Cc: Postgres hackers <pgsql-hackers@postgresql.org>
Date: 2018-06-25T08:57:29Z
Lists: pgsql-hackers
On Mon, Jun 25, 2018 at 4:43 AM, Michael Paquier <michael@paquier.xyz>
wrote:

> Hi all,
>
> I was just looking at the code of pg_basebackup, and noticed that we
> don't actually check if the two last empty blocks of any tar file
> produced are correctly fsync'd or not:
> @@ -957,7 +957,10 @@ tar_finish(void)
>
>  /* sync the empty blocks as well, since they're after the last file */
>  if (tar_data->sync)
> -   fsync(tar_data->fd);
> +   {
> +       if (fsync(tar_data->fd) != 0)
> +           return false;
> +   }
>
> That looks incorrect to me, hence shouldn't something like the attached
> be done?  Magnus and others, any opinions?
>

Yup, that seems like an issue and a correct fix to me.
-- 
 Magnus Hagander
 Me: https://www.hagander.net/ <http://www.hagander.net/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

Commits

  1. Correct handling of fsync failures with tar mode of walmethods.c