Re: pg_basebackup stream xlog to tar

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_basebackup stream xlog to tar
Date: 2016-09-01 07:19:39
Message-ID: CAB7nPqRF0vH7mLSXPJxYzRhgo_OPN0wTho1uCsJdD1jktz8XfQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Sep 1, 2016 at 6:58 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> Attached patch adds support for -X stream to work with .tar and .tar.gz file
> formats.

Nice.

> If tar mode is specified, a separate pg_xlog.tar (or .tar.gz) file is
> created and the data is streamed into it. Regular mode is (should not) see
> any changes in how it works.

Could you use XLOGDIR from xlog_internal.h at least?

> The implementation creates a "walmethod" for directory and one for tar,
> which is basically a set of function pointers that we pass around as part of
> the StreamCtl structure. All calls to modify the files are sent through the
> current method, using the normal open/read/write calls as it is now for
> directories, and the more complicated method for tar and targz.

That looks pretty cool by looking at the code.

> The tar method doesn't support all things that are required for
> pg_receivexlog, but I don't think it makes any real sense to have support
> for pg_receivexlog in tar mode. But it does support all the things that
> pg_basebackup needs.

Agreed. Your patch is enough complicated.

> Some smaller pieces of functionality like unlinking files on failure and
> padding files have been moved into the walmethod because they have to be
> differently implemented (we cannot pre-pad a compressed file -- the size
> will depend on the compression ration anyway -- for example).
>
> AFAICT we never actually documented that -X stream doesn't work with tar in
> the manpage of current versions. Only in the error message. We might want to
> fix that in backbranches.

+1 for documenting that in back-branches.

> In passing this also fixes an XXX comment about not re-lseeking on the WAL
> file all the time -- the walmethod now tracks the current position in the
> file in a variable.
>
> Finally, to make this work, the pring_tar_number() function is now exported
> from port/tar.c along with the other ones already exported from there.

walmethods.c:387:9: warning: comparison of unsigned expression < 0 is
always false [-Wtautological-compare]
if (r < 0)
This patch is generating a warning for me with clang.

I have just tested this feature:
$ pg_basebackup -X stream -D data -F t
Which generates that:
$ ls data
base.tar pg_xlog.tar
However after decompressing pg_xlog.tar the segments don't have a correct size:
$ ls -l 0*
-rw------- 1 mpaquier _guest 3.9M Sep 1 16:12 000000010000000000000011

Even if that's filling them with zeros during pg_basebackup when a
segment is done, those should be 16MB to allow users to reuse them
directly.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Abhijit Menon-Sen 2016-09-01 07:22:10 Re: Proposal for changes to recovery.conf API
Previous Message Kyotaro HORIGUCHI 2016-09-01 07:15:16 Re: Comment on GatherPath.single_copy