Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitf9c92a5

Browse files
committed
Code review for pgstat_get_crashed_backend_activity patch.
Avoid possibly dumping core when pgstat_track_activity_query_size has aless-than-default value; avoid uselessly searching for the query stringof a successfully-exited backend; don't bother putting out an ERRDETAIL ifwe don't have a query to show; some other minor stylistic improvements.
1 parent5ac5980 commitf9c92a5

File tree

5 files changed

+66
-47
lines changed

5 files changed

+66
-47
lines changed

‎src/backend/postmaster/pgstat.c‎

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2760,27 +2760,33 @@ pgstat_get_backend_current_activity(int pid, bool checkUser)
27602760
* pgstat_get_crashed_backend_activity() -
27612761
*
27622762
*Return a string representing the current activity of the backend with
2763-
*the specified PID. Like the function above, but reads shared memory with
2764-
*the expectation that it may be corrupt. Returns either a pointer to a
2765-
*constant string, or writes into the 'buffer' argument and returns it.
2763+
*the specified PID. Like the function above, but reads shared memory with
2764+
*the expectation that it may be corrupt. On success, copy the string
2765+
*into the "buffer" argument and return that pointer. On failure,
2766+
*return NULL.
27662767
*
2767-
*This function is only intended to be used by postmaster to report the
2768-
*query that crashedthe backend. In particular, no attempt is made to
2768+
*This function is only intended to be used bythepostmaster to report the
2769+
*query that crasheda backend. In particular, no attempt is made to
27692770
*follow the correct concurrency protocol when accessing the
2770-
*BackendStatusArray. But that's OK, in the worst case we'll return a
2771-
*corrupted message. We also must take care not to trip on ereport(ERROR).
2772-
*
2773-
*Note: return strings for special cases match pg_stat_get_backend_activity.
2771+
*BackendStatusArray. But that's OK, in the worst case we'll return a
2772+
*corrupted message. We also must take care not to trip on ereport(ERROR).
27742773
* ----------
27752774
*/
27762775
constchar*
2777-
pgstat_get_crashed_backend_activity(intpid,char*buffer,
2778-
intlen)
2776+
pgstat_get_crashed_backend_activity(intpid,char*buffer,intbuflen)
27792777
{
27802778
volatilePgBackendStatus*beentry;
27812779
inti;
27822780

27832781
beentry=BackendStatusArray;
2782+
2783+
/*
2784+
* We probably shouldn't get here before shared memory has been set up,
2785+
* but be safe.
2786+
*/
2787+
if (beentry==NULL||BackendActivityBuffer==NULL)
2788+
returnNULL;
2789+
27842790
for (i=1;i <=MaxBackends;i++)
27852791
{
27862792
if (beentry->st_procpid==pid)
@@ -2790,26 +2796,29 @@ pgstat_get_crashed_backend_activity(int pid, char *buffer,
27902796
constchar*activity_last;
27912797

27922798
/*
2793-
* Wecan't access activitypointer before we verify that it
2794-
* fallsintoBackendActivityBuffer. To make sure that the entire
2795-
* string including its ending is contained within the buffer,
2796-
*we subtract one activity length fromit.
2799+
* Wemustn't access activitystring before we verify that it
2800+
* fallswithin theBackendActivityBuffer. To make sure that the
2801+
*entirestring including its ending is contained within the
2802+
*buffer, subtract one activity length fromthe buffer size.
27972803
*/
27982804
activity_last=BackendActivityBuffer+BackendActivityBufferSize
2799-
-pgstat_track_activity_query_size;
2805+
-pgstat_track_activity_query_size;
28002806

28012807
if (activity<BackendActivityBuffer||
28022808
activity>activity_last)
2803-
return"<command string corrupt>";
2809+
returnNULL;
28042810

2805-
if (*(activity)=='\0')
2806-
return"<command string empty>";
2811+
/* If no string available, no point in a report */
2812+
if (activity[0]=='\0')
2813+
returnNULL;
28072814

28082815
/*
28092816
* Copy only ASCII-safe characters so we don't run into encoding
2810-
* problems when reporting the message.
2817+
* problems when reporting the message; and be sure not to run
2818+
* off the end of memory.
28112819
*/
2812-
ascii_safe_strncpy(buffer,activity,len);
2820+
ascii_safe_strlcpy(buffer,activity,
2821+
Min(buflen,pgstat_track_activity_query_size));
28132822

28142823
returnbuffer;
28152824
}
@@ -2818,9 +2827,10 @@ pgstat_get_crashed_backend_activity(int pid, char *buffer,
28182827
}
28192828

28202829
/* PID not found */
2821-
return"<backend information not available>";
2830+
returnNULL;
28222831
}
28232832

2833+
28242834
/* ------------------------------------------------------------
28252835
* Local support functions follow
28262836
* ------------------------------------------------------------

‎src/backend/postmaster/postmaster.c‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2777,12 +2777,17 @@ HandleChildCrash(int pid, int exitstatus, const char *procname)
27772777
staticvoid
27782778
LogChildExit(intlev,constchar*procname,intpid,intexitstatus)
27792779
{
2780-
charactivity_buffer[1024];/* default track_activity_query_size */
2781-
constchar*activity;
2780+
/*
2781+
* size of activity_buffer is arbitrary, but set equal to default
2782+
* track_activity_query_size
2783+
*/
2784+
charactivity_buffer[1024];
2785+
constchar*activity=NULL;
27822786

2783-
activity=pgstat_get_crashed_backend_activity(pid,
2784-
activity_buffer,
2785-
sizeof(activity_buffer));
2787+
if (!EXIT_STATUS_0(exitstatus))
2788+
activity=pgstat_get_crashed_backend_activity(pid,
2789+
activity_buffer,
2790+
sizeof(activity_buffer));
27862791

27872792
if (WIFEXITED(exitstatus))
27882793
ereport(lev,
@@ -2792,7 +2797,7 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
27922797
"server process" */
27932798
(errmsg("%s (PID %d) exited with exit code %d",
27942799
procname,pid,WEXITSTATUS(exitstatus)),
2795-
errdetail("Running query: %s",activity)));
2800+
activity ?errdetail("Failed process was running: %s",activity) :0));
27962801
elseif (WIFSIGNALED(exitstatus))
27972802
#if defined(WIN32)
27982803
ereport(lev,
@@ -2803,7 +2808,7 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
28032808
(errmsg("%s (PID %d) was terminated by exception 0x%X",
28042809
procname,pid,WTERMSIG(exitstatus)),
28052810
errhint("See C include file \"ntstatus.h\" for a description of the hexadecimal value."),
2806-
errdetail("Running query: %s",activity)));
2811+
activity ?errdetail("Failed process was running: %s",activity) :0));
28072812
#elif defined(HAVE_DECL_SYS_SIGLIST)&&HAVE_DECL_SYS_SIGLIST
28082813
ereport(lev,
28092814

@@ -2814,7 +2819,7 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
28142819
procname,pid,WTERMSIG(exitstatus),
28152820
WTERMSIG(exitstatus)<NSIG ?
28162821
sys_siglist[WTERMSIG(exitstatus)] :"(unknown)"),
2817-
errdetail("Running query: %s",activity)));
2822+
activity ?errdetail("Failed process was running: %s",activity) :0));
28182823
#else
28192824
ereport(lev,
28202825

@@ -2823,7 +2828,7 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
28232828
"server process" */
28242829
(errmsg("%s (PID %d) was terminated by signal %d",
28252830
procname,pid,WTERMSIG(exitstatus)),
2826-
errdetail("Running query: %s",activity)));
2831+
activity ?errdetail("Failed process was running: %s",activity) :0));
28272832
#endif
28282833
else
28292834
ereport(lev,
@@ -2833,7 +2838,7 @@ LogChildExit(int lev, const char *procname, int pid, int exitstatus)
28332838
"server process" */
28342839
(errmsg("%s (PID %d) exited with unrecognized status %d",
28352840
procname,pid,exitstatus),
2836-
errdetail("Running query: %s",activity)));
2841+
activity ?errdetail("Failed process was running: %s",activity) :0));
28372842
}
28382843

28392844
/*

‎src/backend/utils/adt/ascii.c‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -160,35 +160,38 @@ to_ascii_default(PG_FUNCTION_ARGS)
160160
}
161161

162162
/* ----------
163-
* "Escape" a string in unknown encoding to a valid ASCII string.
164-
* Replace non-ASCII bytes with '?'
165-
* This must not trigger ereport(ERROR), as it is called from postmaster.
163+
* Copy a string in an arbitrary backend-safe encoding, converting it to a
164+
* valid ASCII string by replacing non-ASCII bytes with '?'. Otherwise the
165+
* behavior is identical to strlcpy(), except that we don't bother with a
166+
* return value.
166167
*
167-
* Unlike C strncpy(), the result is always terminated with exactly one null
168-
* byte.
168+
* This must not trigger ereport(ERROR), as it is called in postmaster.
169169
* ----------
170170
*/
171171
void
172-
ascii_safe_strncpy(char*dest,constchar*src,intlen)
172+
ascii_safe_strlcpy(char*dest,constchar*src,size_tdestsiz)
173173
{
174-
inti;
174+
if (destsiz==0)/* corner case: no room for trailing nul */
175+
return;
175176

176-
for (i=0;i< (len-1);i++)
177+
while (--destsiz>0)
177178
{
178-
unsignedcharch=src[i];/* use unsigned char here to avoid compiler warning */
179+
/* use unsigned char here to avoid compiler warning */
180+
unsignedcharch=*src++;
179181

180182
if (ch=='\0')
181183
break;
182184
/* Keep printable ASCII characters */
183185
if (32 <=ch&&ch <=127)
184-
dest[i]=ch;
186+
*dest=ch;
185187
/* White-space is also OK */
186188
elseif (ch=='\n'||ch=='\r'||ch=='\t')
187-
dest[i]=ch;
189+
*dest=ch;
188190
/* Everything else is replaced with '?' */
189191
else
190-
dest[i]='?';
192+
*dest='?';
193+
dest++;
191194
}
192195

193-
dest[i]='\0';
196+
*dest='\0';
194197
}

‎src/include/pgstat.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -721,7 +721,7 @@ extern void pgstat_report_xact_timestamp(TimestampTz tstamp);
721721
externvoidpgstat_report_waiting(boolwaiting);
722722
externconstchar*pgstat_get_backend_current_activity(intpid,boolcheckUser);
723723
externconstchar*pgstat_get_crashed_backend_activity(intpid,char*buffer,
724-
intlen);
724+
intbuflen);
725725

726726
externPgStat_TableStatus*find_tabstat_entry(Oidrel_id);
727727
externPgStat_BackendFunctionEntry*find_funcstat_entry(Oidfunc_id);

‎src/include/utils/ascii.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
externDatumto_ascii_encname(PG_FUNCTION_ARGS);
1717
externDatumto_ascii_enc(PG_FUNCTION_ARGS);
1818
externDatumto_ascii_default(PG_FUNCTION_ARGS);
19-
externvoidascii_safe_strncpy(char*dest,constchar*src,intlen);
19+
20+
externvoidascii_safe_strlcpy(char*dest,constchar*src,size_tdestsiz);
2021

2122
#endif/* _ASCII_H_ */

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp