Skip site navigation (1) Skip section navigation (2)

Re: Include WAL in base backup

From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Stephen Frost <sfrost(at)snowman(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Include WAL in base backup
Date: 2011-01-30 19:22:02
Message-ID: AANLkTinTVnX9OekxOBGK5=0j2BAEBQu5Ju=XEbUn+_XJ@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
On Thu, Jan 27, 2011 at 05:44, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Wed, Jan 26, 2011 at 5:17 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>> We should, and the easiest way is to actually use XLogRead() since the
>> code is already there. How about the way in this patch?
>
> Thanks for the update. I reread the patch.
>
> +               MemSet(&statbuf, 0, sizeof(statbuf));
> +               statbuf.st_mode=S_IRUSR | S_IWUSR;
> +#ifndef WIN32
> +               statbuf.st_uid=getuid();
> +               statbuf.st_gid=getgid();
> +#endif
> +               statbuf.st_size=XLogSegSize;
> +               statbuf.st_mtime=time(NULL);
>
> Can you put a space around "="?

I'll run pgindent, it'll take care of that.


> Which is correct? Since we cannot start the PostgreSQL when
> getuid != geteuid, I don't think that there is really difference between
> getuid and geteuid. But other code always uses geteuid, so I think
> that geteuid should be used here instead of getuid for the sake of
> consistency.

Yeah, changed for consistency.


> +                       XLogFileName(xlogname, ThisTimeLineID, logid, logseg);
> +
> +                       sprintf(fn, "./pg_xlog/%s", xlogname);
> +                       _tarWriteHeader(fn, NULL, &statbuf);
>
> Can we use XLogFilePath? instead? Because sendFile has not been
> used.

We can now, changed.

> What I said in upthread is wrong. We should use XLByteToPrevSeg
> for endptr and check "logseg > endlogseg". Otherwise, if endptr is
> not a boundary byte, endlogid/endlogseg indicates the last
> necessary WAL file, but it's not sent.

Yeah, thanks for this - and thanks to Heikki for straightening it out for me.


> +       if (percent > 100)
> +               percent = 100;
>
> I'm not sure if this is good idea because the total received can go
> further than the estimated size though the percentage stops at 100.
> This looks more confusing than the previous behavior. Anyway,
> I think that we need to document about the combination of -P and
> -x in pg_basebackup.

I found it less confusing - but still somewhat confusing. I'll add some docs.


> In pg_basebackup.sgml
>     <term><option>--checkpoint <replaceable
> class="parameter">fast|spread</replaceable></option></term>
>
> Though this is not the problem of the patch, I found the inconsistency
> of the descriptions about options of pg_basebackup. We should
> s/--checkpoint/--checkpoint=

Agreed, changed.

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

In response to

Responses

pgsql-hackers by date

Next:From: Pavel StehuleDate: 2011-01-30 19:28:41
Subject: updated patch for foreach stmt
Previous:From: Robert HaasDate: 2011-01-30 19:12:12
Subject: Re: multiset patch review

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group