Skip to content

Commit

Permalink
Resolve high thread count when using findSocketUsingThreading (#2104)
Browse files Browse the repository at this point in the history
* Close a resource leak to prevent thread spawn.

* Add resource leak fix.

* Added test for thread count during find socket using threading.

* Changed threshold to 5x

* Changing servername from 'localhost' to prevent test failures.

* Replace explicit close for socket, with try

* Delete test.java that shouldn't have been pushed

* Playing with the threadPoolExecutor thread limit.

* ..

* Increase threshold.

* Removed TODO

* After catching exception, we should continue.

* Removed lingering artifacts.

* Added debugging

* More debugging

* Even more debugging.

* More debugging...

* Try sleep change.

* Trying without fix again to see coco behaviour with a final weight of 20000.

* 10,000

* Addded some debugging in the IOBUffer

* ...

* Added condition to force multithreading.

* More debugging earlier on

* Edit test

* More debugging.

* Added print statements further up in the call stack.

* for some reason, we're not logging

* More debug

* Trying without setting localhost in the test as this messes things up.

* Manually set the iNetAddress to confirm the test is actually running.

* Cleanup

* Final cleanup before review.

* Test cleanup.

* We can't ignore the InterruptedException. If we catch, log, and interrupt the thread.

* We don't want to continue, we don't to reinterrupt.

* A different try with reflection.

* One more try with reflection.

* Exclude junit test from DB and DW.

* whitespace

* Remove localhost?

* Moved back in localhost, otherwise the correct condition is not reached, and the multithread route is not used.

* Check for localhost + PR comments

* ...

* Resolve failures that came up after inserting assertTrue() statement

* Cleanup again
  • Loading branch information
Jeffery-Wasty authored Jun 6, 2023
1 parent 7e476f4 commit 0eec249
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 2 deletions.
11 changes: 10 additions & 1 deletion src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -2954,7 +2954,16 @@ private void findSocketUsingThreading(InetAddress[] inetAddrs, int portNumber,
if (timeRemaining <= 0 || (!result.equals(Result.UNKNOWN)))
break;

parentCondition.await(timeRemaining, TimeUnit.MILLISECONDS);
try {
parentCondition.await(timeRemaining, TimeUnit.MILLISECONDS);
} catch (InterruptedException ie) {
// Don't re-interrupt the current thread here.
//
// Thread interrupt is how parentCondition.signalAll works, signaling any thread on await.
// Re-interrupting will just interrupt the next Sleep call that follows this logic path in the
// loop in the SqlServerConnection.login method, causing too many, too fast retries.
// Instead consume the interruption, and let the thread continue.
}

if (logger.isLoggable(Level.FINER)) {
logger.finer(this.toString() + " The parent thread wokeup.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import java.io.IOException;
import java.io.Reader;
import java.lang.management.ManagementFactory;
import java.sql.Clob;
import java.sql.Connection;
import java.sql.DriverManager;
Expand All @@ -32,6 +33,7 @@
import javax.sql.ConnectionEvent;
import javax.sql.PooledConnection;

import org.junit.Assume;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -934,6 +936,65 @@ public void run() {
assertTrue(status && future.isCancelled(), TestResource.getResource("R_threadInterruptNotSet"));
}


/**
* Test thread count when finding socket using threading.
*/
@Test
@Tag(Constants.xAzureSQLDB)
@Tag(Constants.xAzureSQLDW)
public void testThreadCountWhenFindingSocket() {
ExecutorService executor = null;
ManagementFactory.getThreadMXBean().resetPeakThreadCount();

// First, check to see if there is a reachable local host, or else test will fail.
try {
SQLServerDataSource ds = new SQLServerDataSource();
ds.setServerName("localhost");
Connection con = ds.getConnection();
} catch (SQLServerException e) {
// Assume this will be an error different than 'localhost is unreachable'. If it is 'localhost is
// unreachable' abort and skip the test.
Assume.assumeFalse(e.getMessage().startsWith(TestResource.getResource("R_tcpipConnectionToHost")));
}

try {
executor = Executors.newSingleThreadExecutor(r -> new Thread(r, ""));
executor.submit(() -> {
try {
SQLServerDataSource ds = new SQLServerDataSource();
ds.setServerName("localhost");
Thread.sleep(5000);
Connection conn2 = ds.getConnection();
} catch (Exception e) {
if (!(e instanceof SQLServerException)) {
fail(TestResource.getResource("R_unexpectedException") + e.getMessage());
}
}
});
SQLServerDataSource ds = new SQLServerDataSource();
ds.setServerName("localhost");
Connection conn = ds.getConnection();
Thread.sleep(5000);
} catch (Exception e) {
if (!(e instanceof SQLServerException)) {
fail(TestResource.getResource("R_unexpectedException") + e.getMessage());
}
} finally {
if (executor != null) {
executor.shutdown();
}
}

// At this point, thread count has returned to normal. If the peak was more
// than 2 times the current, this is an issue and the test should fail.
if (ManagementFactory.getThreadMXBean().getPeakThreadCount()
> 2 * ManagementFactory.getThreadMXBean().getThreadCount()) {
fail(TestResource.getResource("R_unexpectedThreadCount"));
}
}


/**
* Test calling method to get redirected server string.
*/
Expand Down
3 changes: 2 additions & 1 deletion src/test/java/com/microsoft/sqlserver/jdbc/TestResource.java
Original file line number Diff line number Diff line change
Expand Up @@ -202,5 +202,6 @@ protected Object[][] getContents() {
{"R_cekDecryptionFailed", "Failed to decrypt a column encryption key using key store provider: {0}."},
{"R_connectTimedOut", "connect timed out"},
{"R_sessionKilled", "Cannot continue the execution because the session is in the kill state"},
{"R_failedFedauth", "Failed to acquire fedauth token: "}};
{"R_failedFedauth", "Failed to acquire fedauth token: "},
{"R_unexpectedThreadCount", "Thread count is higher than expected."}};
}

0 comments on commit 0eec249

Please sign in to comment.