You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tomee.apache.org by rm...@apache.org on 2016/06/07 15:17:51 UTC
[2/2] tomee git commit: TOMEE-1832 avoid useless locking in dbcp2 +
TOMEE-1833 ExceptionSelector to activate or not failover
TOMEE-1832 avoid useless locking in dbcp2 + TOMEE-1833 ExceptionSelector to activate or not failover
Project: http://git-wip-us.apache.org/repos/asf/tomee/repo
Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/0175ecd2
Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/0175ecd2
Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/0175ecd2
Branch: refs/heads/master
Commit: 0175ecd2e7807ee45fcfdeebf6c6dffa6f69c1bf
Parents: 28a95b7
Author: Romain manni-Bucau <rm...@gmail.com>
Authored: Tue Jun 7 17:17:31 2016 +0200
Committer: Romain manni-Bucau <rm...@gmail.com>
Committed: Tue Jun 7 17:17:31 2016 +0200
----------------------------------------------------------------------
.../assembler/classic/util/ServiceInfos.java | 32 +-
.../resource/jdbc/dbcp/BasicDataSource.java | 185 +-
.../resource/jdbc/router/FailOverRouter.java | 66 +-
.../jdbc/FailOverRouterErrorHandlerTest.java | 8 -
.../FailOverRouterExceptionSelectorTest.java | 2198 ++++++++++++++++++
5 files changed, 2336 insertions(+), 153 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tomee/blob/0175ecd2/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/util/ServiceInfos.java
----------------------------------------------------------------------
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/util/ServiceInfos.java b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/util/ServiceInfos.java
index ecc0c59..41ac9ce 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/util/ServiceInfos.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/util/ServiceInfos.java
@@ -168,24 +168,14 @@ public final class ServiceInfos {
final List<Object> val = new ArrayList<>(elt.length);
for (final String e : elt) {
if (!e.trim().isEmpty()) {
- val.add(resolve(services, e.startsWith("$") ? e.substring(1) : e));
+ val.add(e.startsWith("@") ? lookup(e) : resolve(services, e.startsWith("$") ? e.substring(1) : e));
}
}
serviceRecipe.setProperty(key, val);
} else if (valueStr.startsWith("$")) {
serviceRecipe.setProperty(key, resolve(services, valueStr.substring(1)));
} else if (valueStr.startsWith("@")) {
- final Context jndiContext = SystemInstance.get().getComponent(ContainerSystem.class).getJNDIContext();
- try {
- serviceRecipe.setProperty(key, jndiContext.lookup(JndiConstants.OPENEJB_RESOURCE_JNDI_PREFIX + valueStr.substring(1)));
- } catch (final NamingException e) {
- try {
- serviceRecipe.setProperty(key, jndiContext.lookup(valueStr.substring(1)));
- } catch (final NamingException e1) {
- Logger.getInstance(LogCategory.OPENEJB, ServiceInfos.class).warning("Value " + valueStr + " starting with @ but doesn't point to an existing resource, using raw value");
- serviceRecipe.setProperty(key, value);
- }
- }
+ serviceRecipe.setProperty(key, lookup(value));
} else {
serviceRecipe.setProperty(key, value);
}
@@ -195,6 +185,24 @@ public final class ServiceInfos {
}
}
+ private static Object lookup(final Object value) {
+ final String name = String.valueOf(value).substring(1);
+ final Context jndiContext = SystemInstance.get().getComponent(ContainerSystem.class).getJNDIContext();
+ Object lookup;
+ try {
+ lookup = jndiContext.lookup(JndiConstants.OPENEJB_RESOURCE_JNDI_PREFIX + name);
+ } catch (final NamingException e) {
+ try {
+ lookup = jndiContext.lookup(name);
+ } catch (final NamingException e1) {
+ Logger.getInstance(LogCategory.OPENEJB, ServiceInfos.class)
+ .warning("Value " + name + " starting with @ but doesn't point to an existing resource, using raw value");
+ lookup = value;
+ }
+ }
+ return lookup;
+ }
+
public interface Factory {
Object newInstance(final Class<?> clazz) throws Exception;
}
http://git-wip-us.apache.org/repos/asf/tomee/blob/0175ecd2/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicDataSource.java
----------------------------------------------------------------------
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicDataSource.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicDataSource.java
index 995221c..37096b3 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicDataSource.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/BasicDataSource.java
@@ -29,23 +29,21 @@ import org.apache.openejb.resource.jdbc.IsolationLevels;
import org.apache.openejb.resource.jdbc.plugin.DataSourcePlugin;
import org.apache.openejb.util.reflection.Reflections;
+import javax.sql.CommonDataSource;
+import javax.sql.DataSource;
+import javax.sql.XADataSource;
import java.io.File;
import java.io.ObjectStreamException;
import java.io.Serializable;
import java.sql.SQLException;
import java.sql.SQLFeatureNotSupportedException;
import java.util.Properties;
-import java.util.concurrent.locks.ReentrantLock;
import java.util.logging.Logger;
-import javax.sql.CommonDataSource;
-import javax.sql.DataSource;
-import javax.sql.XADataSource;
@SuppressWarnings({"UnusedDeclaration"})
public class BasicDataSource extends org.apache.commons.dbcp2.BasicDataSource implements Serializable {
- private final ReentrantLock lock = new ReentrantLock();
-
- private Logger logger;
+ private volatile Logger logger;
+ private volatile DataSource dsRef = null;
/**
* The password codec to be used to retrieve the plain text password from a
@@ -105,14 +103,8 @@ public class BasicDataSource extends org.apache.commons.dbcp2.BasicDataSource im
*
* @return the password codec class
*/
- public String getPasswordCipher() {
- final ReentrantLock l = lock;
- l.lock();
- try {
- return this.passwordCipher;
- } finally {
- l.unlock();
- }
+ public synchronized String getPasswordCipher() {
+ return this.passwordCipher;
}
/**
@@ -122,121 +114,61 @@ public class BasicDataSource extends org.apache.commons.dbcp2.BasicDataSource im
*
* @param passwordCipher password codec value
*/
- public void setPasswordCipher(final String passwordCipher) {
- final ReentrantLock l = lock;
- l.lock();
- try {
- this.passwordCipher = passwordCipher;
- } finally {
- l.unlock();
- }
+ public synchronized void setPasswordCipher(final String passwordCipher) {
+ this.passwordCipher = passwordCipher;
}
@Override
- public void setPassword(final String password) {
- final ReentrantLock l = lock;
- l.lock();
- try {
- // keep the encrypted value if it's encrypted
- this.initialPassword = password;
- super.setPassword(password);
- } finally {
- l.unlock();
- }
+ public synchronized void setPassword(final String password) {
+ // keep the encrypted value if it's encrypted
+ this.initialPassword = password;
+ super.setPassword(password);
}
- public String getUserName() {
- final ReentrantLock l = lock;
- l.lock();
- try {
- return super.getUsername();
- } finally {
- l.unlock();
- }
+ public synchronized String getUserName() {
+ return super.getUsername();
}
- public void setUserName(final String string) {
- final ReentrantLock l = lock;
- l.lock();
- try {
- super.setUsername(string);
- } finally {
- l.unlock();
- }
+ public synchronized void setUserName(final String string) {
+ super.setUsername(string);
}
- public String getJdbcDriver() {
- final ReentrantLock l = lock;
- l.lock();
- try {
- return super.getDriverClassName();
- } finally {
- l.unlock();
- }
+ public synchronized String getJdbcDriver() {
+ return super.getDriverClassName();
}
- public void setJdbcDriver(final String string) {
- final ReentrantLock l = lock;
- l.lock();
- try {
- super.setDriverClassName(string);
- } finally {
- l.unlock();
- }
+ public synchronized void setJdbcDriver(final String string) {
+ super.setDriverClassName(string);
}
- public String getJdbcUrl() {
- final ReentrantLock l = lock;
- l.lock();
- try {
- return super.getUrl();
- } finally {
- l.unlock();
- }
+ public synchronized String getJdbcUrl() {
+ return super.getUrl();
}
public void setJdbcUrl(final String string) {
- final ReentrantLock l = lock;
- l.lock();
- try {
- super.setUrl(string);
- } finally {
- l.unlock();
- }
+ super.setUrl(string);
}
- public void setDefaultTransactionIsolation(final String s) {
- final ReentrantLock l = lock;
- l.lock();
- try {
- if (s == null || s.equals("")) {
- return;
- }
- final int level = IsolationLevels.getIsolationLevel(s);
- super.setDefaultTransactionIsolation(level);
- } finally {
- l.unlock();
+ public synchronized void setDefaultTransactionIsolation(final String s) {
+ if (s == null || s.equals("")) {
+ return;
}
+ final int level = IsolationLevels.getIsolationLevel(s);
+ super.setDefaultTransactionIsolation(level);
}
- public void setMaxWait(final int maxWait) {
- final ReentrantLock l = lock;
- l.lock();
- try {
- super.setMaxWaitMillis((long) maxWait);
- } finally {
- l.unlock();
- }
+ public synchronized void setMaxWait(final int maxWait) {
+ super.setMaxWaitMillis((long) maxWait);
}
@Override
protected DataSource createDataSource() throws SQLException {
- final ReentrantLock l = lock;
- l.lock();
- try {
- final Object dataSource = Reflections.get(this, "dataSource");
- if (dataSource != null) {
- return DataSource.class.cast(dataSource);
+ if (dsRef != null) {
+ return dsRef;
+ }
+ synchronized (this) {
+ if (dsRef != null) {
+ return dsRef;
}
// check password codec if available
@@ -265,7 +197,7 @@ public class BasicDataSource extends org.apache.commons.dbcp2.BasicDataSource im
// create the data source
if (helper == null || !helper.enableUserDirHack()) {
try {
- return super.createDataSource();
+ super.createDataSource();
} catch (final Throwable e) {
throw toSQLException(e);
}
@@ -278,17 +210,15 @@ public class BasicDataSource extends org.apache.commons.dbcp2.BasicDataSource im
final File base = SystemInstance.get().getBase().getDirectory();
systemProperties.setProperty("user.dir", base.getAbsolutePath());
try {
- return super.createDataSource();
+ super.createDataSource();
} catch (final Throwable e) {
throw toSQLException(e);
}
} finally {
systemProperties.setProperty("user.dir", userDir);
}
-
}
- } finally {
- l.unlock();
+ return dsRef = DataSource.class.cast(Reflections.get(this, "dataSource"));
}
}
@@ -299,21 +229,17 @@ public class BasicDataSource extends org.apache.commons.dbcp2.BasicDataSource im
return new SQLException("Failed to create DataSource", e);
}
- public void close() throws SQLException {
- //TODO - Prevent unuathorized call
- final ReentrantLock l = lock;
- l.lock();
+ @Override
+ public synchronized void close() throws SQLException {
try {
- try {
- unregisterMBean();
- } catch (final Exception ignored) {
- // no-op
- }
-
- super.close();
- } finally {
- l.unlock();
+ unregisterMBean();
+ } catch (final Exception ignored) {
+ // no-op
}
+
+ super.close();
+ dsRef = null;
+ logger = null;
}
private void unregisterMBean() {
@@ -322,20 +248,19 @@ public class BasicDataSource extends org.apache.commons.dbcp2.BasicDataSource im
}
}
+ @Override
public Logger getParentLogger() throws SQLFeatureNotSupportedException {
- final ReentrantLock l = lock;
- l.lock();
try {
-
if (null == this.logger) {
- this.logger = (Logger) Reflections.invokeByReflection(createDataSource(), "getParentLogger", new Class<?>[0], null);
+ synchronized (this) {
+ if (null == this.logger) {
+ this.logger = (Logger) Reflections.invokeByReflection(createDataSource(), "getParentLogger", new Class<?>[0], null);
+ }
+ }
}
-
return this.logger;
} catch (final Throwable e) {
throw new SQLFeatureNotSupportedException();
- } finally {
- l.unlock();
}
}
http://git-wip-us.apache.org/repos/asf/tomee/blob/0175ecd2/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/router/FailOverRouter.java
----------------------------------------------------------------------
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/router/FailOverRouter.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/router/FailOverRouter.java
index 0ab5b35..2556d42 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/router/FailOverRouter.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/router/FailOverRouter.java
@@ -32,6 +32,7 @@ import java.lang.reflect.InvocationHandler;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
+import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@@ -47,6 +48,7 @@ public class FailOverRouter extends AbstractRouter {
public static final String DEFAULT_STRATEGY = "default";
+ private ExceptionSelector exceptionSelectorRuntime;
private ErrorHandler errorHandlerRuntime;
private Strategy strategyRuntime;
private DataSource facade;
@@ -84,6 +86,21 @@ public class FailOverRouter extends AbstractRouter {
this.strategyRuntime = strategy;
}
+ public void setExceptionSelectorInstance(final ExceptionSelector selector) {
+ exceptionSelectorRuntime = selector;
+ }
+
+ public void setExceptionSelector(final String selector) {
+ try {
+ exceptionSelectorRuntime = "mysql".equalsIgnoreCase(selector) ?
+ new MySQLExceptionSelector() :
+ ExceptionSelector.class.cast(Thread.currentThread().getContextClassLoader()
+ .loadClass(selector.trim()).newInstance());
+ } catch (final InstantiationException | IllegalAccessException | ClassNotFoundException e) {
+ throw new IllegalArgumentException(e);
+ }
+ }
+
public void setErrorHandlerInstance(final ErrorHandler errorHandler) {
errorHandlerRuntime = errorHandler;
}
@@ -214,7 +231,7 @@ public class FailOverRouter extends AbstractRouter {
private void initFacade() {
facade = DataSource.class.cast(Proxy.newProxyInstance(
Thread.currentThread().getContextClassLoader(),
- new Class<?>[]{DataSource.class}, new FacadeHandler(dataSources, strategyRuntime, errorHandlerRuntime)));
+ new Class<?>[]{DataSource.class}, new FacadeHandler(dataSources, strategyRuntime, errorHandlerRuntime, exceptionSelectorRuntime)));
}
public Collection<DataSourceHolder> getDataSources() {
@@ -230,15 +247,18 @@ public class FailOverRouter extends AbstractRouter {
private static class FacadeHandler implements InvocationHandler {
private static final TransactionSynchronizationRegistry SYNCHRONIZATION_REGISTRY = SystemInstance.get().getComponent(TransactionSynchronizationRegistry.class);
+ private final TransactionManager transactionManager;
private final Collection<DataSourceHolder> delegates;
private final Strategy strategy;
- private final TransactionManager transactionManager;
private final ErrorHandler handler;
+ private final ExceptionSelector selector;
- public FacadeHandler(final Collection<DataSourceHolder> dataSources, final Strategy strategy, final ErrorHandler handler) {
+ public FacadeHandler(final Collection<DataSourceHolder> dataSources, final Strategy strategy,
+ final ErrorHandler handler, final ExceptionSelector selector) {
this.delegates = dataSources;
this.strategy = strategy;
this.handler = handler;
+ this.selector = selector;
this.transactionManager = OpenEJB.getTransactionManager();
}
@@ -290,6 +310,14 @@ public class FailOverRouter extends AbstractRouter {
break;
}
} catch (final InvocationTargetException ite) {
+ final Throwable cause = ite.getTargetException();
+ if (selector != null && !selector.shouldFailover(cause)) {
+ if (failed != null) {
+ handler.onError(failed, ds);
+ }
+ throw cause;
+ }
+
if (handler != null) {
if (failed == null) {
failed = new HashMap<>();
@@ -315,6 +343,38 @@ public class FailOverRouter extends AbstractRouter {
}
}
+ public interface ExceptionSelector {
+ boolean shouldFailover(final Throwable sqle);
+ }
+
+ public abstract class SQLExceptionSelector implements ExceptionSelector {
+ @Override
+ public boolean shouldFailover(final Throwable sqle) {
+ return SQLException.class.isInstance(sqle) && shouldFailover(SQLException.class.cast(sqle));
+ }
+
+ abstract boolean shouldFailover(final SQLException sqle);
+ }
+
+ public class MySQLExceptionSelector extends SQLExceptionSelector {
+ private final Class<?> communicationException;
+
+ public MySQLExceptionSelector() {
+ try {
+ communicationException = Thread.currentThread().getContextClassLoader().loadClass("com.mysql.jdbc.CommunicationsException");
+ } catch (final ClassNotFoundException e) {
+ throw new IllegalArgumentException("com.mysql.jdbc.CommunicationsException not available, please use another ExceptionSelector");
+ }
+ }
+
+ @Override
+ public boolean shouldFailover(final SQLException ex) {
+ final String sqlState = ex.getSQLState();
+ return (sqlState != null && sqlState.startsWith("08")) ||
+ communicationException.isInstance(ex);
+ }
+ }
+
public interface ErrorHandler {
void onError(final Map<String, Throwable> errorByFailingDataSource, final DataSourceHolder finallyUsedOrNull);
}
http://git-wip-us.apache.org/repos/asf/tomee/blob/0175ecd2/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/FailOverRouterErrorHandlerTest.java
----------------------------------------------------------------------
diff --git a/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/FailOverRouterErrorHandlerTest.java b/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/FailOverRouterErrorHandlerTest.java
index f638c04..38ed9e7 100644
--- a/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/FailOverRouterErrorHandlerTest.java
+++ b/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/FailOverRouterErrorHandlerTest.java
@@ -57,10 +57,8 @@ import java.sql.Struct;
import java.sql.Time;
import java.sql.Timestamp;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.Calendar;
import java.util.Collection;
-import java.util.Iterator;
import java.util.Map;
import java.util.Properties;
import java.util.concurrent.CopyOnWriteArraySet;
@@ -108,12 +106,6 @@ public class FailOverRouterErrorHandlerTest {
}
}
- private void rotate() {
- final Iterator<FailOverRouter.DataSourceHolder> it = router.getDataSources().iterator();
- final FailOverRouter.DataSourceHolder ds1 = it.next();
- router.updateDataSources(Arrays.asList(it.next(), it.next(), ds1));
- }
-
@Configuration
public Properties configuration() {
// datasources