Repository: phoenix Updated Branches: refs/heads/master 8adcc87a6 -> 3a4429f5f
PHOENIX-1172 Prevent lock contention in ConnectionQueryServicesImpl. Fix exception handling. (Samarth Jain) Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/3a4429f5 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/3a4429f5 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/3a4429f5 Branch: refs/heads/master Commit: 3a4429f5fc5e7018153fb3084b27317ed3eb0d67 Parents: 8adcc87 Author: James Taylor <jtay...@salesforce.com> Authored: Sun Aug 17 13:14:56 2014 -0700 Committer: James Taylor <jtay...@salesforce.com> Committed: Sun Aug 17 13:28:48 2014 -0700 ---------------------------------------------------------------------- .../query/ConnectionQueryServicesImpl.java | 57 +++++++++++--------- 1 file changed, 33 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/3a4429f5/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java index dedef15..67fb40f 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java @@ -157,15 +157,18 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement private final StatsManager statsManager; // Cache the latest meta data here for future connections - @GuardedBy("latestMetaDataLock") - private PMetaData latestMetaData; + // writes guarded by "latestMetaDataLock" + private volatile PMetaData latestMetaData; private final Object latestMetaDataLock = new Object(); // Lowest HBase version on the cluster. private int lowestClusterHBaseVersion = Integer.MAX_VALUE; private boolean hasInvalidIndexConfiguration = false; + + @GuardedBy("connectionCountLock") private int connectionCount = 0; - + private final Object connectionCountLock = new Object(); + private HConnection connection; private volatile boolean initialized; @@ -173,7 +176,8 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement private volatile boolean closed; private volatile SQLException initializationException; - protected ConcurrentMap<SequenceKey,Sequence> sequenceMap = Maps.newConcurrentMap(); + // setting this member variable guarded by "connectionCountLock" + private volatile ConcurrentMap<SequenceKey,Sequence> sequenceMap = Maps.newConcurrentMap(); private KeyValueBuilder kvBuilder; private PMetaData newEmptyMetaData() { @@ -525,11 +529,11 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement @Override public PhoenixConnection connect(String url, Properties info) throws SQLException { checkClosed(); - synchronized (latestMetaDataLock) { - throwConnectionClosedIfNullMetaData(); - latestMetaDataLock.notifyAll(); - return new PhoenixConnection(this, url, info, latestMetaData); + PMetaData metadata = latestMetaData; + if (metadata == null) { + throwConnectionClosedException(); } + return new PhoenixConnection(this, url, info, metadata); } @@ -1291,11 +1295,11 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement MetaDataUtil.VIEW_INDEX_TABLE_PREFIX_BYTES.length, physicalIndexTableName.length-MetaDataUtil.VIEW_INDEX_TABLE_PREFIX_BYTES.length); try { - synchronized (latestMetaDataLock) { - throwConnectionClosedIfNullMetaData(); - table = latestMetaData.getTable(new PTableKey(tenantId, name)); - latestMetaDataLock.notifyAll(); + PMetaData metadata = latestMetaData; + if (metadata == null) { + throwConnectionClosedException(); } + table = metadata.getTable(new PTableKey(tenantId, name)); if (table.getTimeStamp() >= timestamp) { // Table in cache is newer than client timestamp which shouldn't be the case throw new TableNotFoundException(table.getSchemaName().getString(), table.getTableName().getString()); } @@ -1501,7 +1505,6 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement return null; } checkClosed(); - SQLException sqlE = null; PhoenixConnection metaConnection = null; try { openConnection(); @@ -1542,22 +1545,26 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement metaConnection = addColumnsIfNotExists(metaConnection, PhoenixDatabaseMetaData.SEQUENCE_TABLE_NAME, MetaDataProtocol.MIN_SYSTEM_TABLE_TIMESTAMP, newColumns); } - } catch (SQLException e) { - sqlE = e; + } catch (Exception e) { + if (e instanceof SQLException) { + initializationException = (SQLException)e; + } else { + // wrap every other exception into a SQLException + initializationException = new SQLException(e); + } } finally { try { if (metaConnection != null) metaConnection.close(); } catch (SQLException e) { - if (sqlE != null) { - sqlE.setNextException(e); + if (initializationException != null) { + initializationException.setNextException(e); } else { - sqlE = e; + initializationException = e; } } finally { try { - if (sqlE != null) { - initializationException = sqlE; - throw sqlE; + if (initializationException != null) { + throw initializationException; } } finally { initialized = true; @@ -1975,14 +1982,16 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement } @Override - public synchronized void addConnection(PhoenixConnection connection) throws SQLException { - connectionCount++; + public void addConnection(PhoenixConnection connection) throws SQLException { + synchronized (connectionCountLock) { + connectionCount++; + } } @Override public void removeConnection(PhoenixConnection connection) throws SQLException { ConcurrentMap<SequenceKey,Sequence> formerSequenceMap = null; - synchronized(this) { + synchronized (connectionCountLock) { if (--connectionCount == 0) { if (!this.sequenceMap.isEmpty()) { formerSequenceMap = this.sequenceMap;