From 9880ff326c7d7ca4b53a365a36b62f823692a42d Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Fri, 21 Nov 2008 01:49:28 +0000 Subject: [PATCH] A whole bunch of revampery on the way to collection caching, including some basic stats reporting and query and cache logging. PersistenceContext no longer sticks its nose so deeply into the business of Query and Modifier. It just passes itself along and allows them to do any cache activity they need during their normal execution. The one situation where we keep our nose in their business is to allow a Query to return a result from the cache before it is even invoked so that we can avoid requesting a database connection if we won't ever need one. --- .../com/samskivert/depot/DepotMarshaller.java | 37 ++-- .../com/samskivert/depot/DepotRepository.java | 40 ++-- .../com/samskivert/depot/FindAllQuery.java | 177 +++++++++++------- .../com/samskivert/depot/FindOneQuery.java | 75 +++++--- src/java/com/samskivert/depot/Key.java | 22 ++- src/java/com/samskivert/depot/Modifier.java | 41 ++-- src/java/com/samskivert/depot/Operation.java | 7 +- .../samskivert/depot/PersistenceContext.java | 64 ++++--- src/java/com/samskivert/depot/Query.java | 32 +--- .../com/samskivert/depot/SchemaMigration.java | 32 ++-- src/java/com/samskivert/depot/Stats.java | 138 ++++++++++++++ .../samskivert/depot/clause/SelectClause.java | 26 +++ .../com/samskivert/depot/clause/Where.java | 6 + .../depot/expression/ColumnExp.java | 6 + .../samskivert/depot/expression/ValueExp.java | 6 + .../depot/operator/Conditionals.java | 32 ++++ .../com/samskivert/depot/operator/Logic.java | 10 +- .../depot/operator/SQLOperator.java | 19 ++ 18 files changed, 548 insertions(+), 222 deletions(-) create mode 100644 src/java/com/samskivert/depot/Stats.java diff --git a/src/java/com/samskivert/depot/DepotMarshaller.java b/src/java/com/samskivert/depot/DepotMarshaller.java index 1ef7168..c1f5345 100644 --- a/src/java/com/samskivert/depot/DepotMarshaller.java +++ b/src/java/com/samskivert/depot/DepotMarshaller.java @@ -578,7 +578,7 @@ public class DepotMarshaller // check to see if our schema version table exists, create it if not ctx.invoke(new Modifier() { @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { liaison.createTableIfMissing( conn, SCHEMA_VERSION_TABLE, new String[] { P_COLUMN, V_COLUMN, MV_COLUMN }, @@ -697,7 +697,7 @@ public class DepotMarshaller final Iterable indexen = _indexes.values(); ctx.invoke(new Modifier() { @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { // create the table String[] primaryKeyColumns = null; if (_pkColumns != null) { @@ -803,8 +803,8 @@ public class DepotMarshaller if (hasPrimaryKey() && metaData.pkName == null) { log.info("Adding primary key."); ctx.invoke(new Modifier() { - @Override public Integer invoke (Connection conn, DatabaseLiaison liaison) - throws SQLException { + @Override + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { liaison.addPrimaryKey( conn, getTableName(), fieldsToColumns(getPrimaryKeyFields())); return 0; @@ -815,8 +815,8 @@ public class DepotMarshaller final String pkName = metaData.pkName; log.info("Dropping primary key: " + pkName); ctx.invoke(new Modifier() { - @Override public Integer invoke (Connection conn, DatabaseLiaison liaison) - throws SQLException { + @Override + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { liaison.dropPrimaryKey(conn, getTableName(), pkName); return 0; } @@ -833,8 +833,8 @@ public class DepotMarshaller } // but this is a new, named index, so we create it ctx.invoke(new Modifier() { - @Override public Integer invoke (Connection conn, DatabaseLiaison liaison) - throws SQLException { + @Override + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { liaison.addIndexToTable( conn, getTableName(), fieldsToColumns(index.fields()), ixName, index.unique()); @@ -869,8 +869,8 @@ public class DepotMarshaller final String[] colArr = colSet.toArray(new String[colSet.size()]); final String fName = indexName; ctx.invoke(new Modifier() { - @Override public Integer invoke (Connection conn, DatabaseLiaison liaison) - throws SQLException { + @Override + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { liaison.addIndexToTable(conn, getTableName(), colArr, fName, true); return 0; } @@ -891,8 +891,8 @@ public class DepotMarshaller // but not this one, so let's create it ctx.invoke(new Modifier() { - @Override public Integer invoke (Connection conn, DatabaseLiaison liaison) - throws SQLException { + @Override + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { builder.addFullTextSearch(conn, DepotMarshaller.this, recordFts); return 0; } @@ -937,8 +937,8 @@ public class DepotMarshaller // initialValue which will use the new column name to obtain the sequence name which // ain't going to work either; we punt! ctx.invoke(new Modifier() { - @Override public Integer invoke (Connection conn, DatabaseLiaison liaison) - throws SQLException { + @Override + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { valgen.create(conn, liaison); return 0; } @@ -983,7 +983,7 @@ public class DepotMarshaller protected abstract class SimpleModifier extends Modifier { @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { Statement stmt = conn.createStatement(); try { return invoke(liaison, stmt); @@ -1035,10 +1035,13 @@ public class DepotMarshaller throws DatabaseException { return ctx.invoke(new Query.Trivial() { - @Override public TableMetaData invoke (Connection conn, DatabaseLiaison dl) - throws SQLException { + @Override public TableMetaData invoke (PersistenceContext ctx, Connection conn, + DatabaseLiaison dl) throws SQLException { return new TableMetaData(conn.getMetaData(), tableName); } + public void updateStats (Stats stats) { + // nothing doing + } }); } diff --git a/src/java/com/samskivert/depot/DepotRepository.java b/src/java/com/samskivert/depot/DepotRepository.java index d3cb49f..a3261a1 100644 --- a/src/java/com/samskivert/depot/DepotRepository.java +++ b/src/java/com/samskivert/depot/DepotRepository.java @@ -37,6 +37,7 @@ import com.google.common.collect.Lists; import com.google.common.collect.Sets; import com.samskivert.util.ArrayUtil; +import com.samskivert.util.StringUtil; import com.samskivert.jdbc.ConnectionProvider; import com.samskivert.jdbc.DatabaseLiaison; @@ -308,7 +309,7 @@ public abstract class DepotRepository * the latest data. */ protected List> findAllKeys ( - Class type, boolean forUpdate, Collection clauses) + final Class type, boolean forUpdate, Collection clauses) throws DatabaseException { final List> keys = Lists.newArrayList(); @@ -319,36 +320,53 @@ public abstract class DepotRepository if (forUpdate) { _ctx.invoke(new Modifier(null) { - @Override public Integer invoke (Connection conn, DatabaseLiaison liaison) - throws SQLException { + @Override + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { PreparedStatement stmt = builder.prepare(conn); try { ResultSet rs = stmt.executeQuery(); while (rs.next()) { keys.add(marsh.makePrimaryKey(rs)); } + // TODO: cache this result? + if (PersistenceContext.CACHE_DEBUG) { + log.info("Loaded " + StringUtil.shortClassName(type) + " keys", + "count", keys.size()); + } return 0; } finally { JDBCUtil.close(stmt); } } + @Override public void updateStats (Stats stats) { + stats.noteQuery(0, 1, 0, 0); // one uncached query + } }); } else { _ctx.invoke(new Query.Trivial() { @Override - public Void invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + public Void invoke (PersistenceContext ctx, Connection conn, + DatabaseLiaison liaison) throws SQLException { PreparedStatement stmt = builder.prepare(conn); try { ResultSet rs = stmt.executeQuery(); while (rs.next()) { keys.add(marsh.makePrimaryKey(rs)); } + // TODO: cache this result? + if (PersistenceContext.CACHE_DEBUG) { + log.info("Loaded " + StringUtil.shortClassName(type) + " keys", + "count", keys.size()); + } return null; } finally { JDBCUtil.close(stmt); } } + public void updateStats (Stats stats) { + stats.noteQuery(0, 1, 0, 0); // one uncached query + } }); } @@ -375,7 +393,7 @@ public abstract class DepotRepository // key will be null if record was supplied without a primary key return _ctx.invoke(new CachingModifier(record, key, key) { @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { // if needed, update our modifier's key so that it can cache our results Set identityFields = Collections.emptySet(); if (_key == null) { @@ -427,7 +445,7 @@ public abstract class DepotRepository return _ctx.invoke(new CachingModifier(record, key, key) { @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { PreparedStatement stmt = builder.prepare(conn); try { return stmt.executeUpdate(); @@ -464,7 +482,7 @@ public abstract class DepotRepository return _ctx.invoke(new CachingModifier(record, key, key) { @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { PreparedStatement stmt = builder.prepare(conn); // clear out _result so that we don't rewrite this partial record to the cache _result = null; @@ -604,7 +622,7 @@ public abstract class DepotRepository return _ctx.invoke(new Modifier(invalidator) { @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { PreparedStatement stmt = builder.prepare(conn); try { return stmt.executeUpdate(); @@ -735,7 +753,7 @@ public abstract class DepotRepository return _ctx.invoke(new Modifier(invalidator) { @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { PreparedStatement stmt = builder.prepare(conn); try { return stmt.executeUpdate(); @@ -773,7 +791,7 @@ public abstract class DepotRepository final boolean[] created = new boolean[1]; _ctx.invoke(new CachingModifier(record, key, key) { @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { PreparedStatement stmt = null; try { if (_key != null) { @@ -897,7 +915,7 @@ public abstract class DepotRepository return _ctx.invoke(new Modifier(invalidator) { @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { PreparedStatement stmt = builder.prepare(conn); try { return stmt.executeUpdate(); diff --git a/src/java/com/samskivert/depot/FindAllQuery.java b/src/java/com/samskivert/depot/FindAllQuery.java index 6dcf86b..7c41d7f 100644 --- a/src/java/com/samskivert/depot/FindAllQuery.java +++ b/src/java/com/samskivert/depot/FindAllQuery.java @@ -75,13 +75,12 @@ public abstract class FindAllQuery } } - SelectClause select = - new SelectClause(_type, _marsh.getPrimaryKeyFields(), clauses); - _builder = _ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, select)); - _builder.newQuery(select); + _select = new SelectClause(_type, _marsh.getPrimaryKeyFields(), clauses); + _builder = _ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, _select)); + _builder.newQuery(_select); } - public List invoke (Connection conn, DatabaseLiaison liaison) + public List invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison) throws SQLException { Map, T> entities = Maps.newHashMap(); @@ -96,15 +95,11 @@ public abstract class FindAllQuery Key key = _marsh.makePrimaryKey(rs); allKeys.add(key); - // TODO: All this cache fiddling needs to move to PersistenceContext? - CacheAdapter.CachedValue hit = _ctx.cacheLookup(key); - if (hit != null) { - T value = hit.getValue(); - if (value != null) { - @SuppressWarnings("unchecked") T newValue = (T) value.clone(); - entities.put(key, newValue); - continue; - } + T value = _ctx.cacheLookup(key); + if (value != null) { + @SuppressWarnings("unchecked") T newValue = (T) value.clone(); + entities.put(key, newValue); + continue; } fetchKeys.add(key); @@ -114,8 +109,23 @@ public abstract class FindAllQuery JDBCUtil.close(stmt); } - return loadAndResolve(conn, allKeys, fetchKeys, entities, stmtString); + _cachedRecords = entities.size(); + _uncachedRecords = fetchKeys.size(); + + if (PersistenceContext.CACHE_DEBUG) { + log.info("Loaded " + _marsh.getTableName(), "query", _select, + "keys", keysToString(allKeys)); + } + + return loadAndResolve(ctx, conn, allKeys, fetchKeys, entities, stmtString); } + + public void updateStats (Stats stats) { + stats.noteQuery(0, 1, _cachedRecords, _uncachedRecords); // one uncached query + } + + protected SelectClause _select; + protected int _cachedRecords, _uncachedRecords; } /** @@ -132,29 +142,33 @@ public abstract class FindAllQuery _builder = ctx.getSQLBuilder(new DepotTypes(ctx, _type)); } - public List invoke (Connection conn, DatabaseLiaison liaison) + public List invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison) throws SQLException { Map, T> entities = Maps.newHashMap(); Set> fetchKeys = Sets.newHashSet(); for (Key key : _keys) { - // TODO: All this cache fiddling needs to move to PersistenceContext? - CacheAdapter.CachedValue hit = _ctx.cacheLookup(key); - if (hit != null) { - T value = hit.getValue(); - if (value != null) { - @SuppressWarnings("unchecked") T newValue = (T) value.clone(); - entities.put(key, newValue); - continue; - } + T value = _ctx.cacheLookup(key); + if (value != null) { + @SuppressWarnings("unchecked") T newValue = (T) value.clone(); + entities.put(key, newValue); + continue; } fetchKeys.add(key); } - return loadAndResolve(conn, _keys, fetchKeys, entities, null); + _cachedRecords = entities.size(); + _uncachedRecords = fetchKeys.size(); + + return loadAndResolve(ctx, conn, _keys, fetchKeys, entities, null); + } + + public void updateStats (Stats stats) { + stats.noteQuery(0, 0, _cachedRecords, _uncachedRecords); } protected Collection> _keys; + protected int _cachedRecords, _uncachedRecords; } /** @@ -168,12 +182,12 @@ public abstract class FindAllQuery throws DatabaseException { super(ctx, type); - SelectClause select = new SelectClause(type, _marsh.getFieldNames(), clauses); - _builder = ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, select)); - _builder.newQuery(select); + _select = new SelectClause(type, _marsh.getFieldNames(), clauses); + _builder = ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, _select)); + _builder.newQuery(_select); } - public List invoke (Connection conn, DatabaseLiaison liaison) + public List invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison) throws SQLException { List result = Lists.newArrayList(); @@ -182,12 +196,25 @@ public abstract class FindAllQuery ResultSet rs = stmt.executeQuery(); while (rs.next()) { result.add(_marsh.createObject(rs)); + _uncachedRecords++; } } finally { JDBCUtil.close(stmt); } + if (PersistenceContext.CACHE_DEBUG) { + log.info("Loaded " + _marsh.getTableName(), "query", _select, + "uncached", _uncachedRecords); + } + // TODO: do we want to cache these results? return result; } + + public void updateStats (Stats stats) { + stats.noteQuery(0, 1, 0, _uncachedRecords); + } + + protected SelectClause _select; + protected int _uncachedRecords; } public FindAllQuery (PersistenceContext ctx, Class type) @@ -198,45 +225,46 @@ public abstract class FindAllQuery _marsh = _ctx.getMarshaller(type); } - // from Query - public CacheKey getCacheKey () + // from interface Query + public List getCachedResult (PersistenceContext ctx) { - return null; + return null; // TODO } - // from Query - public void updateCache (PersistenceContext ctx, List result) { - if (_marsh.hasPrimaryKey()) { - for (T bit : result) { - ctx.cacheStore(_marsh.getPrimaryKey(bit), bit.clone()); - } - } - } +// // from interface Query +// public List transformCacheHit (CacheKey key, List bits) +// { +// if (bits == null) { +// return bits; +// } - // from Query - public List transformCacheHit (CacheKey key, List bits) +// List result = Lists.newArrayList(); +// for (T bit : bits) { +// if (bit != null) { +// @SuppressWarnings("unchecked") T cbit = (T) bit.clone(); +// result.add(cbit); +// } else { +// result.add(null); +// } +// } +// return result; +// } + + // from interface Operation + public void updateStats (Stats stats) { - if (bits == null) { - return bits; - } - - List result = Lists.newArrayList(); - for (T bit : bits) { - if (bit != null) { - @SuppressWarnings("unchecked") T cbit = (T) bit.clone(); - result.add(cbit); - } else { - result.add(null); - } - } - return result; + // TODO } - protected List loadAndResolve (Connection conn, Collection> allKeys, - Set> fetchKeys, Map, T> entities, - String origStmt) + protected List loadAndResolve (PersistenceContext ctx, Connection conn, + Collection> allKeys, Set> fetchKeys, + Map, T> entities, String origStmt) throws SQLException { + if (PersistenceContext.CACHE_DEBUG && fetchKeys.size() > 0) { + log.info("Loading " + _marsh.getTableName(), "keys", keysToString(fetchKeys)); + } + // if we're fetching a huge number of records, we have to do it in multiple queries if (fetchKeys.size() > In.MAX_KEYS) { int keyCount = fetchKeys.size(); @@ -248,11 +276,11 @@ public abstract class FindAllQuery iter.remove(); } keyCount -= keys.size(); - loadRecords(conn, keys, entities, origStmt); + loadRecords(ctx, conn, keys, entities, origStmt); } while (keyCount > 0); } else if (fetchKeys.size() > 0) { - loadRecords(conn, fetchKeys, entities, origStmt); + loadRecords(ctx, conn, fetchKeys, entities, origStmt); } List result = Lists.newArrayList(); @@ -265,10 +293,11 @@ public abstract class FindAllQuery return result; } - protected void loadRecords (Connection conn, Set> keys, Map, T> entities, - String origStmt) + protected void loadRecords (PersistenceContext ctx, Connection conn, Set> keys, + Map, T> entities, String origStmt) throws SQLException { + boolean hasPrimaryKey = _marsh.hasPrimaryKey(); _builder.newQuery(new SelectClause(_type, _marsh.getFieldNames(), new KeySet(_type, keys))); PreparedStatement stmt = _builder.prepare(conn); @@ -282,6 +311,10 @@ public abstract class FindAllQuery if (entities.put(key, obj) != null) { dups++; } + // cache our result if it has a primary key + if (hasPrimaryKey) { + ctx.cacheStore(_marsh.getPrimaryKey(obj), obj.clone()); + } got.add(key); cnt++; } @@ -292,11 +325,27 @@ public abstract class FindAllQuery "wanted", keys, "got", got, "dups", dups, new Exception()); } + if (PersistenceContext.CACHE_DEBUG) { + log.info("Cached " + _marsh.getTableName(), "count", cnt); + } + } finally { JDBCUtil.close(stmt); } } + protected String keysToString (Iterable> keySet) + { + StringBuilder builder = new StringBuilder("("); + for (Key key : keySet) { + if (builder.length() > 1) { + builder.append(", "); + } + key.toShortString(builder); + } + return builder.append(")").toString(); + } + protected PersistenceContext _ctx; protected SQLBuilder _builder; protected DepotMarshaller _marsh; diff --git a/src/java/com/samskivert/depot/FindOneQuery.java b/src/java/com/samskivert/depot/FindOneQuery.java index d88d0d3..9c698fa 100644 --- a/src/java/com/samskivert/depot/FindOneQuery.java +++ b/src/java/com/samskivert/depot/FindOneQuery.java @@ -27,9 +27,13 @@ import java.sql.SQLException; import com.samskivert.jdbc.DatabaseLiaison; import com.samskivert.jdbc.JDBCUtil; +import com.samskivert.util.StringUtil; + import com.samskivert.depot.clause.QueryClause; import com.samskivert.depot.clause.SelectClause; +import static com.samskivert.depot.Log.log; + /** * The implementation of {@link DepotRepository#load} functionality. */ @@ -49,21 +53,30 @@ public class FindOneQuery _builder.newQuery(_select); } - // from Query - public CacheKey getCacheKey () + // from interface Query + public T getCachedResult (PersistenceContext ctx) { - WhereClause where = _select.getWhereClause(); - if (where != null && where instanceof CacheKey) { - return (CacheKey) where; + CacheKey key = getCacheKey(); + if (key == null) { + return null; } - return null; + T value = ctx.cacheLookup(key); + if (value == null) { + return null; + } + _cachedRecords = 1; + // we do not want to return a reference to the actual cached entity so we clone it + @SuppressWarnings("unchecked") T cvalue = (T) value.clone(); + return cvalue; } - // from Query - public T invoke (Connection conn, DatabaseLiaison liaison) throws SQLException + // from interface Query + public T invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison) + throws SQLException { PreparedStatement stmt = _builder.prepare(conn); try { + // load up the record in question T result = null; ResultSet rs = stmt.executeQuery(); if (rs.next()) { @@ -71,6 +84,25 @@ public class FindOneQuery } // TODO: if (rs.next()) issue warning? rs.close(); + + // potentially cache the result + CacheKey key = getCacheKey(); + if (key == null) { + // no row-specific cache key was given, if we can, create a key from the record + if (result != null && _marsh.hasPrimaryKey()) { + key = _marsh.getPrimaryKey(result); + } + } + if (PersistenceContext.CACHE_DEBUG) { + log.info("Loaded " + (key != null ? key : _marsh.getTableName())); + } + if (key != null) { + ctx.cacheStore(key, (result != null) ? result.clone() : null); + if (PersistenceContext.CACHE_DEBUG) { + log.info("Cached " + key); + } + } + return result; } finally { @@ -78,33 +110,20 @@ public class FindOneQuery } } - // from Query - public void updateCache (PersistenceContext ctx, T result) + // from interface Operation + public void updateStats (Stats stats) { - CacheKey key = getCacheKey(); - if (key == null) { - // no row-specific cache key was given - if (result == null || !_marsh.hasPrimaryKey()) { - return; - } - // if we can, create a key from what was actually returned - key = _marsh.getPrimaryKey(result); - } - ctx.cacheStore(key, (result != null) ? result.clone() : null); + stats.noteQuery(0, 0, _cachedRecords, 1-_cachedRecords); } - // from Query - public T transformCacheHit (CacheKey key, T value) + protected CacheKey getCacheKey () { - if (value == null) { - return null; - } - // we do not want to return a reference to the actual cached entity so we clone it - @SuppressWarnings("unchecked") T cvalue = (T) value.clone(); - return cvalue; + WhereClause where = _select.getWhereClause(); + return (where != null && where instanceof CacheKey) ? (CacheKey)where : null; } protected DepotMarshaller _marsh; protected SelectClause _select; protected SQLBuilder _builder; + protected int _cachedRecords; } diff --git a/src/java/com/samskivert/depot/Key.java b/src/java/com/samskivert/depot/Key.java index d0b43b0..a38272e 100644 --- a/src/java/com/samskivert/depot/Key.java +++ b/src/java/com/samskivert/depot/Key.java @@ -199,6 +199,20 @@ public class Key extends WhereClause ctx.cacheInvalidate(this); } + /** + * Appends just the key=value portion of our {@link #toString} to the supplied buffer. + */ + public void toShortString (StringBuilder builder) + { + String[] keyFields = KeyUtil.getKeyFields(_pClass); + for (int ii = 0; ii < keyFields.length; ii ++) { + if (ii > 0) { + builder.append(":"); + } + builder.append(keyFields[ii]).append("=").append(_values.get(ii)); + } + } + @Override // from WhereClause public void validateQueryType (Class pClass) { @@ -229,13 +243,7 @@ public class Key extends WhereClause { StringBuilder builder = new StringBuilder(_pClass.getSimpleName()); builder.append("("); - String[] keyFields = KeyUtil.getKeyFields(_pClass); - for (int ii = 0; ii < keyFields.length; ii ++) { - if (ii > 0) { - builder.append(", "); - } - builder.append(keyFields[ii]).append("=").append(_values.get(ii)); - } + toShortString(builder); builder.append(")"); return builder.toString(); } diff --git a/src/java/com/samskivert/depot/Modifier.java b/src/java/com/samskivert/depot/Modifier.java index e1dbaa6..44594e4 100644 --- a/src/java/com/samskivert/depot/Modifier.java +++ b/src/java/com/samskivert/depot/Modifier.java @@ -3,7 +3,7 @@ // // Depot library - a Java relational persistence library // Copyright (C) 2006-2008 Michael Bayne and Pär Winzell -// +// // This library is free software; you can redistribute it and/or modify it // under the terms of the GNU Lesser General Public License as published // by the Free Software Foundation; either version 2.1 of the License, or @@ -41,8 +41,8 @@ public abstract class Modifier implements Operation super(null); } - @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + @Override // from Modifier + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { Statement stmt = conn.createStatement(); try { return stmt.executeUpdate(createQuery(liaison)); @@ -86,25 +86,21 @@ public abstract class Modifier implements Operation } @Override // from Modifier - public void cacheUpdate (PersistenceContext ctx) + public Integer invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison) + throws SQLException { - super.cacheUpdate(ctx); + Integer rows = super.invoke(ctx, conn, liaison); // if we have both a key and a record, cache if (_key != null && _result != null) { ctx.cacheStore(_key, _result.clone()); } + return rows; } protected CacheKey _key; protected T _result; } - /** - * Overriden to perform the actual database modifications represented by this object; should - * return the number of modified rows. - */ - public abstract Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException; - /** * Constructs a {@link Modifier} without a cache invalidator. */ @@ -121,24 +117,27 @@ public abstract class Modifier implements Operation _invalidator = invalidator; } - /** - * Do any cache invalidation needed for this modification. This method is called just - * before the database statement is executed. - */ - public void cacheInvalidation (PersistenceContext ctx) + // from interface Operation + public Integer invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison) + throws SQLException { if (_invalidator != null) { _invalidator.invalidate(ctx); } + return invoke(conn, liaison); + } + + // from interface Operation + public void updateStats (Stats stats) + { + // nothing to update for modifiers } /** - * Do any cache updates needed for this modification. This method is called just after - * the successful execution of the database statement. + * Overriden to perform the actual database modifications represented by this object; should + * return the number of modified rows. */ - public void cacheUpdate (PersistenceContext ctx) - { - } + protected abstract int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException; protected CacheInvalidator _invalidator; } diff --git a/src/java/com/samskivert/depot/Operation.java b/src/java/com/samskivert/depot/Operation.java index d79b9c1..bbb18cf 100644 --- a/src/java/com/samskivert/depot/Operation.java +++ b/src/java/com/samskivert/depot/Operation.java @@ -33,6 +33,11 @@ public interface Operation /** * Performs the actual JDBC interactions associated with this operation. */ - public T invoke (Connection conn, DatabaseLiaison liaison) + public T invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison) throws SQLException; + + /** + * Called after the operation has been invoked so that it can update our runtime statistics. + */ + public void updateStats (Stats stats); } diff --git a/src/java/com/samskivert/depot/PersistenceContext.java b/src/java/com/samskivert/depot/PersistenceContext.java index 55d5594..3aa336d 100644 --- a/src/java/com/samskivert/depot/PersistenceContext.java +++ b/src/java/com/samskivert/depot/PersistenceContext.java @@ -53,6 +53,9 @@ public class PersistenceContext /** Allow toggling of query logging and other debug output via a system property. */ public static final boolean DEBUG = Boolean.getBoolean("com.samskivert.depot.debug"); + /** Allow toggling of cache-related logging via a system property. */ + public static final boolean CACHE_DEBUG = Boolean.getBoolean("com.samskivert.depot.cache_debug"); + /** Map {@link TableGenerator} instances by name. */ public Map tableGenerators = Maps.newHashMap(); @@ -173,6 +176,14 @@ public class PersistenceContext _conprov.shutdown(); } + /** + * Returns a snapshot of our current runtime statistics. + */ + public Stats.Snapshot getStats () + { + return _stats.getSnapshot(); + } + /** * Create and return a new {@link SQLBuilder} for the appropriate dialect. * @@ -258,26 +269,14 @@ public class PersistenceContext public T invoke (Query query) throws DatabaseException { - CacheKey key = query.getCacheKey(); - // if there is a cache key, check the cache - if (key != null && _cache != null) { - CacheAdapter.CachedValue cacheHit = cacheLookup(key); - if (cacheHit != null) { - log.debug("cache hit [key=" + key + ", hit=" + cacheHit + "]"); - T value = cacheHit.getValue(); - value = query.transformCacheHit(key, value); - if (value != null) { - return value; - } - log.debug("transformCacheHit returned null; rejecting cached value."); - } - log.debug("cache miss [key=" + key + "]"); + // we check to see if the query is already cached before invoking it to avoid requesting a + // database connection if we don't actually need one + T result = query.getCachedResult(this); + if (result != null) { + query.updateStats(_stats); + return result; } - // otherwise, perform the query - T result = invoke(query, true); - // and let the caller figure out if it wants to cache itself somehow - query.updateCache(this, result); - return result; + return invoke(query, true); } /** @@ -286,12 +285,7 @@ public class PersistenceContext public int invoke (Modifier modifier) throws DatabaseException { - modifier.cacheInvalidation(this); - int rows = invoke(modifier, true); - if (rows > 0) { - modifier.cacheUpdate(this); - } - return rows; + return invoke(modifier, true); } /** @@ -305,13 +299,14 @@ public class PersistenceContext /** * Looks up an entry in the cache by the given key. */ - public CacheAdapter.CachedValue cacheLookup (CacheKey key) + public T cacheLookup (CacheKey key) { if (_cache == null) { return null; } CacheAdapter.CacheBin bin = _cache.getCache(key.getCacheId()); - return bin.lookup(key.getCacheKey()); + CacheAdapter.CachedValue ref = bin.lookup(key.getCacheKey()); + return (ref == null) ? null : ref.getValue(); } /** @@ -530,6 +525,7 @@ public class PersistenceContext boolean isReadOnly = !(op instanceof Modifier); Connection conn; + long preConnect = System.nanoTime(); try { conn = _conprov.getConnection(_ident, isReadOnly); } catch (PersistenceException pe) { @@ -541,10 +537,15 @@ public class PersistenceContext // connection pooling which will block in getConnection() instead of returning a connection // that someone else may be using synchronized (conn) { + long preInvoke = System.nanoTime(); try { - // if this becomes more complex than this single statement, then this should turn - // into a method call that contains the complexity - return op.invoke(conn, _liaison); + // invoke our database operation + T value = op.invoke(this, conn, _liaison); + // note the time it took to invoke this operation + _stats.noteOp(isReadOnly, preConnect, preInvoke, System.nanoTime()); + // have the operation update any appropriate runtime statistics as well + op.updateStats(_stats); + return value; } catch (SQLException sqe) { if (!isReadOnly) { @@ -594,6 +595,9 @@ public class PersistenceContext protected DatabaseLiaison _liaison; protected boolean _warnOnLazyInit; + /** Used to track various statistics. */ + protected Stats _stats = new Stats(); + /** The object through which all our caching is relayed, or null, for no caching. */ protected CacheAdapter _cache; diff --git a/src/java/com/samskivert/depot/Query.java b/src/java/com/samskivert/depot/Query.java index 7df2559..0879502 100644 --- a/src/java/com/samskivert/depot/Query.java +++ b/src/java/com/samskivert/depot/Query.java @@ -34,40 +34,22 @@ public interface Query extends Operation /** A simple base class for non-complex queries. */ public abstract class Trivial implements Query { - public abstract T invoke (Connection conn, DatabaseLiaison liaison) throws SQLException; + public abstract T invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison) + throws SQLException; - public CacheKey getCacheKey () { + public T getCachedResult (PersistenceContext ctx) { return null; } public T transformCacheHit (CacheKey key, T value) { return value; } - - public void updateCache (PersistenceContext ctx, T result) { - } } /** - * Any query may elect to utilize the built-in cache by returning a non-null {@link CacheKey} - * in this method. This is done automatically by the {@link DepotRepository} when looking up - * single entities by primary key, but even entire collections can be cached under a single - * key. - * - * Great care must be taken to invalidate such cached collections when their constituent - * entities are invalidated. This is generally done using {@link CacheListener} and - * {@link CacheInvalidator}. + * If this query has a simple cached result, it should return the non-null result from this + * method. If null is returned, the query will be {@link #invoke}d to obtain its result from + * persistent storage. */ - public CacheKey getCacheKey (); - - /** - * Overriden by subclasses to perform special operations when the query would return a cache - * hit. The value may be mutated, modified, or null may be return to force a database hit. - */ - public T transformCacheHit (CacheKey key, T value); - - /** - * Overriden by subclasses to perform case-by-case cache updates. - */ - public void updateCache (PersistenceContext ctx, T result); + public T getCachedResult (PersistenceContext ctx); } diff --git a/src/java/com/samskivert/depot/SchemaMigration.java b/src/java/com/samskivert/depot/SchemaMigration.java index c51f476..0252697 100644 --- a/src/java/com/samskivert/depot/SchemaMigration.java +++ b/src/java/com/samskivert/depot/SchemaMigration.java @@ -53,7 +53,7 @@ public abstract class SchemaMigration extends Modifier } @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { if (!liaison.tableContainsColumn(conn, _tableName, _columnName)) { // we'll accept this inconsistency log.warning(_tableName + "." + _columnName + " already dropped."); @@ -78,8 +78,12 @@ public abstract class SchemaMigration extends Modifier _newColumnName = newColumnName; } + @Override public boolean runBeforeDefault () { + return true; + } + @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { if (!liaison.tableContainsColumn(conn, _tableName, _oldColumnName)) { if (liaison.tableContainsColumn(conn, _tableName, _newColumnName)) { // we'll accept this inconsistency @@ -104,10 +108,6 @@ public abstract class SchemaMigration extends Modifier conn, _tableName, _oldColumnName, _newColumnName, _newColumnDef) ? 1 : 0; } - @Override public boolean runBeforeDefault () { - return true; - } - @Override protected void init (String tableName, Map> marshallers) { super.init(tableName, marshallers); @@ -135,18 +135,18 @@ public abstract class SchemaMigration extends Modifier _fieldName = fieldName; } + @Override public boolean runBeforeDefault () { + return false; + } + @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { log.info("Updating type of '" + _fieldName + "' in " + _tableName); return liaison.changeColumn(conn, _tableName, _fieldName, _newColumnDef.getType(), _newColumnDef.isNullable(), _newColumnDef.isUnique(), _newColumnDef.getDefaultValue()) ? 1 : 0; } - @Override public boolean runBeforeDefault () { - return false; - } - @Override protected void init (String tableName, Map> marshallers) { super.init(tableName, marshallers); @@ -174,8 +174,12 @@ public abstract class SchemaMigration extends Modifier _defaultValue = defaultValue; } + @Override public boolean runBeforeDefault () { + return true; + } + @Override - public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { + protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { // override the default value in the column definition with the one provided ColumnDefinition defColumnDef = new ColumnDefinition( _newColumnDef.getType(), _newColumnDef.isNullable(), @@ -190,10 +194,6 @@ public abstract class SchemaMigration extends Modifier return 0; } - @Override public boolean runBeforeDefault () { - return true; - } - @Override protected void init (String tableName, Map> marshallers) { super.init(tableName, marshallers); diff --git a/src/java/com/samskivert/depot/Stats.java b/src/java/com/samskivert/depot/Stats.java new file mode 100644 index 0000000..17550fa --- /dev/null +++ b/src/java/com/samskivert/depot/Stats.java @@ -0,0 +1,138 @@ +// +// $Id$ +// +// Depot library - a Java relational persistence library +// Copyright (C) 2006-2008 Michael Bayne and Pär Winzell +// +// This library is free software; you can redistribute it and/or modify it +// under the terms of the GNU Lesser General Public License as published +// by the Free Software Foundation; either version 2.1 of the License, or +// (at your option) any later version. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + +package com.samskivert.depot; + +import com.samskivert.util.Histogram; + +import static com.samskivert.depot.Log.log; + +/** + * Manages Depot performance statistics. + */ +public class Stats +{ + /** + * An immutable class used to report statistics on repository activity. Statistics are tracked + * from the start of the VM and are never reset. + */ + public static class Snapshot + { + /** The total number of queries and modifiers executed. */ + public final int totalOps; + + /** The total number of milliseconds spent waiting for a JDBC connection. */ + public final long connectionWaitTime; + + /** The total number of collection queries that were loaded from the cache. */ + public final int cachedQueries; + + /** The total number of collection queries that were loaded from the database. */ + public final int uncachedQueries; + + /** The number of record loads (individual or as part of a collection query) that were + * loaded from the cache. */ + public final long cachedRecords; + + /** The number of record loads (individual or as part of a collection query) that were + * loaded from the database. */ + public final long uncachedRecords; + + /** A histogram of query durations (with 500ms buckets). */ + public final Histogram queryHisto; + + /** The total number of milliseconds spent executing queries. */ + public final long queryTime; + + /** A histogram of modifier durations (with 500ms buckets). */ + public final Histogram modifierHisto; + + /** The total number of milliseconds spent executing modifiers. */ + public final long modifierTime; + + /** Creates a stats instance. */ + protected Snapshot (int totalOps, long connectionWaitTime, + int cachedQueries, int uncachedQueries, + int cachedRecords, int uncachedRecords, + Histogram queryHisto, long queryTime, + Histogram modifierHisto, long modifierTime) + { + this.totalOps = totalOps; + this.connectionWaitTime = connectionWaitTime; + this.cachedQueries = cachedQueries; + this.uncachedQueries = uncachedQueries; + this.cachedRecords = cachedRecords; + this.uncachedRecords = uncachedRecords; + this.queryHisto = queryHisto; + this.queryTime = queryTime; + this.modifierHisto = modifierHisto; + this.modifierTime = modifierTime; + } + } + + public synchronized Snapshot getSnapshot () + { + return new Snapshot(_totalOps, _connectionWaitTime, + _cachedQueries, _uncachedQueries, _cachedRecords, _uncachedRecords, + _readHisto.clone(), _readTime, _writeHisto.clone(), _writeTime); + } + + public synchronized void noteOp ( + boolean isReadOnly, long preConnect, long preInvoke, long postInvoke) + { + _totalOps++; + _connectionWaitTime += (preInvoke - preConnect) / 1000L; + + long opTime = (postInvoke - preInvoke) / 1000L; + if (opTime > Integer.MAX_VALUE) { + log.warning("ZOMG! A database operation took " + opTime + "ms to complete!"); + opTime = Integer.MAX_VALUE; + } + + if (isReadOnly) { + _readTime += opTime; + _readHisto.addValue((int)opTime); + } else { + _writeTime += opTime; + _writeHisto.addValue((int)opTime); + } + } + + public synchronized void noteQuery (int cachedQueries, int uncachedQueries, + int cachedRecords, int uncachedRecords) + { + _cachedQueries += cachedQueries; + _uncachedQueries += uncachedQueries; + _cachedRecords += cachedRecords; + _uncachedRecords += uncachedRecords; + } + + protected int _totalOps; + protected long _connectionWaitTime; + + protected Histogram _readHisto = new Histogram(0, 500, 20); + protected long _readTime; + + protected Histogram _writeHisto = new Histogram(0, 500, 20); + protected long _writeTime; + + protected int _cachedQueries, _uncachedQueries; + protected int _cachedRecords, _uncachedRecords; +} diff --git a/src/java/com/samskivert/depot/clause/SelectClause.java b/src/java/com/samskivert/depot/clause/SelectClause.java index 5746d45..abb3c89 100644 --- a/src/java/com/samskivert/depot/clause/SelectClause.java +++ b/src/java/com/samskivert/depot/clause/SelectClause.java @@ -195,6 +195,32 @@ public class SelectClause extends QueryClause builder.visit(this); } + @Override // from Object + public String toString () + { + StringBuilder builder = new StringBuilder(); + builder.append("(where=").append(_where); + if (_fromOverride != null) { + builder.append(", from=").append(_fromOverride); + } + if (!_joinClauses.isEmpty()) { + builder.append(", join=").append(_joinClauses); + } + if (_orderBy != null) { + builder.append(", orderBy=").append(_orderBy); + } + if (_groupBy != null) { + builder.append(", groupBy=").append(_groupBy); + } + if (_limit != null) { + builder.append(", limit=").append(_limit); + } + if (_forUpdate != null) { + builder.append(", forUpdate=").append(_forUpdate); + } + return builder.append(")").toString(); + } + /** Persistent class fields mapped to field override clauses. */ protected Map _disMap = Maps.newHashMap(); diff --git a/src/java/com/samskivert/depot/clause/Where.java b/src/java/com/samskivert/depot/clause/Where.java index d088f42..05bc7b2 100644 --- a/src/java/com/samskivert/depot/clause/Where.java +++ b/src/java/com/samskivert/depot/clause/Where.java @@ -85,6 +85,12 @@ public class Where extends WhereClause _condition.addClasses(classSet); } + @Override // from Object + public String toString () + { + return String.valueOf(_condition); + } + protected static SQLExpression toCondition (ColumnExp[] columns, Comparable[] values) { SQLExpression[] comparisons = new SQLExpression[columns.length]; diff --git a/src/java/com/samskivert/depot/expression/ColumnExp.java b/src/java/com/samskivert/depot/expression/ColumnExp.java index 9cbe386..0a6f118 100644 --- a/src/java/com/samskivert/depot/expression/ColumnExp.java +++ b/src/java/com/samskivert/depot/expression/ColumnExp.java @@ -59,6 +59,12 @@ public class ColumnExp return _pField; } + @Override // from Object + public String toString () + { + return "\"" + _pField + "\""; // TODO: qualify with record name and be uber verbose? + } + /** The table that hosts the column we reference, or null. */ protected final Class _pClass; diff --git a/src/java/com/samskivert/depot/expression/ValueExp.java b/src/java/com/samskivert/depot/expression/ValueExp.java index cfbfe9e..9b6412d 100644 --- a/src/java/com/samskivert/depot/expression/ValueExp.java +++ b/src/java/com/samskivert/depot/expression/ValueExp.java @@ -51,6 +51,12 @@ public class ValueExp return _value; } + @Override // from Object + public String toString () + { + return (_value instanceof Number) ? String.valueOf(_value) : ("'" + _value + "'"); + } + /** The value to be bound to the SQL parameters. */ protected Object _value; } diff --git a/src/java/com/samskivert/depot/operator/Conditionals.java b/src/java/com/samskivert/depot/operator/Conditionals.java index 1366ab0..0a51919 100644 --- a/src/java/com/samskivert/depot/operator/Conditionals.java +++ b/src/java/com/samskivert/depot/operator/Conditionals.java @@ -69,6 +69,12 @@ public abstract class Conditionals { } + @Override // from Object + public String toString () + { + return "IsNull(" + _column + ")"; + } + protected ColumnExp _column; } @@ -240,6 +246,20 @@ public abstract class Conditionals _column.addClasses(classSet); } + @Override // from Object + public String toString () + { + StringBuilder builder = new StringBuilder(); + builder.append(_column).append(" in ("); + for (int ii = 0; ii < _values.length; ii++) { + if (ii > 0) { + builder.append(", "); + } + builder.append(_values[ii]); + } + return builder.append(")").toString(); + } + protected ColumnExp _column; protected Comparable[] _values; } @@ -286,6 +306,12 @@ public abstract class Conditionals return _clause; } + @Override // from Object + public String toString () + { + return "Exists(" + _clause + ")"; + } + protected SelectClause _clause; } @@ -329,6 +355,12 @@ public abstract class Conditionals { } + @Override // from Object + public String toString () + { + return "FullText(" + _name + "=" + _query + ")"; + } + protected Class _pClass; protected String _name; protected String _query; diff --git a/src/java/com/samskivert/depot/operator/Logic.java b/src/java/com/samskivert/depot/operator/Logic.java index 655ff8b..67e0720 100644 --- a/src/java/com/samskivert/depot/operator/Logic.java +++ b/src/java/com/samskivert/depot/operator/Logic.java @@ -49,7 +49,7 @@ public abstract class Logic @Override public String operator() { - return "or"; + return " or "; } } @@ -70,7 +70,7 @@ public abstract class Logic @Override public String operator() { - return "and"; + return " and "; } } @@ -102,6 +102,12 @@ public abstract class Logic _condition.addClasses(classSet); } + @Override // from Object + public String toString () + { + return "Not(" + _condition + ")"; + } + protected SQLExpression _condition; } } diff --git a/src/java/com/samskivert/depot/operator/SQLOperator.java b/src/java/com/samskivert/depot/operator/SQLOperator.java index 38a6cba..1b1731c 100644 --- a/src/java/com/samskivert/depot/operator/SQLOperator.java +++ b/src/java/com/samskivert/depot/operator/SQLOperator.java @@ -69,6 +69,19 @@ public interface SQLOperator extends SQLExpression */ public abstract String operator (); + @Override // from Object + public String toString () + { + StringBuilder builder = new StringBuilder("("); + for (SQLExpression condition : _conditions) { + if (builder.length() > 1) { + builder.append(operator()); + } + builder.append(condition); + } + return builder.append(")").toString(); + } + protected SQLExpression[] _conditions; } @@ -116,6 +129,12 @@ public interface SQLOperator extends SQLExpression return _rhs; } + @Override // from Object + public String toString () + { + return "(" + _lhs + operator() + _rhs + ")"; + } + protected SQLExpression _lhs; protected SQLExpression _rhs; }