Re: pg_rewind failure by file deletion in source server

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_rewind failure by file deletion in source server
Date: 2015-06-19 04:14:57
Message-ID: CAB7nPqSUrFVLMsR1bKkuQ40gMqxFYyR31hacMVtYkkN0N=sO7Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jun 18, 2015 at 10:55 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> On 06/16/2015 02:04 AM, Michael Paquier wrote:
>>
>> In order to reduce the risk of failure to a minimum and to preserve
>> the performance of the tool when using --source-server, I think that
>> we should add some check using pg_stat_file to see if a file is still
>> present or not, and if it is missing we can safely skip it thanks to
>> minRecoveryPoint. Now the problem is that pg_stat_file fails
>> automatically if the file targeted is missing. Hence, to avoid a bunch
>> of round trips with the server with one call to pg_stat_dir per file,
>> I think that we should add some if_not_exists option to pg_stat_file,
>> defaulting to false, to skip the error related to the file missing and
>> have it return NULL in this case. Then we could use this filter on the
>> file path in libpq_executeFileMap() to fetch only the file chunks that
>> actually exist on the server.
>
>
> You'll also need to add the option to pg_read_binary_file, though, because
> even if you do a test with pg_stat_file() just before reading the file,
> there's a race condition: someone might still delete file between
> pg_stat_file() and pg_read_file().

I propose to return NULL values if the file does not exist and
if_not_exists = true for both of them. Does that sound fine?

> Listing the directories with pg_ls_dir() has the same problem.

(After some discussion on IM with Heikki on this one).
This is actually more tricky because pg_ls_dir() does not return '.'
or '..' that we could use to identify that the directory actually
exists or not when it is empty. Hence I think that we should add two
options to pg_ls_dir:
- include_self, default to false. If set to true, '.' is added in the
list of items.
- if_not_exists, to bypass error that a folder does not exist, default
at false. If if_not_exists = true and include_self = true, returning
only '.' would mean that the folder exist but that it is empty. If
if_not_exists = true and include_self = false, no rows are returned.
We could as well ERROR as well if both options are set like that. I am
fine with any of them as long as behavior is properly documented.

> As does pg_tablespace_location().

NULL if tablespace path does not exist anymore. Is that fine.

>> Note that we could as well use some plpgsql-ing to do the same, but
>> the extension of pg_stat_file looks more useful to me. Thoughts?
>
>
> Hmm. You'll need to add the option to all of those functions. Maybe it's
> nevertheless the simplest approach.

With plpgsql you could use a try/catch/raising block to do the work.
But it still looks better to me to have alternative options with the
in-core functions.

I am fine to spend time on all those things and provide test cases,
let's just get a precise picture of what we want first.
Regards,
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2015-06-19 04:17:23 Re: dblink: add polymorphic functions - review
Previous Message Brendan Jurd 2015-06-19 01:12:14 Re: proposal: row_to_array function