Re: Incorrect fsync handling in pg_basebackup's tar_finish - Mailing list pgsql-hackers

From Kuntal Ghosh
Subject Re: Incorrect fsync handling in pg_basebackup's tar_finish
Date
Msg-id CAGz5QCKXFwuG_e1NBE20mSQdZENnna6ybk_8u=kzx=Cb8AXTsw@mail.gmail.com
Whole thread Raw
In response to Re: Incorrect fsync handling in pg_basebackup's tar_finish  (Magnus Hagander <magnus@hagander.net>)
Responses Re: Incorrect fsync handling in pg_basebackup's tar_finish
List pgsql-hackers
On Mon, Jun 25, 2018 at 2:27 PM, Magnus Hagander <magnus@hagander.net> wrote:
>
>
> 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?
In the same note, in tar_close(), we fsync on close. We're not
checking the status of fsync there. Should we introduce the same check
there as well?


-- 
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com


pgsql-hackers by date:

Previous
From: Kuntal Ghosh
Date:
Subject: Re: Loaded footgun open_datasync on Windows
Next
From: Korry Douglas
Date:
Subject: Re: PATCH: backtraces for error messages