https://bz.apache.org/bugzilla/show_bug.cgi?id=64415
--- Comment #5 from Christopher Schultz <ch...@christopherschultz.net> ---
An updated patch which avoids one of the race-conditions identified in the
previous patch. Comments welcome. Only compile-tested at this point.
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..2bf5c74 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,18 +213,55 @@
return qs;
}
+ static class MiniQueryStats {
+ public final QueryStats queryStats;
+ public final long lastInvocation;
+
+ public MiniQueryStats(QueryStats queryStats) {
+ this.queryStats = queryStats;
+ this.lastInvocation = queryStats.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);
+ // Make a defensive deep-copy of the query stats list to prevent
+ // concurrent changes to the lastModified member during list-sort
+ ArrayList<MiniQueryStats> list = new ArrayList<>(queries.size());
+ for(QueryStats stats : queries.values())
+ list.add(new MiniQueryStats(stats));
+
+ Collections.sort(list, miniQueryStatsComparator);
int removeIndex = 0;
while (queries.size() > maxQueries) {
- String sql = list.get(removeIndex).getQuery();
- queries.remove(sql);
- if (log.isDebugEnabled()) log.debug("Removing slow query, capacity
reached:"+sql);
+ MiniQueryStats mqs = list.get(removeIndex);
+ // Check to see if the lastInvocation has been updated since we
+ // took our snapshot. If the timestamps disagree, it means
+ // that this item is no longer the oldest (and it likely now
+ // one of the newest).
+ if(mqs.lastInvocation == mqs.queryStats.lastInvocation) {
+ String sql = mqs.queryStats.query;
+ queries.remove(sql);
+ if (log.isDebugEnabled()) log.debug("Removing slow query,
capacity reached:"+sql);
+ }
removeIndex++;
}
}
--
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
