Re: pg_basebackup stream xlog to tar

On Thu, Sep 1, 2016 at 6:58 AM, Magnus Hagander <magnus@hagander.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.