Small Bug in pgstat display during recovery conflict resolution

Started by Andres Freundabout 16 years ago2 messageshackers
Jump to latest
#1Andres Freund
andres@anarazel.de

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

Attachments:

0001-The-stat-reporting-in-ResolveRecoveryConflictWithVir.patchtext/x-patch; charset=ISO-8859-1; name=0001-The-stat-reporting-in-ResolveRecoveryConflictWithVir.patchDownload+14-17
#2Simon Riggs
simon@2ndQuadrant.com
In reply to: Andres Freund (#1)
Re: Small Bug in pgstat display during recovery conflict resolution

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