From d5f10d505ac6a12eb163382452ca6bf81f739b9d Mon Sep 17 00:00:00 2001 From: v-xiangs Date: Mon, 27 Feb 2017 13:59:10 -0800 Subject: [PATCH 1/9] replace Thread with ExecutorService --- .../com/microsoft/sqlserver/jdbc/IOBuffer.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java b/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java index 9fd654576..3bb245153 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java @@ -55,6 +55,8 @@ import java.util.Set; import java.util.SimpleTimeZone; import java.util.TimeZone; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.SynchronousQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -6824,7 +6826,7 @@ final void TryProcessFeatureExtAck(boolean featureExtAckReceived) throws SQLServ final class TimeoutTimer implements Runnable { private final int timeoutSeconds; private final TDSCommand command; - private Thread timerThread; + private static final ExecutorService executor = Executors.newCachedThreadPool(); private volatile boolean canceled = false; TimeoutTimer(int timeoutSeconds, @@ -6836,18 +6838,16 @@ final class TimeoutTimer implements Runnable { this.command = command; } - final void start() { - timerThread = new Thread(this); - timerThread.setDaemon(true); - timerThread.start(); + final void start() { + executor.execute(this); } final void stop() { + executor.shutdownNow(); canceled = true; - timerThread.interrupt(); } - public void run() { + public void run() { int secondsRemaining = timeoutSeconds; try { // Poll every second while time is left on the timer. From 72d596b25b4db26a477ef8601fede8953c9af36c Mon Sep 17 00:00:00 2001 From: v-xiangs Date: Mon, 27 Feb 2017 15:13:00 -0800 Subject: [PATCH 2/9] if ExecutorService is shutdown already, reset it --- src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java b/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java index 3bb245153..cdd84ceba 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java @@ -6826,7 +6826,7 @@ final void TryProcessFeatureExtAck(boolean featureExtAckReceived) throws SQLServ final class TimeoutTimer implements Runnable { private final int timeoutSeconds; private final TDSCommand command; - private static final ExecutorService executor = Executors.newCachedThreadPool(); + private ExecutorService executor = Executors.newCachedThreadPool(); private volatile boolean canceled = false; TimeoutTimer(int timeoutSeconds, @@ -6838,7 +6838,11 @@ final class TimeoutTimer implements Runnable { this.command = command; } - final void start() { + final void start() { + //if ExecutorService is shutdown already, reset it + if(executor.isShutdown()){ + executor = Executors.newCachedThreadPool(); + } executor.execute(this); } From 29794641a4b94e15088de780366ea133d8688371 Mon Sep 17 00:00:00 2001 From: v-xiangs Date: Tue, 28 Feb 2017 11:51:24 -0800 Subject: [PATCH 3/9] use futures and set threads as Daemon --- .../microsoft/sqlserver/jdbc/IOBuffer.java | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java b/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java index cdd84ceba..5ebe70742 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java @@ -57,7 +57,9 @@ import java.util.TimeZone; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.SynchronousQueue; +import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -6826,7 +6828,15 @@ final void TryProcessFeatureExtAck(boolean featureExtAckReceived) throws SQLServ final class TimeoutTimer implements Runnable { private final int timeoutSeconds; private final TDSCommand command; - private ExecutorService executor = Executors.newCachedThreadPool(); + private volatile Future task; + private static final ExecutorService executor = Executors.newCachedThreadPool(new ThreadFactory() { + @Override + public Thread newThread(Runnable r) { + Thread t = Executors.defaultThreadFactory().newThread(r); + t.setDaemon(true); + return t; + } + }); private volatile boolean canceled = false; TimeoutTimer(int timeoutSeconds, @@ -6839,15 +6849,11 @@ final class TimeoutTimer implements Runnable { } final void start() { - //if ExecutorService is shutdown already, reset it - if(executor.isShutdown()){ - executor = Executors.newCachedThreadPool(); - } - executor.execute(this); + task = executor.submit(this); } final void stop() { - executor.shutdownNow(); + task.cancel(false); canceled = true; } From 585c0b53778c81a95c667738d86917ba8ce314df Mon Sep 17 00:00:00 2001 From: v-xiangs Date: Tue, 28 Feb 2017 17:19:14 -0800 Subject: [PATCH 4/9] thread can be interrupted --- src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java b/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java index 5ebe70742..84e5d89b3 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java @@ -6853,7 +6853,7 @@ final void start() { } final void stop() { - task.cancel(false); + task.cancel(true); canceled = true; } From 4e468384e905a972f5ebec9b894ec0ca83eaab47 Mon Sep 17 00:00:00 2001 From: v-xiangs Date: Wed, 1 Mar 2017 13:56:29 -0800 Subject: [PATCH 5/9] group TimeoutTimer threads --- src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java b/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java index 84e5d89b3..d295f65c8 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java @@ -6826,17 +6826,23 @@ final void TryProcessFeatureExtAck(boolean featureExtAckReceived) throws SQLServ * a reason like "timed out". */ final class TimeoutTimer implements Runnable { + private static final String threadGroupName = "mssql-jdbc-TimeoutTimer"; private final int timeoutSeconds; private final TDSCommand command; private volatile Future task; + private static final ExecutorService executor = Executors.newCachedThreadPool(new ThreadFactory() { + private final ThreadGroup tg = new ThreadGroup(threadGroupName); + private final String threadNamePrefix = tg.getName() + "-"; + private final AtomicInteger threadNumber = new AtomicInteger(0); @Override public Thread newThread(Runnable r) { - Thread t = Executors.defaultThreadFactory().newThread(r); + Thread t = new Thread(tg, r, threadNamePrefix + threadNumber.incrementAndGet()); t.setDaemon(true); return t; } }); + private volatile boolean canceled = false; TimeoutTimer(int timeoutSeconds, From ff2d8118f5ac3a812bcc4df1631f2840c3f75d7e Mon Sep 17 00:00:00 2001 From: v-xiangs Date: Fri, 3 Mar 2017 17:22:34 -0800 Subject: [PATCH 6/9] added Connection Pool test cases --- pom.xml | 16 +++++- .../jdbc/connection/PoolingTest.java | 54 +++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index c9dfd8402..70aa437e5 100644 --- a/pom.xml +++ b/pom.xml @@ -114,7 +114,19 @@ junit-jupiter-engine 5.0.0-M3 test - + + + com.zaxxer + HikariCP + 2.6.0 + test + + + org.apache.commons + commons-dbcp2 + 2.1.1 + test + + --> diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java index 399f56343..860413c61 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java @@ -13,12 +13,16 @@ import java.sql.Connection; import java.sql.DriverManager; +import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; import java.util.UUID; +import javax.sql.DataSource; import javax.sql.PooledConnection; +import org.apache.commons.dbcp2.BasicDataSource; import org.junit.jupiter.api.Test; import org.junit.platform.runner.JUnitPlatform; import org.junit.runner.RunWith; @@ -30,6 +34,8 @@ import com.microsoft.sqlserver.testframework.DBConnection; import com.microsoft.sqlserver.testframework.DBTable; import com.microsoft.sqlserver.testframework.util.RandomUtil; +import com.zaxxer.hikari.HikariConfig; +import com.zaxxer.hikari.HikariDataSource; @RunWith(JUnitPlatform.class) public class PoolingTest extends AbstractTest { @@ -139,4 +145,52 @@ public void testConnectionPoolClientConnectionId() throws SQLException { assertEquals(Id1, Id2, "ClientConnection Ids from pool are not the same."); } + + @Test + public void testHikariCP() throws SQLException { + HikariConfig config = new HikariConfig(); + config.setJdbcUrl(connectionString); + HikariDataSource ds = new HikariDataSource(config); + + connect(ds); + } + + @Test + public void testApacheDBCP() throws SQLException { + BasicDataSource ds = new BasicDataSource(); + ds.setUrl(connectionString); + + connect(ds); + } + + private static void connect(DataSource ds) throws SQLException { + Connection con = null; + PreparedStatement pst = null; + ResultSet rs = null; + + try { + con = ds.getConnection(); + con.isValid(5); + + pst = con.prepareStatement("SELECT SUSER_SNAME()"); + rs = pst.executeQuery(); + + while (rs.next()) { + rs.getString(1); + } + } + finally { + if (rs != null) { + rs.close(); + } + + if (pst != null) { + pst.close(); + } + + if (con != null) { + con.close(); + } + } + } } From ba54360493a332a5236332ed6ad7acee090fadb9 Mon Sep 17 00:00:00 2001 From: v-xiangs Date: Tue, 7 Mar 2017 09:32:39 -0800 Subject: [PATCH 7/9] close datasource, test setQueryTimeout(), count number of threads --- .../jdbc/connection/PoolingTest.java | 36 ++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java index 860413c61..c9e6a19ff 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java @@ -11,6 +11,8 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; +import java.lang.management.ManagementFactory; +import java.lang.management.ThreadInfo; import java.sql.Connection; import java.sql.DriverManager; import java.sql.PreparedStatement; @@ -152,7 +154,12 @@ public void testHikariCP() throws SQLException { config.setJdbcUrl(connectionString); HikariDataSource ds = new HikariDataSource(config); - connect(ds); + try{ + connect(ds); + } + finally{ + ds.close(); + } } @Test @@ -160,7 +167,12 @@ public void testApacheDBCP() throws SQLException { BasicDataSource ds = new BasicDataSource(); ds.setUrl(connectionString); - connect(ds); + try{ + connect(ds); + } + finally{ + ds.close(); + } } private static void connect(DataSource ds) throws SQLException { @@ -170,11 +182,12 @@ private static void connect(DataSource ds) throws SQLException { try { con = ds.getConnection(); - con.isValid(5); - pst = con.prepareStatement("SELECT SUSER_SNAME()"); + pst.setQueryTimeout(5); rs = pst.executeQuery(); + assertTrue(countTimeoutThreads() >= 1, "Timeout timer is missing."); + while (rs.next()) { rs.getString(1); } @@ -193,4 +206,19 @@ private static void connect(DataSource ds) throws SQLException { } } } + + private static int countTimeoutThreads() { + int count = 0; + String threadName = "mssql-jdbc-TimeoutTimer"; + + ThreadInfo[] tinfos = ManagementFactory.getThreadMXBean().getThreadInfo(ManagementFactory.getThreadMXBean().getAllThreadIds(), 0); + + for (ThreadInfo ti : tinfos) { + if ((ti.getThreadName().startsWith(threadName)) && (ti.getThreadState().equals(java.lang.Thread.State.TIMED_WAITING))) { + count++; + } + } + + return count; + } } From 1780179782eae00506bacdcf5b4d0a1a8342a312 Mon Sep 17 00:00:00 2001 From: v-xiangs Date: Thu, 9 Mar 2017 16:46:59 -0800 Subject: [PATCH 8/9] add java docs for tests --- .../sqlserver/jdbc/connection/PoolingTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java index c9e6a19ff..c3772b0e1 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java @@ -148,6 +148,9 @@ public void testConnectionPoolClientConnectionId() throws SQLException { assertEquals(Id1, Id2, "ClientConnection Ids from pool are not the same."); } + /** + * test connection pool with HikariCP + */ @Test public void testHikariCP() throws SQLException { HikariConfig config = new HikariConfig(); @@ -162,6 +165,9 @@ public void testHikariCP() throws SQLException { } } + /** + * test connection pool with Apache DBCP + */ @Test public void testApacheDBCP() throws SQLException { BasicDataSource ds = new BasicDataSource(); @@ -175,6 +181,9 @@ public void testApacheDBCP() throws SQLException { } } + /** + * setup connection, get connection from pool, and test threads + */ private static void connect(DataSource ds) throws SQLException { Connection con = null; PreparedStatement pst = null; @@ -207,6 +216,11 @@ private static void connect(DataSource ds) throws SQLException { } } + /** + * count number of mssql-jdbc-TimeoutTimer threads + * + * @return + */ private static int countTimeoutThreads() { int count = 0; String threadName = "mssql-jdbc-TimeoutTimer"; From a9d41cc4a0f421af09019b25af7bddf808cfe0e5 Mon Sep 17 00:00:00 2001 From: v-xiangs Date: Thu, 9 Mar 2017 16:55:32 -0800 Subject: [PATCH 9/9] add class level java doc --- .../sqlserver/jdbc/connection/PoolingTest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java index c3772b0e1..cdbc6e438 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/connection/PoolingTest.java @@ -39,6 +39,10 @@ import com.zaxxer.hikari.HikariConfig; import com.zaxxer.hikari.HikariDataSource; +/** + * Tests pooled connection + * + */ @RunWith(JUnitPlatform.class) public class PoolingTest extends AbstractTest { @Test @@ -150,6 +154,8 @@ public void testConnectionPoolClientConnectionId() throws SQLException { /** * test connection pool with HikariCP + * + * @throws SQLException */ @Test public void testHikariCP() throws SQLException { @@ -167,6 +173,8 @@ public void testHikariCP() throws SQLException { /** * test connection pool with Apache DBCP + * + * @throws SQLException */ @Test public void testApacheDBCP() throws SQLException { @@ -181,8 +189,12 @@ public void testApacheDBCP() throws SQLException { } } + /** * setup connection, get connection from pool, and test threads + * + * @param ds + * @throws SQLException */ private static void connect(DataSource ds) throws SQLException { Connection con = null;