https://bz.apache.org/bugzilla/show_bug.cgi?id=64415
--- Comment #3 from Christopher Schultz <ch...@christopherschultz.net> ---
Here is a patch that has been only compile-tested.
It introduces a small class to represent the important information used to make
decisions during removeOldest() and creates a (fairly) stable collection to
sort.
===== CUT =====
diff --git
a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java
b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java
index b6c207d..7657ab5 100644
---
a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java
+++
b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java
@@ -213,16 +213,44 @@
return qs;
}
+ static class MiniQueryStats {
+ public final String query;
+ public final long lastInvocation;
+
+ public MiniQueryStats(String query, long lastInvocation) {
+ this.query = query;
+ this.lastInvocation = lastInvocation;
+ }
+ }
+
+ static class MiniQueryStatsComparator implements
Comparator<MiniQueryStats>
+ {
+ @Override
+ public int compare(MiniQueryStats stats1, MiniQueryStats stats2) {
+ return Long.compare(handleZero(stats1.lastInvocation),
+ handleZero(stats2.lastInvocation));
+ }
+
+ private static long handleZero(long value) {
+ return value == 0 ? Long.MAX_VALUE : value;
+ }
+ }
+
+ private MiniQueryStatsComparator miniQueryStatsComparator = new
MiniQueryStatsComparator();
+
/**
* Sort QueryStats by last invocation time
* @param queries The queries map
*/
protected void removeOldest(ConcurrentHashMap<String,QueryStats> queries)
{
- ArrayList<QueryStats> list = new ArrayList<>(queries.values());
- Collections.sort(list, queryStatsComparator);
+ ArrayList<MiniQueryStats> list = new ArrayList<>(queries.size());
+ for(QueryStats stats : queries.values())
+ list.add(new MiniQueryStats(stats.query, stats.lastInvocation));
+
+ Collections.sort(list, miniQueryStatsComparator);
int removeIndex = 0;
while (queries.size() > maxQueries) {
- String sql = list.get(removeIndex).getQuery();
+ String sql = list.get(removeIndex).query;
queries.remove(sql);
if (log.isDebugEnabled()) log.debug("Removing slow query, capacity
reached:"+sql);
removeIndex++;
===== CUT =====
There are a few race-conditions still present:
1. If the Map.values changes during the execution of the copy-loop, it's
possible that QueryStats objects could be counted more than once, or not at
all. I'm not an expert at how collections behave under high contention.
2. If the snapshot of the stats-by-query-string schedules the removal of an
"old" query, but the query is executed after the snapshot was taken, then the
query will be removed from the stats pool which may be surprising. One way to
get around this is to check that miniQueryStats.lastInvocation ==
queryStats.lastInvocation and skip that item if it's been run recently.
Possibly more.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
