On Tue, Jan 1, 2013 at 7:13 PM, Boszormenyi Zoltan <zb@cybertec.at> wrote:
2013-01-01 18:25 keltezéssel, Magnus Hagander írta:
On Fri, Nov 30, 2012 at 10:13 AM, Boszormenyi Zoltan <zb@cybertec.at> wrote:
Hi,
now that PQconninfo() was committed, I rebased the subsequent patches. Actual code change was only in the last patch, to conform to the committed PQconninfo() API.
I've applied a modified version of the "tar unification" patch to master. It didn't apply cleanly so I ended up redoing a number of the things manually, but the end result is fairly similar. I don't think it'll cause anything but really trivial merge conflicts against the 04 patch, but I didn't actually check that (e.g. it's tarCreateHeader() not _tarCreateHeader() now).
Thanks.
I took at look at the basebackup patch as well. Easier to get now that it's commented :) There's quite a lot of code in there whereby it tries to remove recovery.conf from the tar stream if it's already in there. That seems like an ugly way to do it. I see two other options, that I think both are better. If we do need it to be removed, we should probably pass that as a parameter up to the walsender, and make sure the file isn't included in the first place. But we can also leave it in there - the tar format is perfectly happy to have multiple copies of the same file in the archive - it'll just use whichever copy comes last.
IIRC, Fujii Masao's wish was to remove the recovery.conf from the tar stream. I know that the tar format is perfectly happy with two files with the same path name but his reasoning was that GUIs (like WinRar) may get confused by two such files and cannot decide which one to extract. I didn't actually test this but it is a reasonable suspicion.
Hmm. Somehow, I missed that part of the discussion. Sorry, didn't realize it had been mentioned already.
Passing a parameter to the walsender may be a better solution. It's a bad idea only because it wasn't mine. ;-)
The only problem with this idea is that currently there's nothing that stops pg_basebackup to be a generic and backwards-compatible tool. It simply receives a TAR stream via plain PQgetCopyData() and optionally extracts it. The client-side solution to skip the recovery.conf file keeps this backward compatible feature. I tested this and pg_basebackup from 9.3dev happily backs up a 9.2.2 database...
I thought commit add6c3179a4d4fa3e62dd3e86a00f23303336bac at leasdt broke that? In particular, did you test where any of those values were different between client and server? Or maybe just didn't test in -x mode?
I actually thought there was something else that broke the compatibility, but I can't seem to find it so perhaps that was something that was never committed.
