From c3e1f50a90d1862cda48481aa3e13a935976decb Mon Sep 17 00:00:00 2001 From: fhanik Date: Thu, 11 Dec 2008 04:58:11 +0000 Subject: [PATCH] Refactor a bit git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@725575 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/tomcat/jdbc/pool/PoolProperties.java | 2 +- .../jdbc/pool/interceptor/SlowQueryReport.java | 81 ++++++++++++++++------ .../tomcat/jdbc/test/TestSlowQueryReport.java | 10 +-- 3 files changed, 64 insertions(+), 29 deletions(-) diff --git a/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PoolProperties.java b/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PoolProperties.java index dbf499278..c3953cfce 100644 --- a/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PoolProperties.java +++ b/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/PoolProperties.java @@ -54,7 +54,7 @@ public class PoolProperties { protected int removeAbandonedTimeout = 60; protected boolean logAbandoned = false; protected int loginTimeout = 10000; - protected String name = "Tomcat Connection Pool["+(poolCounter.addAndGet(1))+","+System.identityHashCode(PoolProperties.class)+"]"; + protected String name = "Tomcat Connection Pool["+(poolCounter.addAndGet(1))+":"+System.identityHashCode(PoolProperties.class)+"]"; protected String password; protected String username; protected long validationInterval = 30000; diff --git a/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java b/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java index 495ffb43c..6a47ac58e 100644 --- a/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java +++ b/modules/jdbc-pool/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java @@ -24,10 +24,8 @@ import java.sql.CallableStatement; import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Statement; -import java.util.HashMap; -import java.util.IdentityHashMap; -import java.util.LinkedHashMap; -import java.util.Map.Entry; +import java.util.Iterator; +import java.util.concurrent.ConcurrentHashMap; import javax.management.openmbean.CompositeData; @@ -53,12 +51,12 @@ public class SlowQueryReport extends AbstractCreateStatementInterceptor { /** * we will be keeping track of query stats on a per pool basis, do we want this, or global? */ - protected static HashMap> perPoolStats = - new HashMap>(); + protected static ConcurrentHashMap> perPoolStats = + new ConcurrentHashMap>(); /** * the queries that are used for this interceptor. */ - protected HashMap queries = null; + protected ConcurrentHashMap queries = null; /** * The threshold in milliseconds. If the query is faster than this, we don't measure it */ @@ -73,8 +71,8 @@ public class SlowQueryReport extends AbstractCreateStatementInterceptor { * @param pool - the pool we want to retrieve stats for * @return a hash map containing statistics for 0 to maxQueries */ - public static HashMap getPoolStats(ConnectionPool pool) { - return perPoolStats.get(pool); + public static ConcurrentHashMap getPoolStats(String poolname) { + return perPoolStats.get(poolname); } /** @@ -171,19 +169,18 @@ public class SlowQueryReport extends AbstractCreateStatementInterceptor { */ public void reset(ConnectionPool parent, PooledConnection con) { //see if we already created a map for this pool - queries = SlowQueryReport.perPoolStats.get(parent); + queries = SlowQueryReport.perPoolStats.get(parent.getName()); if (queries==null) { //create the map to hold our stats //however TODO we need to improve the eviction //selection - queries = new LinkedHashMap() { - @Override - protected boolean removeEldestEntry(Entry eldest) { - return size()>maxQueries; - } - + queries = new ConcurrentHashMap() { + }; - perPoolStats.put(parent.getName(), queries); + if (perPoolStats.putIfAbsent(parent.getName(), queries)!=null) { + //there already was one + queries = SlowQueryReport.perPoolStats.get(parent.getName()); + } } } @@ -206,6 +203,7 @@ public class SlowQueryReport extends AbstractCreateStatementInterceptor { private long minInvocationTime = Long.MAX_VALUE; private long minInvocationDate; private long totalInvocationTime; + private volatile long lastInvocation = 0; public String toString() { StringBuffer buf = new StringBuffer("QueryStats[query:"); @@ -244,6 +242,7 @@ public class SlowQueryReport extends AbstractCreateStatementInterceptor { } nrOfInvocations++; totalInvocationTime+=invocationTime; + lastInvocation = now; } public String getQuery() { @@ -285,6 +284,10 @@ public class SlowQueryReport extends AbstractCreateStatementInterceptor { } return false; } + + public boolean isOlderThan(QueryStats other) { + return this.lastInvocation < other.lastInvocation; + } } /** @@ -323,7 +326,12 @@ public class SlowQueryReport extends AbstractCreateStatementInterceptor { long delta = (process)?(System.currentTimeMillis()-start):Long.MIN_VALUE; //see if we meet the requirements to measure if (delta>threshold) { - reportSlowQuery(args, name, start, delta); + try { + //report the slow query + reportSlowQuery(args, name, start, delta); + }catch (Exception t) { + if (log.isWarnEnabled()) log.warn("Unable to process slow query",t); + } } //perform close cleanup if (close) { @@ -342,13 +350,40 @@ public class SlowQueryReport extends AbstractCreateStatementInterceptor { } //if we have a query, record the stats if (sql!=null) { - QueryStats qs = SlowQueryReport.this.queries.get(sql); - if (qs == null) { - qs = new QueryStats(sql); - SlowQueryReport.this.queries.put((String)sql,qs); + QueryStats qs = getQueryStats(sql); + if (qs!=null) qs.add(delta,start); + } + } + + private QueryStats getQueryStats(String sql) { + ConcurrentHashMap queries = SlowQueryReport.this.queries; + if (queries==null) return null; + QueryStats qs = queries.get(sql); + if (qs == null) { + qs = new QueryStats(sql); + if (queries.putIfAbsent(sql,qs)!=null) { + qs = queries.get(sql); + } else { + //we added a new element, see if we need to remove the oldest + if (queries.size() > maxQueries) { + removeOldest(queries); + } } - qs.add(delta,start); } + return qs; + } + + /** + * TODO - implement a better algorithm + * @param queries + */ + protected void removeOldest(ConcurrentHashMap queries) { + Iterator it = queries.keySet().iterator(); + while (queries.size()>maxQueries && it.hasNext()) { + String sql = it.next(); + it.remove(); + if (log.isDebugEnabled()) log.debug("Removing slow query, capacity reached:"+sql); + } } } } diff --git a/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/TestSlowQueryReport.java b/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/TestSlowQueryReport.java index 424d65917..547ab0879 100644 --- a/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/TestSlowQueryReport.java +++ b/modules/jdbc-pool/test/org/apache/tomcat/jdbc/test/TestSlowQueryReport.java @@ -5,7 +5,7 @@ import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.Statement; -import java.util.HashMap; +import java.util.Map; import org.apache.tomcat.jdbc.pool.ConnectionPool; import org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReport; @@ -30,7 +30,7 @@ public class TestSlowQueryReport extends DefaultTestCase { rs.close(); st.close(); } - HashMap map = SlowQueryReport.getPoolStats(datasource.getPool()); + Map map = SlowQueryReport.getPoolStats(datasource.getPool().getName()); assertNotNull(map); assertEquals(1,map.size()); String key = map.keySet().iterator().next(); @@ -56,7 +56,7 @@ public class TestSlowQueryReport extends DefaultTestCase { con.close(); tearDown(); //make sure we actually did clean up when the pool closed - assertNull(SlowQueryReport.getPoolStats(pool)); + assertNull(SlowQueryReport.getPoolStats(pool.getName())); } public void testFastSql() throws Exception { @@ -72,13 +72,13 @@ public class TestSlowQueryReport extends DefaultTestCase { rs.close(); st.close(); } - HashMap map = SlowQueryReport.getPoolStats(datasource.getPool()); + Map map = SlowQueryReport.getPoolStats(datasource.getPool().getName()); assertNotNull(map); assertEquals(0,map.size()); ConnectionPool pool = datasource.getPool(); con.close(); tearDown(); - assertNull(SlowQueryReport.getPoolStats(pool)); + assertNull(SlowQueryReport.getPoolStats(pool.getName())); } } -- 2.11.0