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

Commit8bbe4cb

Browse files
committed
Improve contrib/pg_stat_statements' handling of garbage collection failure.
If we can't read the query texts file (whether because out-of-memory, orfor some other reason), give up and reset the file to empty, discarding allstored query texts, though not the statistics per se. We used to leavethings alone and hope for better luck next time, but the problem is thatthe file is only going to get bigger and even harder to slurp into memory.Better to do something that will get us out of trouble.Likewise reset the file to empty for any other failure within gc_qtexts().The previous behavior after a write error was to discard query texts butnot do anything to truncate the file, which is just weird.Also, increase the maximum supported file size from MaxAllocSize toMaxAllocHugeSize; this makes it more likely we'll be able to do a garbagecollection successfully.Also, fix recalculation of mean_query_len within entry_dealloc() to matchthe calculation in gc_qtexts(). The previous coding overlooked thepossibility of dropped texts (query_len == -1) and would underestimate themean of the remaining entries in such cases, thus possibly causing excessgarbage collection cycles.In passing, add some errdetail to the log entry that complains aboutinsufficient memory to read the query texts file, which after all wasJim Nasby's original complaint.Back-patch to 9.4 where the current handling of query texts wasintroduced.Peter Geoghegan, rather editorialized upon by me
1 parent86b1e67 commit8bbe4cb

File tree

1 file changed

+75
-18
lines changed

1 file changed

+75
-18
lines changed

‎contrib/pg_stat_statements/pg_stat_statements.c

Lines changed: 75 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ typedef struct pgssEntry
170170
pgssHashKeykey;/* hash key of entry - MUST BE FIRST */
171171
Counterscounters;/* the statistics for this query */
172172
Sizequery_offset;/* query text offset in external file */
173-
intquery_len;/* # of valid bytes in query string */
173+
intquery_len;/* # of valid bytes in query string, or -1 */
174174
intencoding;/* query text encoding */
175175
slock_tmutex;/* protects the counters only */
176176
}pgssEntry;
@@ -1705,7 +1705,8 @@ entry_cmp(const void *lhs, const void *rhs)
17051705
}
17061706

17071707
/*
1708-
* Deallocate least used entries.
1708+
* Deallocate least-used entries.
1709+
*
17091710
* Caller must hold an exclusive lock on pgss->lock.
17101711
*/
17111712
staticvoid
@@ -1716,17 +1717,27 @@ entry_dealloc(void)
17161717
pgssEntry*entry;
17171718
intnvictims;
17181719
inti;
1719-
Sizetotlen=0;
1720+
Sizetottextlen;
1721+
intnvalidtexts;
17201722

17211723
/*
17221724
* Sort entries by usage and deallocate USAGE_DEALLOC_PERCENT of them.
17231725
* While we're scanning the table, apply the decay factor to the usage
1724-
* values.
1726+
* values, and update the mean query length.
1727+
*
1728+
* Note that the mean query length is almost immediately obsolete, since
1729+
* we compute it before not after discarding the least-used entries.
1730+
* Hopefully, that doesn't affect the mean too much; it doesn't seem worth
1731+
* making two passes to get a more current result. Likewise, the new
1732+
* cur_median_usage includes the entries we're about to zap.
17251733
*/
17261734

17271735
entries=palloc(hash_get_num_entries(pgss_hash)*sizeof(pgssEntry*));
17281736

17291737
i=0;
1738+
tottextlen=0;
1739+
nvalidtexts=0;
1740+
17301741
hash_seq_init(&hash_seq,pgss_hash);
17311742
while ((entry=hash_seq_search(&hash_seq))!=NULL)
17321743
{
@@ -1736,20 +1747,27 @@ entry_dealloc(void)
17361747
entry->counters.usage *=STICKY_DECREASE_FACTOR;
17371748
else
17381749
entry->counters.usage *=USAGE_DECREASE_FACTOR;
1739-
/* Accumulate total size, too. */
1740-
totlen+=entry->query_len+1;
1750+
/* In the mean length computation, ignore dropped texts. */
1751+
if (entry->query_len >=0)
1752+
{
1753+
tottextlen+=entry->query_len+1;
1754+
nvalidtexts++;
1755+
}
17411756
}
17421757

1758+
/* Sort into increasing order by usage */
17431759
qsort(entries,i,sizeof(pgssEntry*),entry_cmp);
17441760

1761+
/* Record the (approximate) median usage */
17451762
if (i>0)
1746-
{
1747-
/* Record the (approximate) median usage */
17481763
pgss->cur_median_usage=entries[i /2]->counters.usage;
1749-
/* Record the mean query length */
1750-
pgss->mean_query_len=totlen /i;
1751-
}
1764+
/* Record the mean query length */
1765+
if (nvalidtexts>0)
1766+
pgss->mean_query_len=tottextlen /nvalidtexts;
1767+
else
1768+
pgss->mean_query_len=ASSUMED_LENGTH_INIT;
17521769

1770+
/* Now zap an appropriate fraction of lowest-usage entries */
17531771
nvictims=Max(10,i*USAGE_DEALLOC_PERCENT /100);
17541772
nvictims=Min(nvictims,i);
17551773

@@ -1892,15 +1910,17 @@ qtext_load_file(Size *buffer_size)
18921910
}
18931911

18941912
/* Allocate buffer; beware that off_t might be wider than size_t */
1895-
if (stat.st_size <=MaxAllocSize)
1913+
if (stat.st_size <=MaxAllocHugeSize)
18961914
buf= (char*)malloc(stat.st_size);
18971915
else
18981916
buf=NULL;
18991917
if (buf==NULL)
19001918
{
19011919
ereport(LOG,
19021920
(errcode(ERRCODE_OUT_OF_MEMORY),
1903-
errmsg("out of memory")));
1921+
errmsg("out of memory"),
1922+
errdetail("Could not allocate enough memory to read pg_stat_statement file \"%s\".",
1923+
PGSS_TEXT_FILE)));
19041924
CloseTransientFile(fd);
19051925
returnNULL;
19061926
}
@@ -2002,13 +2022,17 @@ need_gc_qtexts(void)
20022022
* occur in the foreseeable future.
20032023
*
20042024
* The caller must hold an exclusive lock on pgss->lock.
2025+
*
2026+
* At the first sign of trouble we unlink the query text file to get a clean
2027+
* slate (although existing statistics are retained), rather than risk
2028+
* thrashing by allowing the same problem case to recur indefinitely.
20052029
*/
20062030
staticvoid
20072031
gc_qtexts(void)
20082032
{
20092033
char*qbuffer;
20102034
Sizeqbuffer_size;
2011-
FILE*qfile;
2035+
FILE*qfile=NULL;
20122036
HASH_SEQ_STATUShash_seq;
20132037
pgssEntry*entry;
20142038
Sizeextent;
@@ -2023,12 +2047,15 @@ gc_qtexts(void)
20232047
return;
20242048

20252049
/*
2026-
* Load the old texts file. If we fail (out of memory, for instance) just
2027-
* skip the garbage collection.
2050+
* Load the old texts file. If we fail (out of memory, for instance),
2051+
* invalidate query texts. Hopefully this is rare. It might seem better
2052+
* to leave things alone on an OOM failure, but the problem is that the
2053+
* file is only going to get bigger; hoping for a future non-OOM result is
2054+
* risky and can easily lead to complete denial of service.
20282055
*/
20292056
qbuffer=qtext_load_file(&qbuffer_size);
20302057
if (qbuffer==NULL)
2031-
return;
2058+
gotogc_fail;
20322059

20332060
/*
20342061
* We overwrite the query texts file in place, so as to reduce the risk of
@@ -2063,6 +2090,7 @@ gc_qtexts(void)
20632090
/* Trouble ... drop the text */
20642091
entry->query_offset=0;
20652092
entry->query_len=-1;
2093+
/* entry will not be counted in mean query length computation */
20662094
continue;
20672095
}
20682096

@@ -2147,7 +2175,36 @@ gc_qtexts(void)
21472175
entry->query_len=-1;
21482176
}
21492177

2150-
/* Seems like a good idea to bump the GC count even though we failed */
2178+
/*
2179+
* Destroy the query text file and create a new, empty one
2180+
*/
2181+
(void)unlink(PGSS_TEXT_FILE);
2182+
qfile=AllocateFile(PGSS_TEXT_FILE,PG_BINARY_W);
2183+
if (qfile==NULL)
2184+
ereport(LOG,
2185+
(errcode_for_file_access(),
2186+
errmsg("could not write new pg_stat_statement file \"%s\": %m",
2187+
PGSS_TEXT_FILE)));
2188+
else
2189+
FreeFile(qfile);
2190+
2191+
/* Reset the shared extent pointer */
2192+
pgss->extent=0;
2193+
2194+
/* Reset mean_query_len to match the new state */
2195+
pgss->mean_query_len=ASSUMED_LENGTH_INIT;
2196+
2197+
/*
2198+
* Bump the GC count even though we failed.
2199+
*
2200+
* This is needed to make concurrent readers of file without any lock on
2201+
* pgss->lock notice existence of new version of file. Once readers
2202+
* subsequently observe a change in GC count with pgss->lock held, that
2203+
* forces a safe reopen of file. Writers also require that we bump here,
2204+
* of course. (As required by locking protocol, readers and writers don't
2205+
* trust earlier file contents until gc_count is found unchanged after
2206+
* pgss->lock acquired in shared or exclusive mode respectively.)
2207+
*/
21512208
record_gc_qtexts();
21522209
}
21532210

0 commit comments

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