From: | Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com> |
---|---|
To: | Magnus Hagander <magnus(at)hagander(dot)net> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Postgres hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Incorrect fsync handling in pg_basebackup's tar_finish |
Date: | 2018-06-25 12:18:54 |
Message-ID: | CAGz5QCKXFwuG_e1NBE20mSQdZENnna6ybk_8u=kzx=Cb8AXTsw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Jun 25, 2018 at 2:27 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>
>
> On Mon, Jun 25, 2018 at 4:43 AM, Michael Paquier <michael(at)paquier(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Korry Douglas | 2018-06-25 12:25:29 | Re: PATCH: backtraces for error messages |
Previous Message | Kuntal Ghosh | 2018-06-25 11:58:08 | Re: Loaded footgun open_datasync on Windows |