From: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Dilip kumar <dilip(dot)kumar(at)huawei(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Magnus Hagander <magnus(at)hagander(dot)net>, Noah Misch <noah(at)leadboat(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pg_basebackup vs. Windows and tablespaces |
Date: | 2015-05-12 00:20:16 |
Message-ID: | 55514740.3040205@dunslane.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 05/11/2015 02:02 AM, Amit Kapila wrote:
> On Sun, May 10, 2015 at 6:01 AM, Andrew Dunstan <andrew(at)dunslane(dot)net
> <mailto:andrew(at)dunslane(dot)net>> wrote:
> >
> >
> >
> > This generally looks good, but I have a couple of questions before I
> commit it.
> >
> > First, why is the new option for the BASE_BACKUP command of the
> Streaming Replication protcol "TAR"? It seems rather misleading.
> Shouldn't it be something like "TABLESPACEMAP"?
> >
>
> The reason to keep new option's name as TAR was that tablespace_map
> was generated for that format type, but I agree with you that something
> like "TABLESPACEMAP" suits better, so I have changed it to
> "TABLESPACE_MAP". Putting '_' in name makes it somewhat consistent
> with other names and filename it generates with this new option.
>
>
> > Second, these lines in xlog.c seem wrong:
> >
> > else if ((ch == '\n' || ch == '\r') && prev_ch == '\\')
> > str[i-1] = '\n';
> >
> > It looks to me like we should be putting ch in the string, not
> arbitrarily transforming \r into \n.
> >
>
> You are right, I have changed it as per your suggestion.
>
>
OK, I have cleaned this up a bit - I had already started so I didn't
take your latest patch but instead applied relevant changes to my
changeset. Here is my latest version.
In testing I notice that now "pg_baseback -F t" leaves it completely up
to the user on all platforms to create the relevant links in pg_tblspc/.
It includes the tablespace_map file in base.tar, but that's really just
informational. I think we need to add something to the pg_basebackup
docs about that, at the very least (and it will also need to be a
release note item.)
cheers
andrew
Attachment | Content-Type | Size |
---|---|---|
backup_tablespace_fix-ad.patch | text/x-patch | 36.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | David E. Wheeler | 2015-05-12 00:24:20 | Re: Fixing busted citext function declarations |
Previous Message | Tom Lane | 2015-05-12 00:01:01 | Re: Fixing busted citext function declarations |