Re: [HACKERS] Support for pg_receivexlog --format=plain|tar

Поиск
Список
Период
Сортировка
От Magnus Hagander
Тема Re: [HACKERS] Support for pg_receivexlog --format=plain|tar
Дата
Msg-id CABUevEyOZb76gUGdgSQeGcme9tWT5hBnPJjBMnp19q6y-mLitQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [HACKERS] Support for pg_receivexlog --format=plain|tar  (Michael Paquier <michael.paquier@gmail.com>)
Ответы Re: [HACKERS] Support for pg_receivexlog --format=plain|tar  (David Steele <david@pgmasters.net>)
Re: [HACKERS] Support for pg_receivexlog --format=plain|tar  (Michael Paquier <michael.paquier@gmail.com>)
Список pgsql-hackers


On Fri, Dec 30, 2016 at 6:41 AM, Michael Paquier <michael.paquier@gmail.com> wrote:
On Thu, Dec 29, 2016 at 6:13 PM, Magnus Hagander <magnus@hagander.net> wrote:
> On Thu, Dec 29, 2016 at 12:35 AM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Wed, Dec 28, 2016 at 9:31 PM, Magnus Hagander <magnus@hagander.net>
>> wrote:
>> >> - I have switched the directory method to use a file pointer instead
>> >> of a file descriptor as gzwrite returns int as the number of
>> >> uncompressed bytes written.
>> >
>> > I don't really follow that reasoning :) Why does the directory method
>> > have
>> > to change to use a filepointer because of that?
>>
>> The only reason is that write() returns size_t and fwrite returns int,
>> while gzwrite() returns int. It seems more consistent to use fwrite()
>> in this case. Or we don't bother about my nitpicking and just cast
>> stuff.
>
>
> I can at least partially see that argument, but your patch doesn't actually
> use fwrite(), it uses write() with fileno()...

That was part of the one/two things I wanted to change before sending
a fresh patch.

OK.

 
> But also, on my platform (debian jessie), fwrite() returns size_t, and
> write() returns ssize_t. So those are apparently both different from what
> your platform does - which one did you get that one?

It looks like I misread the macos man pages previously. Thay actually
list ssize_t. I find a bit surprising the way gzwrite is designed. It
uses an input an unsigned integer and returns to caller a signed
integer, so this will never work with uncompressed buffers of sizes
higher than 2GB. There's little point to worry about that in
pg_receivexlog though, so let's just cast to ssize_t.

Attached is a simplified new version, I have kept the file descriptor
as originally done. Note that tests are actually difficult to work
out, there is no way to run in batch pg_receivexlog..

A few further notes:

You are using the filemode to gzopen and the mode_compression variable to set the compression level. The pre-existing code in pg_basebackup uses gzsetparams(). Is there a particular reason you didn't do it the same way?

Small comment:
-   if (pad_to_size)
+   if (pad_to_size && dir_data->compression == 0)
    {
        /* Always pre-pad on regular files */


That "always" is not true anymore. Commit-time cleanup can be done of that.

The rest of this looks good to me, but please comment on the gzopen part before we proceed to commit :)
 
--

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Magnus Hagander
Дата:
Сообщение: Re: [HACKERS] Questionable tag usage
Следующее
От: Robert Haas
Дата:
Сообщение: Re: [HACKERS] UNDO and in-place update