From 94eef043f52995c4d29832f6fc0599f86023d1d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=AB=98=E9=93=81?= Date: Thu, 5 May 2016 17:44:38 +0800 Subject: [PATCH 1/5] handle MBean preRegister method, if exists, unregister. --- .../java/com/alibaba/druid/pool/DruidDataSource.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/alibaba/druid/pool/DruidDataSource.java b/src/main/java/com/alibaba/druid/pool/DruidDataSource.java index 8279d8fe3b..fa36a698c7 100644 --- a/src/main/java/com/alibaba/druid/pool/DruidDataSource.java +++ b/src/main/java/com/alibaba/druid/pool/DruidDataSource.java @@ -2879,8 +2879,15 @@ public class DruidDataSource extends DruidAbstractDataSource @Override public ObjectName preRegister(MBeanServer server, ObjectName name) throws Exception { - //do nothing - //return original name to avoid NullPointerException + if (server != null) { + try { + if (server.isRegistered(name)) { + server.unregisterMBean(name); + } + } catch (Exception ex) { + LOG.warn("DruidDataSource preRegister error", ex); + } + } return name; } From 252c048fbfaac2b1256c3dc0b96fb478de567690 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=AB=98=E9=93=81?= Date: Thu, 5 May 2016 18:15:44 +0800 Subject: [PATCH 2/5] improved mysql sql parser support --- .../mysql/parser/MySqlSelectParser.java | 2 +- .../mysql/parser/MySqlStatementParser.java | 32 +------ .../simplejndi}/DruidDataSourceConverter.java | 0 .../bvt/sql/mysql/MySqlUpdateTest_6.java | 2 +- .../bvt/sql/mysql/MySqlUpdateTest_7.java | 87 +++++++++++++++++++ 5 files changed, 90 insertions(+), 33 deletions(-) rename src/main/java/com/alibaba/druid/{pool => support/simplejndi}/DruidDataSourceConverter.java (100%) create mode 100644 src/test/java/com/alibaba/druid/bvt/sql/mysql/MySqlUpdateTest_7.java diff --git a/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlSelectParser.java b/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlSelectParser.java index 3ff115c917..a95353cf9d 100644 --- a/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlSelectParser.java +++ b/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlSelectParser.java @@ -238,7 +238,7 @@ public class MySqlSelectParser extends SQLSelectParser { return tableSrc; } - private MySqlUpdateStatement parseUpdateStatment() { + protected MySqlUpdateStatement parseUpdateStatment() { MySqlUpdateStatement update = new MySqlUpdateStatement(); lexer.nextToken(); diff --git a/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlStatementParser.java b/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlStatementParser.java index 6f7cfcac31..7ffb1f71ad 100644 --- a/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlStatementParser.java +++ b/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlStatementParser.java @@ -242,37 +242,7 @@ public class MySqlStatementParser extends SQLStatementParser { } public SQLUpdateStatement parseUpdateStatement() { - MySqlUpdateStatement stmt = createUpdateStatement(); - - if (lexer.token() == Token.UPDATE) { - lexer.nextToken(); - - if (identifierEquals(LOW_PRIORITY)) { - lexer.nextToken(); - stmt.setLowPriority(true); - } - - if (identifierEquals(IGNORE)) { - lexer.nextToken(); - stmt.setIgnore(true); - } - - SQLTableSource tableSource = this.exprParser.createSelectParser().parseTableSource(); - stmt.setTableSource(tableSource); - } - - parseUpdateSet(stmt); - - if (lexer.token() == (Token.WHERE)) { - lexer.nextToken(); - stmt.setWhere(this.exprParser.expr()); - } - - stmt.setOrderBy(this.exprParser.parseOrderBy()); - - stmt.setLimit(parseLimit()); - - return stmt; + return new MySqlSelectParser(this.exprParser).parseUpdateStatment(); } protected MySqlUpdateStatement createUpdateStatement() { diff --git a/src/main/java/com/alibaba/druid/pool/DruidDataSourceConverter.java b/src/main/java/com/alibaba/druid/support/simplejndi/DruidDataSourceConverter.java similarity index 100% rename from src/main/java/com/alibaba/druid/pool/DruidDataSourceConverter.java rename to src/main/java/com/alibaba/druid/support/simplejndi/DruidDataSourceConverter.java diff --git a/src/test/java/com/alibaba/druid/bvt/sql/mysql/MySqlUpdateTest_6.java b/src/test/java/com/alibaba/druid/bvt/sql/mysql/MySqlUpdateTest_6.java index 8dc9e63916..495742a0b7 100644 --- a/src/test/java/com/alibaba/druid/bvt/sql/mysql/MySqlUpdateTest_6.java +++ b/src/test/java/com/alibaba/druid/bvt/sql/mysql/MySqlUpdateTest_6.java @@ -30,7 +30,7 @@ import com.alibaba.druid.stat.TableStat.Column; public class MySqlUpdateTest_6 extends MysqlTest { public void test_0() throws Exception { - String sql = "/* 6445c36514623747244871009d62f9/0//sub_order_id:1462374729595/order_id:1462374729594/ */" + String sql = "/* 6445c36514623747244871009d62f9/0//sub_order_id:1234567890/order_id:123456/ */" + "SELECT id, user_id, item_id, sku_id, flag" + " , sellable_quantity, fff03, status, feature, feature_lock" + " , version, gmt_create, gmt_modified, root_id, parent_id, dist_type, dist_id" diff --git a/src/test/java/com/alibaba/druid/bvt/sql/mysql/MySqlUpdateTest_7.java b/src/test/java/com/alibaba/druid/bvt/sql/mysql/MySqlUpdateTest_7.java new file mode 100644 index 0000000000..9d25bac4bc --- /dev/null +++ b/src/test/java/com/alibaba/druid/bvt/sql/mysql/MySqlUpdateTest_7.java @@ -0,0 +1,87 @@ +/* + * Copyright 1999-2101 Alibaba Group Holding Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.alibaba.druid.bvt.sql.mysql; + +import java.util.List; + +import org.junit.Assert; + +import com.alibaba.druid.sql.MysqlTest; +import com.alibaba.druid.sql.SQLUtils; +import com.alibaba.druid.sql.ast.SQLStatement; +import com.alibaba.druid.sql.dialect.mysql.parser.MySqlStatementParser; +import com.alibaba.druid.sql.dialect.mysql.visitor.MySqlSchemaStatVisitor; +import com.alibaba.druid.stat.TableStat; +import com.alibaba.druid.stat.TableStat.Column; + +public class MySqlUpdateTest_7 extends MysqlTest { + + public void test_0() throws Exception { + String sql = "UPDATE ROLLBACK_ON_FAIL TARGET_AFFECT_ROW 1 " + + "`table_3966` AS `table_3966_11` SET `version` = `version` + 3, `gmt_modified` = NOW(), `optype` = ?, `feature` = ? " + + "WHERE `sub_biz_order_id` = ? AND `biz_order_type` = ? AND `id` = ? AND `ti_id` = ? AND `optype` = ? AND `root_id` = ?"; + + MySqlStatementParser parser = new MySqlStatementParser(sql); + List statementList = parser.parseStatementList(); + SQLStatement stmt = statementList.get(0); + print(statementList); + + Assert.assertEquals(1, statementList.size()); + + MySqlSchemaStatVisitor visitor = new MySqlSchemaStatVisitor(); + stmt.accept(visitor); + +// System.out.println("Tables : " + visitor.getTables()); +// System.out.println("fields : " + visitor.getColumns()); +// System.out.println("coditions : " + visitor.getConditions()); +// System.out.println("orderBy : " + visitor.getOrderByColumns()); + + Assert.assertEquals(1, visitor.getTables().size()); + Assert.assertEquals(9, visitor.getColumns().size()); + Assert.assertEquals(6, visitor.getConditions().size()); + + Assert.assertTrue(visitor.getTables().containsKey(new TableStat.Name("table_3966"))); + + Assert.assertTrue(visitor.getColumns().contains(new Column("table_3966", "sub_biz_order_id"))); + Assert.assertTrue(visitor.getColumns().contains(new Column("table_3966", "feature"))); + Assert.assertTrue(visitor.getColumns().contains(new Column("table_3966", "id"))); + + { + String output = SQLUtils.toMySqlString(stmt); + Assert.assertEquals("UPDATE ROLLBACK_ON_FAIL TARGET_AFFECT_ROW 1 `table_3966` `table_3966_11`" + + "\nSET `version` = `version` + 3, `gmt_modified` = NOW(), `optype` = ?, `feature` = ?" + + "\nWHERE `sub_biz_order_id` = ?" + + "\n\tAND `biz_order_type` = ?" + + "\n\tAND `id` = ?" + + "\n\tAND `ti_id` = ?" + + "\n\tAND `optype` = ?" + + "\n\tAND `root_id` = ?", // + output); + } + { + String output = SQLUtils.toMySqlString(stmt, SQLUtils.DEFAULT_LCASE_FORMAT_OPTION); + Assert.assertEquals("update rollback_on_fail target_affect_row 1 `table_3966` `table_3966_11`" + + "\nset `version` = `version` + 3, `gmt_modified` = NOW(), `optype` = ?, `feature` = ?" + + "\nwhere `sub_biz_order_id` = ?" + + "\n\tand `biz_order_type` = ?" + + "\n\tand `id` = ?" + + "\n\tand `ti_id` = ?" + + "\n\tand `optype` = ?" + + "\n\tand `root_id` = ?", // + output); + } + } +} From 8e2b1e3e67b20a5e07edd766de5d39073659f748 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=AB=98=E9=93=81?= Date: Thu, 5 May 2016 18:16:07 +0800 Subject: [PATCH 3/5] change DruidDataSourceConverter package --- .../support/simplejndi/DruidDataSourceConverter.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/alibaba/druid/support/simplejndi/DruidDataSourceConverter.java b/src/main/java/com/alibaba/druid/support/simplejndi/DruidDataSourceConverter.java index d14712441a..15276bfc97 100644 --- a/src/main/java/com/alibaba/druid/support/simplejndi/DruidDataSourceConverter.java +++ b/src/main/java/com/alibaba/druid/support/simplejndi/DruidDataSourceConverter.java @@ -14,13 +14,15 @@ * limitations under the License. */ -package com.alibaba.druid.pool; +package com.alibaba.druid.support.simplejndi; import java.sql.SQLException; import java.util.Properties; import org.osjava.sj.loader.convert.Converter; +import com.alibaba.druid.pool.DruidDataSource; +import com.alibaba.druid.pool.DruidDataSourceFactory; import com.alibaba.druid.support.logging.Log; import com.alibaba.druid.support.logging.LogFactory; @@ -28,7 +30,7 @@ import com.alibaba.druid.support.logging.LogFactory; * 支持simple-jndi *

配置示例:

*
-pgDruidTest/converter=com.alibaba.druid.pool.DruidDataSourceConverter
+pgDruidTest/converter=com.alibaba.druid.support.simplejndi.DruidDataSourceConverter
 pgDruidTest/type=javax.sql.DataSource
 pgDruidTest/driverClassName=org.postgresql.Driver
 pgDruidTest/url=jdbc:postgresql://127.0.0.1:5432/kettleRep
@@ -50,7 +52,7 @@ pgDruidTest/connectionProperties=druid.log.stmt.executableSql=true
  */
 public class DruidDataSourceConverter implements Converter {
 
-    private final static Log      LOG                                      = LogFactory.getLog(DruidDataSourceConverter.class);
+    private final static Log LOG = LogFactory.getLog(DruidDataSourceConverter.class);
 	/**
 	 * 
 	 * @see org.osjava.sj.loader.convert.Converter#convert(java.util.Properties, java.lang.String)

From 3708d093d1ad0aa9af1edbc1e2b5f33c0030798d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E9=AB=98=E9=93=81?= 
Date: Thu, 5 May 2016 18:24:07 +0800
Subject: [PATCH 4/5] fixed testcase

---
 .../resources/com/alibaba/druid/pool/simplejndi/jdbc.properties | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/test/resources/com/alibaba/druid/pool/simplejndi/jdbc.properties b/src/test/resources/com/alibaba/druid/pool/simplejndi/jdbc.properties
index 6f3e896c91..e3af7416bb 100644
--- a/src/test/resources/com/alibaba/druid/pool/simplejndi/jdbc.properties
+++ b/src/test/resources/com/alibaba/druid/pool/simplejndi/jdbc.properties
@@ -1,4 +1,4 @@
-druidTest/converter=com.alibaba.druid.pool.DruidDataSourceConverter
+druidTest/converter=com.alibaba.druid.support.simplejndi.DruidDataSourceConverter
 druidTest/type=javax.sql.DataSource
 druidTest/driverClassName=com.alibaba.druid.mock.MockDriver
 druidTest/url=jdbc:fake:dragoon_v25masterdb

From 213570dfc605882056742654e82c1dee30b16ec7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E9=AB=98=E9=93=81?= 
Date: Thu, 5 May 2016 19:34:40 +0800
Subject: [PATCH 5/5] bug fixed for dataSource fill

---
 .../alibaba/druid/pool/DruidDataSource.java   | 38 ++++++++++++++++---
 .../bvt/pool/DruidDataSourceTest_fill2.java   |  9 ++++-
 2 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/src/main/java/com/alibaba/druid/pool/DruidDataSource.java b/src/main/java/com/alibaba/druid/pool/DruidDataSource.java
index fa36a698c7..75fb862de8 100644
--- a/src/main/java/com/alibaba/druid/pool/DruidDataSource.java
+++ b/src/main/java/com/alibaba/druid/pool/DruidDataSource.java
@@ -1333,17 +1333,23 @@ public class DruidDataSource extends DruidAbstractDataSource
                 return;
             }
 
+            boolean result;
             final long lastActiveTimeMillis = System.currentTimeMillis();
             lock.lockInterruptibly();
             try {
                 activeCount--;
                 closeCount++;
 
-                putLast(holder, lastActiveTimeMillis);
+                result = putLast(holder, lastActiveTimeMillis);
                 recycleCount++;
             } finally {
                 lock.unlock();
             }
+            
+            if (!result) {
+                JdbcUtils.close(holder.getConnection());
+                LOG.info("connection recyle failed.");
+            }
         } catch (Throwable e) {
             holder.clearStatementCache();
 
@@ -1478,7 +1484,11 @@ public class DruidDataSource extends DruidAbstractDataSource
         return mbeanRegistered;
     }
 
-    void putLast(DruidConnectionHolder e, long lastActiveTimeMillis) {
+    boolean putLast(DruidConnectionHolder e, long lastActiveTimeMillis) {
+        if (poolingCount >= maxActive) {
+            return false;
+        }
+        
         e.setLastActiveTimeMillis(lastActiveTimeMillis);
         connections[poolingCount] = e;
         incrementPoolingCount();
@@ -1490,6 +1500,8 @@ public class DruidDataSource extends DruidAbstractDataSource
 
         notEmpty.signal();
         notEmptySignalCount++;
+        
+        return true;
     }
 
     DruidConnectionHolder takeLast() throws InterruptedException, SQLException {
@@ -1783,7 +1795,7 @@ public class DruidDataSource extends DruidAbstractDataSource
         return removeAbandonedCount;
     }
 
-    protected void put(PhysicalConnectionInfo physicalConnectionInfo) {
+    protected boolean put(PhysicalConnectionInfo physicalConnectionInfo) {
         DruidConnectionHolder holder = null;
         try {
             holder = new DruidConnectionHolder(DruidDataSource.this, physicalConnectionInfo);
@@ -1797,11 +1809,14 @@ public class DruidDataSource extends DruidAbstractDataSource
                 lock.unlock();
             }
             LOG.error("create connection holder error", ex);
-            return;
+            return false;
         }
 
         lock.lock();
         try {
+            if (poolingCount >= maxActive) {
+                return false;
+            }
             connections[poolingCount] = holder;
             incrementPoolingCount();
 
@@ -1824,6 +1839,8 @@ public class DruidDataSource extends DruidAbstractDataSource
         } finally {
             lock.unlock();
         }
+        
+        return true;
     }
 
     public class CreateConnectionTask implements Runnable {
@@ -1915,7 +1932,11 @@ public class DruidDataSource extends DruidAbstractDataSource
                     continue;
                 }
 
-                put(physicalConnection);
+                boolean result = put(physicalConnection);
+                if (!result) {
+                    JdbcUtils.close(physicalConnection.getPhysicalConnection());
+                    LOG.info("put physical connection to pool failed.");
+                }
                 break;
             }
         }
@@ -2009,7 +2030,11 @@ public class DruidDataSource extends DruidAbstractDataSource
                     continue;
                 }
 
-                put(connection);
+                boolean result = put(connection);
+                if (!result) {
+                    JdbcUtils.close(connection.getPhysicalConnection());
+                    LOG.info("put physical connection to pool failed.");
+                }
 
                 errorCount = 0; // reset errorCount
             }
@@ -2824,6 +2849,7 @@ public class DruidDataSource extends DruidAbstractDataSource
             try {
                 if (!this.isFillable(toCount)) {
                     JdbcUtils.close(holder.getConnection());
+                    LOG.info("fill connections skip.");
                     break;
                 }
                 this.putLast(holder, System.currentTimeMillis());
diff --git a/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_fill2.java b/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_fill2.java
index 789723c6eb..c612246fa5 100644
--- a/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_fill2.java
+++ b/src/test/java/com/alibaba/druid/bvt/pool/DruidDataSourceTest_fill2.java
@@ -78,8 +78,13 @@ public class DruidDataSourceTest_fill2 extends TestCase {
 
         Assert.assertEquals(0, fillErrorCount.get());
 
-        endLatch.await(1000, TimeUnit.MILLISECONDS);
-
+        for (int i = 0; i < 100; ++i) {
+            endLatch.await(100, TimeUnit.MILLISECONDS);
+            if (fillCount.get() > 0) {
+                break;
+            }
+        }
+        Assert.assertEquals(true, dataSource.isFull());
         Assert.assertEquals(true, fillCount.get() > 0);
 
     }