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