rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Code review for pgstat_get_crashed_backend_activity patch. 

Avoid possibly dumping core when pgstat_track_activity_query_size has a
less-than-default value; avoid uselessly searching for the query string
of a successfully-exited backend; don't bother putting out an ERRDETAIL if
we don't have a query to show; some other minor stylistic improvements.
This commit is contained in:
Tom Lane 2011-10-21 16:36:04 -04:00
parent 5ac5980744
commit f9c92a5a3e
5 changed files with 66 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
src/backend/postmaster/pgstat.c
View File

@ -2760,27 +2760,33 @@ pgstat_get_backend_current_activity(int pid, bool checkUser)
* pgstat_get_crashed_backend_activity() -
*
* Return a string representing the current activity of the backend with
* the specified PID. Like the function above, but reads shared memory with
* the expectation that it may be corrupt. Returns either a pointer to a
* constant string, or writes into the 'buffer' argument and returns it.
* the specified PID. Like the function above, but reads shared memory with
* the expectation that it may be corrupt. On success, copy the string
* into the "buffer" argument and return that pointer. On failure,
* return NULL.
*
* This function is only intended to be used by postmaster to report the
* query that crashed the backend. In particular, no attempt is made to
* This function is only intended to be used by the postmaster to report the
* query that crashed a backend. In particular, no attempt is made to
* follow the correct concurrency protocol when accessing the
* BackendStatusArray. But that's OK, in the worst case we'll return a
* corrupted message. We also must take care not to trip on ereport(ERROR).
*
* Note: return strings for special cases match pg_stat_get_backend_activity.
* BackendStatusArray. But that's OK, in the worst case we'll return a
* corrupted message. We also must take care not to trip on ereport(ERROR).
* ----------
*/
const char *
pgstat_get_crashed_backend_activity(int pid, char *buffer,
int len)
pgstat_get_crashed_backend_activity(int pid, char *buffer, int buflen)
{
volatile PgBackendStatus *beentry;
int i;
beentry = BackendStatusArray;
/*
* We probably shouldn't get here before shared memory has been set up,
* but be safe.
*/
if (beentry == NULL || BackendActivityBuffer == NULL)
return NULL;
for (i = 1; i <= MaxBackends; i++)
{
if (beentry->st_procpid == pid)
@ -2790,26 +2796,29 @@ pgstat_get_crashed_backend_activity(int pid, char *buffer,
const char *activity_last;
/*
* We can't access activity pointer before we verify that it
* falls into BackendActivityBuffer. To make sure that the entire
* string including its ending is contained within the buffer,
* we subtract one activity length from it.
* We mustn't access activity string before we verify that it
* falls within the BackendActivityBuffer. To make sure that the
* entire string including its ending is contained within the
* buffer, subtract one activity length from the buffer size.
*/
activity_last = BackendActivityBuffer + BackendActivityBufferSize
- pgstat_track_activity_query_size;
- pgstat_track_activity_query_size;
if (activity < BackendActivityBuffer ||
activity > activity_last)
return "<command string corrupt>";
return NULL;
if (*(activity) == '\0')
return "<command string empty>";
/* If no string available, no point in a report */
if (activity[0] == '\0')
return NULL;
/*
* Copy only ASCII-safe characters so we don't run into encoding
* problems when reporting the message.
* problems when reporting the message; and be sure not to run
* off the end of memory.
*/
ascii_safe_strncpy(buffer, activity, len);
ascii_safe_strlcpy(buffer, activity,
Min(buflen, pgstat_track_activity_query_size));
return buffer;
}
@ -2818,9 +2827,10 @@ pgstat_get_crashed_backend_activity(int pid, char *buffer,
}
/* PID not found */
return "<backend information not available>";
return NULL;
}
/* ------------------------------------------------------------
* Local support functions follow
* ------------------------------------------------------------

25
src/backend/postmaster/postmaster.c
View File

@ -2777,12 +2777,17 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
static void
LogChildExit(int lev, const char *procname, int pid, int exitstatus)
{
char activity_buffer[1024]; /* default track_activity_query_size */
const char *activity;
/*
* size of activity_buffer is arbitrary, but set equal to default
* track_activity_query_size
*/
char activity_buffer[1024];
const char *activity = NULL;
activity = pgstat_get_crashed_backend_activity(pid,
activity_buffer,
sizeof(activity_buffer));
if (!EXIT_STATUS_0(exitstatus))
activity = pgstat_get_crashed_backend_activity(pid,
activity_buffer,
sizeof(activity_buffer));
if (WIFEXITED(exitstatus))
ereport(lev,
@ -2792,7 +2797,7 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
"server process" */
(errmsg("%s (PID %d) exited with exit code %d",
procname, pid, WEXITSTATUS(exitstatus)),
errdetail("Running query: %s", activity)));
activity ? errdetail("Failed process was running: %s", activity) : 0));
else if (WIFSIGNALED(exitstatus))
#if defined(WIN32)
ereport(lev,
@ -2803,7 +2808,7 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
(errmsg("%s (PID %d) was terminated by exception 0x%X",
procname, pid, WTERMSIG(exitstatus)),
errhint("See C include file \"ntstatus.h\" for a description of the hexadecimal value."),
errdetail("Running query: %s", activity)));
activity ? errdetail("Failed process was running: %s", activity) : 0));
#elif defined(HAVE_DECL_SYS_SIGLIST) && HAVE_DECL_SYS_SIGLIST
ereport(lev,
@ -2814,7 +2819,7 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
procname, pid, WTERMSIG(exitstatus),
WTERMSIG(exitstatus) < NSIG ?
sys_siglist[WTERMSIG(exitstatus)] : "(unknown)"),
errdetail("Running query: %s", activity)));
activity ? errdetail("Failed process was running: %s", activity) : 0));
#else
ereport(lev,
@ -2823,7 +2828,7 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
"server process" */
(errmsg("%s (PID %d) was terminated by signal %d",
procname, pid, WTERMSIG(exitstatus)),
errdetail("Running query: %s", activity)));
activity ? errdetail("Failed process was running: %s", activity) : 0));
#endif
else
ereport(lev,
@ -2833,7 +2838,7 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
"server process" */
(errmsg("%s (PID %d) exited with unrecognized status %d",
procname, pid, exitstatus),
errdetail("Running query: %s", activity)));
activity ? errdetail("Failed process was running: %s", activity) : 0));
}
/*

29
src/backend/utils/adt/ascii.c
View File

@ -160,35 +160,38 @@ to_ascii_default(PG_FUNCTION_ARGS)
}
/* ----------
* "Escape" a string in unknown encoding to a valid ASCII string.
* Replace non-ASCII bytes with '?'
* This must not trigger ereport(ERROR), as it is called from postmaster.
* Copy a string in an arbitrary backend-safe encoding, converting it to a
* valid ASCII string by replacing non-ASCII bytes with '?'. Otherwise the
* behavior is identical to strlcpy(), except that we don't bother with a
* return value.
*
* Unlike C strncpy(), the result is always terminated with exactly one null
* byte.
* This must not trigger ereport(ERROR), as it is called in postmaster.
* ----------
*/
void
ascii_safe_strncpy(char *dest, const char *src, int len)
ascii_safe_strlcpy(char *dest, const char *src, size_t destsiz)
{
int i;
if (destsiz == 0) /* corner case: no room for trailing nul */
return;
for (i = 0; i < (len - 1); i++)
while (--destsiz > 0)
{
unsigned char ch = src[i]; /* use unsigned char here to avoid compiler warning */
/* use unsigned char here to avoid compiler warning */
unsigned char ch = *src++;
if (ch == '\0')
break;
/* Keep printable ASCII characters */
if (32 <= ch && ch <= 127)
dest[i] = ch;
*dest = ch;
/* White-space is also OK */
else if (ch == '\n' || ch == '\r' || ch == '\t')
dest[i] = ch;
*dest = ch;
/* Everything else is replaced with '?' */
else
dest[i] = '?';
*dest = '?';
dest++;
}
dest[i] = '\0';
*dest = '\0';
}

2
src/include/pgstat.h
View File

@ -721,7 +721,7 @@ extern void pgstat_report_xact_timestamp(TimestampTz tstamp);
extern void pgstat_report_waiting(bool waiting);
extern const char *pgstat_get_backend_current_activity(int pid, bool checkUser);
extern const char *pgstat_get_crashed_backend_activity(int pid, char *buffer,
int len);
int buflen);
extern PgStat_TableStatus *find_tabstat_entry(Oid rel_id);
extern PgStat_BackendFunctionEntry *find_funcstat_entry(Oid func_id);

3
src/include/utils/ascii.h
View File

@ -16,6 +16,7 @@
extern Datum to_ascii_encname(PG_FUNCTION_ARGS);
extern Datum to_ascii_enc(PG_FUNCTION_ARGS);
extern Datum to_ascii_default(PG_FUNCTION_ARGS);
extern void ascii_safe_strncpy(char *dest, const char *src, int len);
extern void ascii_safe_strlcpy(char *dest, const char *src, size_t destsiz);
#endif /* _ASCII_H_ */