Horton

Thread

  1. pg_ctl emits strange warning message

    Fujii Masao <masao.fujii@gmail.com> — 2010-09-13T10:08:44Z

    Hi,

http://archives.postgresql.org/pgsql-hackers/2010-05/msg00921.php
>>> (2)
>>> pg_ctl -ms stop emits the following warning whenever there is the
>>> backup_label file in $PGDATA.
>>>
>>>      WARNING: online backup mode is active
>>>      Shutdown will not complete until pg_stop_backup() is called.
>>>
>>> This warning doesn't fit in with the shutdown during recovery case.
>>> Since smart shutdown might be requested by other than pg_ctl, the
>>> warning should be emitted in server side rather than client, I think.
>>> How about moving the warning to the server side?
>>
>> Hmm, I'm not sure whether that's a good idea or not.  Perhaps we
>> should discuss for 9.1?
>
> Okay, this is not a critical problem.

Let's revisit this issue. The problem here is that pg_ctl might emit
the following warning messages when it shuts down the standby server.
This is very strange thing since online backup mode is never active
in the standby.

    WARNING: online backup mode is active
    Shutdown will not complete until pg_stop_backup() is called.

Another problem is that the above useful messages are not emitted
when shutdown is requested by other than pg_ctl.

The attached patch moves the warning from pg_ctl to the server side
and makes postmaster emit it only when online backup mode is really
active. This can urge users to call pg_stop_backup to complete smart
shutdown whenever it's requested during online backup. I added the
patch into the next CF.

Comments?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

  2. Re: pg_ctl emits strange warning message

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2010-09-13T13:01:16Z

    On 13/09/10 13:08, Fujii Masao wrote:
> Hi,
>
> http://archives.postgresql.org/pgsql-hackers/2010-05/msg00921.php
>>>> (2)
>>>> pg_ctl -ms stop emits the following warning whenever there is the
>>>> backup_label file in $PGDATA.
>>>>
>>>>       WARNING: online backup mode is active
>>>>       Shutdown will not complete until pg_stop_backup() is called.
>>>>
>>>> This warning doesn't fit in with the shutdown during recovery case.
>>>> Since smart shutdown might be requested by other than pg_ctl, the
>>>> warning should be emitted in server side rather than client, I think.
>>>> How about moving the warning to the server side?
>>>
>>> Hmm, I'm not sure whether that's a good idea or not.  Perhaps we
>>> should discuss for 9.1?
>>
>> Okay, this is not a critical problem.
>
> Let's revisit this issue. The problem here is that pg_ctl might emit
> the following warning messages when it shuts down the standby server.
> This is very strange thing since online backup mode is never active
> in the standby.
>
>      WARNING: online backup mode is active
>      Shutdown will not complete until pg_stop_backup() is called.
>
> Another problem is that the above useful messages are not emitted
> when shutdown is requested by other than pg_ctl.
>
> The attached patch moves the warning from pg_ctl to the server side
> and makes postmaster emit it only when online backup mode is really
> active. This can urge users to call pg_stop_backup to complete smart
> shutdown whenever it's requested during online backup. I added the
> patch into the next CF.

There's this comment by Robert Haas:

http://archives.postgresql.org/pgsql-hackers/2010-05/msg01433.php

Like Robert, I'm also a bit worried that this might make the message 
less prominent. When you run "pg_ctl stop" manually, it's good to get 
the message directly in the terminal.

Another line of attack is to remove the backup_label file earlier during 
recovery. We could remove it as soon as we update pg_control file with 
the same information, ie. the backup start location. And by remove I 
mean rename to backup_label.old, to avoid destroying forensic 
information ;-).

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

  3. Re: pg_ctl emits strange warning message

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2010-09-13T15:10:23Z

    On 13/09/10 16:01, Heikki Linnakangas wrote:
> On 13/09/10 13:08, Fujii Masao wrote:
>> Hi,
>>
>> http://archives.postgresql.org/pgsql-hackers/2010-05/msg00921.php
>>>>> (2)
>>>>> pg_ctl -ms stop emits the following warning whenever there is the
>>>>> backup_label file in $PGDATA.
>>>>>
>>>>> WARNING: online backup mode is active
>>>>> Shutdown will not complete until pg_stop_backup() is called.
>>>>>
>>>>> This warning doesn't fit in with the shutdown during recovery case.
>>>>> Since smart shutdown might be requested by other than pg_ctl, the
>>>>> warning should be emitted in server side rather than client, I think.
>>>>> How about moving the warning to the server side?
>>>>
>>>> Hmm, I'm not sure whether that's a good idea or not. Perhaps we
>>>> should discuss for 9.1?
>>>
>>> Okay, this is not a critical problem.
>>
>> Let's revisit this issue. The problem here is that pg_ctl might emit
>> the following warning messages when it shuts down the standby server.
>> This is very strange thing since online backup mode is never active
>> in the standby.
>>
>> WARNING: online backup mode is active
>> Shutdown will not complete until pg_stop_backup() is called.
>>
>> Another problem is that the above useful messages are not emitted
>> when shutdown is requested by other than pg_ctl.
>>
>> The attached patch moves the warning from pg_ctl to the server side
>> and makes postmaster emit it only when online backup mode is really
>> active. This can urge users to call pg_stop_backup to complete smart
>> shutdown whenever it's requested during online backup. I added the
>> patch into the next CF.
>
> There's this comment by Robert Haas:
>
> http://archives.postgresql.org/pgsql-hackers/2010-05/msg01433.php
>
> Like Robert, I'm also a bit worried that this might make the message
> less prominent. When you run "pg_ctl stop" manually, it's good to get
> the message directly in the terminal.
>
> Another line of attack is to remove the backup_label file earlier during
> recovery. We could remove it as soon as we update pg_control file with
> the same information, ie. the backup start location. And by remove I
> mean rename to backup_label.old, to avoid destroying forensic
> information ;-).

Yet another idea: Check in pg_ctl if recovery.conf is also present. If 
it is, assume we're in recovery and don't print that warning. That would 
be dead simple. I would even consider backpatching it to 9.0, this can 
be regarded as a bug, albeit a minor one.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

  4. Re: pg_ctl emits strange warning message

    Tom Lane <tgl@sss.pgh.pa.us> — 2010-09-13T15:19:40Z

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> Yet another idea: Check in pg_ctl if recovery.conf is also present. If 
> it is, assume we're in recovery and don't print that warning. That would 
> be dead simple.

+1.  This sounds like a much more appropriate approach.

			regards, tom lane

  5. Re: pg_ctl emits strange warning message

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2010-09-13T15:35:19Z

    On 13/09/10 18:19, Tom Lane wrote:
> Heikki Linnakangas<heikki.linnakangas@enterprisedb.com>  writes:
>> Yet another idea: Check in pg_ctl if recovery.conf is also present. If
>> it is, assume we're in recovery and don't print that warning. That would
>> be dead simple.
>
> +1.  This sounds like a much more appropriate approach.

Hmm, looking at this more closely, I'm a bit confused. We already rename 
away backup_label at the beginning of recovery. Under what circumstances 
do we have a problem? This starts to seem like a complete non-issue to me.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

  6. Re: pg_ctl emits strange warning message

    Fujii Masao <masao.fujii@gmail.com> — 2010-09-14T00:39:13Z

    On Tue, Sep 14, 2010 at 12:35 AM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> Hmm, looking at this more closely, I'm a bit confused. We already rename
> away backup_label at the beginning of recovery. Under what circumstances do
> we have a problem? This starts to seem like a complete non-issue to me.

A checkpoint record is read before backup_label file is renamed, so
the problem happens if shutdown is requested while the standby server
is waiting for the WAL containing a checkpoint record to arrive from
the master. This happened some times in my box when testing HS/SR.

To avoid the problem, we should rename backup_label before reading
any WAL record?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

  7. Re: pg_ctl emits strange warning message

    Fujii Masao <masao.fujii@gmail.com> — 2010-09-14T00:55:21Z

    On Tue, Sep 14, 2010 at 12:10 AM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> Yet another idea: Check in pg_ctl if recovery.conf is also present. If it
> is, assume we're in recovery and don't print that warning. That would be
> dead simple. I would even consider backpatching it to 9.0, this can be
> regarded as a bug, albeit a minor one.

Yeah, it's very simple. Attached patch does that.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

  8. Re: pg_ctl emits strange warning message

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2010-09-14T07:46:22Z

    On 14/09/10 03:39, Fujii Masao wrote:
> On Tue, Sep 14, 2010 at 12:35 AM, Heikki Linnakangas
> <heikki.linnakangas@enterprisedb.com>  wrote:
>> Hmm, looking at this more closely, I'm a bit confused. We already rename
>> away backup_label at the beginning of recovery. Under what circumstances do
>> we have a problem? This starts to seem like a complete non-issue to me.
>
> A checkpoint record is read before backup_label file is renamed, so
> the problem happens if shutdown is requested while the standby server
> is waiting for the WAL containing a checkpoint record to arrive from
> the master. This happened some times in my box when testing HS/SR.

Ok, I see. Shouldn't be very common, but might happen if you have a set 
restore_command incorrectly for example.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

  9. Re: pg_ctl emits strange warning message

    Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> — 2010-09-14T08:06:23Z

    On 14/09/10 03:55, Fujii Masao wrote:
> On Tue, Sep 14, 2010 at 12:10 AM, Heikki Linnakangas
> <heikki.linnakangas@enterprisedb.com>  wrote:
>> Yet another idea: Check in pg_ctl if recovery.conf is also present. If it
>> is, assume we're in recovery and don't print that warning. That would be
>> dead simple. I would even consider backpatching it to 9.0, this can be
>> regarded as a bug, albeit a minor one.
>
> Yeah, it's very simple. Attached patch does that.

Committed with some extra comments. I also backpatched this to 9.0.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com

  10. Re: pg_ctl emits strange warning message

    Fujii Masao <masao.fujii@gmail.com> — 2010-09-14T08:35:23Z

    On Tue, Sep 14, 2010 at 5:06 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> Committed with some extra comments. I also backpatched this to 9.0.

Thanks! I marked the patch as committed on CF.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

  11. Re: pg_ctl emits strange warning message

    David Fetter <david@fetter.org> — 2010-09-14T14:57:14Z

    On Tue, Sep 14, 2010 at 05:35:23PM +0900, Fujii Masao wrote:
> On Tue, Sep 14, 2010 at 5:06 PM, Heikki Linnakangas
> <heikki.linnakangas@enterprisedb.com> wrote:
> > Committed with some extra comments. I also backpatched this to 9.0.
> 
> Thanks! I marked the patch as committed on CF.

Thanks!

Cheers,
David (Commitfest Cat Herd of the Month).
-- 
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate