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

Re: Small Bug in pgstat display during recovery conflict resolution

From: Andres Freund <andres(at)anarazel(dot)de>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Small Bug in pgstat display during recovery conflict resolution
Date: 2010-02-07 20:47:39
Message-ID: 201002072147.41688.andres@anarazel.de (view raw)
Hi Simon, Hi all,


if (!logged && (wait_s > 0 || wait_us > 500000))
{
        const char *oldactivitymsg;
        int                     len;

        oldactivitymsg = get_ps_display(&len);
        snprintf(waitactivitymsg, sizeof(waitactivitymsg),
                         "waiting for max_standby_delay (%u s)",
                         MaxStandbyDelay);
        set_ps_display(waitactivitymsg, false);
        if (len > 100)
                len = 100;
        memcpy(waitactivitymsg, oldactivitymsg, len);

        pgstat_report_waiting(true);

        logged = true;
}
..
if (logged)
{
        set_ps_display(waitactivitymsg, false);
        pgstat_report_waiting(false);
}

That doesnt work because get_ps_display returns the internal buffer. This 
leads to the situation that after conflict resolution the 
"waiting for max_standby_delay ..."
message is displayed until the next segment starts where its replaced
again by the 
"... recovering ..." line.

Additionally the old code may print unintialized memory if get_ps_display
 returns a string without a \0 terminator.

The attached patch fixes that.

Andres
Attachment: 0001-The-stat-reporting-in-ResolveRecoveryConflictWithVir.patch
Description: text/x-patch (2.4 KB)
From: Simon Riggs <simon(at)2ndQuadrant(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Small Bug in pgstat display during recovery conflict resolution
Date: 2010-02-13 16:30:36
Message-ID: 1266078636.7341.7795.camel@ebony (view raw)
Committed, thanks.

On Sun, 2010-02-07 at 21:47 +0100, Andres Freund wrote:
> Hi Simon, Hi all,
> 
> 
> if (!logged && (wait_s > 0 || wait_us > 500000))
> {
>         const char *oldactivitymsg;
>         int                     len;
> 
>         oldactivitymsg = get_ps_display(&len);
>         snprintf(waitactivitymsg, sizeof(waitactivitymsg),
>                          "waiting for max_standby_delay (%u s)",
>                          MaxStandbyDelay);
>         set_ps_display(waitactivitymsg, false);
>         if (len > 100)
>                 len = 100;
>         memcpy(waitactivitymsg, oldactivitymsg, len);
> 
>         pgstat_report_waiting(true);
> 
>         logged = true;
> }
> ..
> if (logged)
> {
>         set_ps_display(waitactivitymsg, false);
>         pgstat_report_waiting(false);
> }
> 
> That doesnt work because get_ps_display returns the internal buffer. This 
> leads to the situation that after conflict resolution the 
> "waiting for max_standby_delay ..."
> message is displayed until the next segment starts where its replaced
> again by the 
> "... recovering ..." line.
> 
> Additionally the old code may print unintialized memory if get_ps_display
>  returns a string without a \0 terminator.
> 
> The attached patch fixes that.
> 
> Andres
-- 
 Simon Riggs           www.2ndQuadrant.com



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