Re: Support for pg_receivexlog --format=plain|tar

From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, David Steele <david(at)pgmasters(dot)net>
Subject: Re: Support for pg_receivexlog --format=plain|tar
Date: 2016-12-29 09:13:37
Message-ID: CABUevEx4ud6y8C8GSLZaSJj-poBh3gGCiyaziXcjhhHAT4+vYw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Dec 29, 2016 at 12:35 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com
> wrote:

> On Wed, Dec 28, 2016 at 9:31 PM, Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> > Conditional tests? It probably wouldn't hurt to have them, but that
> would be
> > something more generic (like we'd need something to actually validate it
> --
> > but it would make sense to have a test that, with compression enabled,
> would
> > verify if the uncompressed file turns out to be exactly 16Mb for
> example).
>
> Looking at if the build is compiled with libz via SQL or using
> pg_config is the way to go here. A minimum is doable.
>
> >> A couple of things to be aware of though:
> >> - gzopen, gzwrite and gzclose are used to handle the gz files. That's
> >> unconsistent with the tar method that is based on mainly deflate() and
> >> more low level routines.
> >
> > But chosen for simplicity, I assume?
>
> Yep. That's quite in-line with the current code.
>

Agreed.

> >> - 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()...

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?

(gzwrite does return int)

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Craig Ringer 2016-12-29 09:28:22 Re: [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups
Previous Message Fabien COELHO 2016-12-29 09:11:39 Re: proposal: session server side variables