From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, TipTop Labs <office(at)tiptop-labs(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-bugs(at)postgresql(dot)org> |
Subject: | Re: BUG #14999: pg_rewind corrupts control file global/pg_control |
Date: | 2018-03-05 08:47:06 |
Message-ID: | CAD21AoDiHUUguQ7UUyK_Z4icHM3OiP8OEkpgDCsT3fg3B-P2LA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs pgsql-hackers |
On Mon, Mar 5, 2018 at 4:54 PM, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Fri, Mar 02, 2018 at 04:35:07PM +0100, Dmitry Dolgov wrote:
>> It maybe a stupid question, but why do you need to reset errno here? Is it to
>> avoid its value being carried from previous calls of `open_target_file`? In
>> this case if you put the code with `errno == EACCESS` under the if condition
>> `if (dstfd < 0)`, then as far as I remember you should always get relevant
>> errno. At the same time maybe it's valid to reset `errno` before `open`, like
>> with `getpriority`:
>
> [ ... reads and feels stupid ...]
>
> Of course all the checks should be where dstno is negative...
>
> I have done a second pass on the patch, and attached is a new version.
> This fixes this handling of errno and addresses some typos. I have also
> fixed the test case where one of the read-only files was not properly
> switched to do so. I have also added a commit log message.
@@ -24,7 +24,10 @@
typedef enum
{
FILE_ACTION_CREATE, /* create local
directory or symbolic link */
- FILE_ACTION_COPY, /* copy whole file,
overwriting if exists */
+ FILE_ACTION_COPY_CONFIG, /* copy whole configuration
file, overwriting
+ * if exists */
+ FILE_ACTION_COPY_DATA, /* copy whole relation file,
overwriting if
+ * exists */
FILE_ACTION_COPY_TAIL, /* copy tail from 'oldsize' to
'newsize' */
FILE_ACTION_NONE, /* no action (we might
still copy modified
*
blocks based on the parsed WAL) */
Since files other than relation files such as vm, fsm, WAL are
categorize as FILE_ACTION_COPY_CONFIG, I think the name would cause
misreading. For example, we can use FILE_ACTION_COPY_DATA and
FILE_ACTION_COPY_OTHER?
As you mentioned before, with this patch we end up ignoring file-open
error of database files other than relation files. I guess it's a bit
risky. We can enter them to COPY_DATA but I'd defer it to committer.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2018-03-05 09:01:11 | Re: BUG #14999: pg_rewind corrupts control file global/pg_control |
Previous Message | Michael Paquier | 2018-03-05 07:54:31 | Re: BUG #14999: pg_rewind corrupts control file global/pg_control |
From | Date | Subject | |
---|---|---|---|
Next Message | Jeevan Chalke | 2018-03-05 08:51:08 | Re: [HACKERS] Partition-wise aggregation/grouping |
Previous Message | Vik Fearing | 2018-03-05 08:46:44 | Re: PATCH: psql tab completion for SELECT |