Re: [PATCH] Make pg_basebackup configure and start standby [Review]

2012-10-14 22:31 keltezéssel, Boszormenyi Zoltan írta:
> 2012-10-14 22:26 keltezéssel, Boszormenyi Zoltan írta:
>> 2012-10-14 22:23 keltezéssel, Boszormenyi Zoltan írta:
>>> Hi,
>>>
>>> 2012-10-14 18:41 keltezéssel, Boszormenyi Zoltan írta:
>>>> 2012-10-14 18:02 keltezéssel, Fujii Masao írta:
>>>>> Thanks for updating the patch!
>>>>>
>>>>> On Sun, Oct 14, 2012 at 8:41 PM, Boszormenyi Zoltan <zb(at)cybertec(dot)at> wrote:
>>>>>> Backing up a standby server without -R preserves the original recovery.conf
>>>>>> of the
>>>>>> standby, it points to the standby's source server.
>>>>>>
>>>>>> Backing up a standby server with -R overwrites the original recovery.conf
>>>>>> with the new
>>>>>> one pointing to the standby instead of the standby's source server. Without
>>>>>> -Ft, it is
>>>>>> obvious. With -Ft, there are two recovery.conf files in the tar file and
>>>>>> upon extracting it,
>>>>>> the last written one (the one generated via -R) overwrites the original.
>>>>> The tar file is always extracted such way in all platform which PostgreSQL
>>>>> supports? I'm just concerned about that some tool in some platform might
>>>>> prefer the original recovery.conf when extracting tar file. If the spec of tar
>>>>> format specifies such behavior (i.e., the last written file of the same name
>>>>> is always preferred), it's OK.
>>>>
>>>> Since tar is a sequential archive format, I think this is the behaviour of
>>>> every tar extractor. But I will look at adding code to skip the original
>>>> recovery.conf if it exists in the tar file.
>>>>
>>>>> I found the bug that recovery.conf is included in the tar file of the tablespace
>>>>> instead of base.tar, when there are tablespaces in the server.
>>>>
>>>> You are right, I am looking into this. But I don't know how it got there,
>>>> I check for (rownum == 0 && writerecoveryconf) and rownum == 0
>>>> supposedly means that it's the base.tar. Looking again.
>>>
>>> I made a mistake in the previous check, rownum is not reliable.
>>> The tablespaces are sent first and base backup as the last.
>>> Now recovery.conf is written into base.tar.
>>>
>>>>> Maybe this is nitpicky problem,,,, but...
>>>>> If port number is not explicitly specified in pg_basebackup, the port
>>>>> number is not
>>>>> included to primary_conninfo in recovery.conf which is created during
>>>>> the backup.
>>>>> That is, the standby server using such recovery.conf tries to connect
>>>>> to the default
>>>>> port number because the port number is not supplied in primary_conninfo. This
>>>>> assumes that the default port number is the same between the master and standby.
>>>>> But this is not true. The default port number can be changed in --with-pgport
>>>>> configure option, so the default port number might be different
>>>>> between the master
>>>>> and standby. To avoid this uncertainty, pg_basebackup -R should always include
>>>>> the port number in primary_conninfo?
>>>>
>>>> I think you are right. But, I wouldn't restrict it only to the port setting.
>>>> Any of the values that are set and equal to the compiled-in default,
>>>> it should be written into recovery.conf.
>>>
>>> Now all values that are set (even those being equal to the compiled-in default)
>>> are put into recovery.conf.
>>>
>>>>> When the password is required to connect to the server, pg_basebackup -R
>>>>> always writes the password setting into primary_conninfo in recovery.conf.
>>>>> But if the password is supplied from .pgpass, ISTM that the password setting
>>>>> doesn't need to be written into primary_conninfo. Right?
>>>>
>>>> How can you deduce it from the PQconninfoOption structure?
>>>>
>>>> Also, if the machine you take the base backup on is different
>>>> from the one where you actually use the backup on, it can be
>>>> different not only in the --with-pgport compilation option but
>>>> in the presence of .pgpass or the PGPASSWORD envvar, too.
>>>> The administrator is there for a reason or there is no .pgpass
>>>> or PGPASSWORD at all.
>>>>
>>>>> + The password written into recovery.conf is not escaped even if special
>>>>> + characters appear in it. The administrator must review recovery.conf
>>>>> + to ensure proper escaping.
>>>>>
>>>>> Is it difficult to make pg_basebackup escape the special characters in the
>>>>> password? It's better if we can remove this restriction.
>>>>
>>>> It's not difficult. What other characters need to be escaped besides single quotes?
>>>
>>> All written values are escaped.
>>>
>>> Other changes: the recovery.conf in base.tar is correctly skipped if it exists
>>> and -R is given. The new recovery.conf is written with padding to round up to
>>> 512, the TAR chunk size.
>>
>> Also, the check for conflict between -R and -x/-X is now removed.

The documentation for option -R has changed to reflect this and
there is no different error code 2 now: it would make the behaviour
inconsistent between -Fp and -Ft.

>>> The PQconninfo patch is also attached but didn't change since the last mail.

Best regards,
Zoltán Böszörményi

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/