You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lens.apache.org by jd...@apache.org on 2015/03/30 12:57:42 UTC
[2/2] incubator-lens git commit: LENS-468 : Connection leak in
JDBCDriver.estimate path
LENS-468 : Connection leak in JDBCDriver.estimate path
Project: http://git-wip-us.apache.org/repos/asf/incubator-lens/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-lens/commit/56c58130
Tree: http://git-wip-us.apache.org/repos/asf/incubator-lens/tree/56c58130
Diff: http://git-wip-us.apache.org/repos/asf/incubator-lens/diff/56c58130
Branch: refs/heads/master
Commit: 56c5813024759dfa4c143f1a940442390360f55b
Parents: adf47a6
Author: jdhok <ja...@inmobi.com>
Authored: Mon Mar 30 16:21:51 2015 +0530
Committer: jdhok <ja...@inmobi.com>
Committed: Mon Mar 30 16:21:51 2015 +0530
----------------------------------------------------------------------
.../org/apache/lens/driver/jdbc/JDBCDriver.java | 30 +++++++++++++-------
.../apache/lens/driver/jdbc/TestJdbcDriver.java | 19 +++++++++++++
2 files changed, 39 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/56c58130/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java
----------------------------------------------------------------------
diff --git a/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java b/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java
index 121b56b..92f7b96 100644
--- a/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java
+++ b/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java
@@ -636,13 +636,9 @@ public class JDBCDriver implements LensDriver {
DEFAULT_JDBC_VALIDATE_THROUGH_PREPARE);
if (validateThroughPrepare) {
PreparedStatement stmt = null;
- try {
- // Estimate queries need to get connection from estimate pool to make sure
- // we are not blocked by data queries.
- stmt = prepareInternal(pContext, getEstimateConnection(), true, "validate-");
- } catch (SQLException e) {
- throw new LensException(e);
- }
+ // Estimate queries need to get connection from estimate pool to make sure
+ // we are not blocked by data queries.
+ stmt = prepareInternal(pContext, true, true, "validate-");
if (stmt != null) {
try {
stmt.close();
@@ -728,12 +724,23 @@ public class JDBCDriver implements LensDriver {
throw new NullPointerException("Null driver query for " + pContext.getUserQuery());
}
checkConfigured();
- return prepareInternal(pContext, getConnection(), false, "prepare-");
+ return prepareInternal(pContext, false, false, "prepare-");
}
- private PreparedStatement prepareInternal(AbstractQueryContext pContext, final Connection conn,
- boolean checkConfigured, String metricCallStack) throws LensException {
+ /**
+ * Prepare statment on the database server
+ * @param pContext query context
+ * @param calledForEstimate set this to true if this call will use the estimate connection pool
+ * @param checkConfigured set this to true if this call needs to check whether JDBC driver is configured
+ * @param metricCallStack stack for metrics API
+ * @return prepared statement
+ * @throws LensException
+ */
+ private PreparedStatement prepareInternal(AbstractQueryContext pContext,
+ boolean calledForEstimate,
+ boolean checkConfigured,
+ String metricCallStack) throws LensException {
// Caller might have already verified configured status and driver query, so we don't have
// to do this check twice. Caller must set checkConfigured to false in that case.
if (checkConfigured) {
@@ -750,8 +757,11 @@ public class JDBCDriver implements LensDriver {
sqlRewriteGauge.markSuccess();
MethodMetricsContext jdbcPrepareGauge = MethodMetricsFactory.createMethodGauge(pContext.getDriverConf(this), true,
metricCallStack + JDBC_PREPARE_GAUGE);
+
PreparedStatement stmt = null;
+ Connection conn = null;
try {
+ conn = calledForEstimate ? getEstimateConnection() : getConnection();
stmt = conn.prepareStatement(rewrittenQuery);
if (stmt.getWarnings() != null) {
throw new LensException(stmt.getWarnings());
http://git-wip-us.apache.org/repos/asf/incubator-lens/blob/56c58130/lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java
----------------------------------------------------------------------
diff --git a/lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java b/lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java
index 550a468..09bc6a5 100644
--- a/lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java
+++ b/lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java
@@ -265,6 +265,25 @@ public class TestJdbcDriver {
Assert.assertEquals(cost.getEstimatedExecTimeMillis(), 0);
Assert.assertEquals(cost.getEstimatedResourceUsage(), 0.0);
Assert.assertNotNull(ctx.getFinalDriverQuery(driver));
+
+ // Test connection leak for estimate
+ final int maxEstimateConnections =
+ driver.getEstimateConnectionConf().getInt(JDBCDriverConfConstants.JDBC_POOL_MAX_SIZE, 50);
+ for (int i = 0; i < maxEstimateConnections + 10; i++) {
+ try {
+ LOG.info("Iteration#" + (i + 1));
+ String query = i > maxEstimateConnections ? "SELECT * FROM estimate_test" : "CREATE TABLE FOO(ID INT)";
+ ExplainQueryContext context = createExplainContext(query, baseConf);
+ cost = driver.estimate(context);
+ } catch (LensException exc) {
+ Throwable th = exc.getCause();
+ while (th != null) {
+ assertFalse(th instanceof SQLException);
+ th = th.getCause();
+ }
+ }
+ }
+
}
/**