Resolve high thread count when using findSocketUsingThreading#2104

Merged

Jeffery-Wasty merged 46 commits intomainfrom

infiniteThreads

Jun 6, 2023

Contributor

Jeffery-Wasty commented Mar 31, 2023 •

edited

Loading

findSocketUsingThreading can reach high thread counts for some users. This was due to an uncaught exception.

Jeffery-Wasty requested review from lilgreenbird and tkyc

April 3, 2023 16:47

Jeffery-Wasty added this to the 12.3.0 milestone

Jeffery-Wasty linked an issue

that may be closed by this pull request

Infinite loop in SQLServerConnection.login, spawning thousands of threads, causes high CPU and threads consumption #1831

Closed

Jeffery-Wasty marked this pull request as draft

April 11, 2023 19:36

Jeffery-Wasty modified the milestones: 12.3.0, 12.3.1

Jeffery-Wasty force-pushed the infiniteThreads branch from 2cdbef9 to 7cf069c Compare

May 9, 2023 17:04

Jeffery-Wasty marked this pull request as ready for review

May 16, 2023 17:15

lilgreenbird requested a review from David-Engel

May 17, 2023 19:52

tkyc reviewed

View reviewed changes

src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java Outdated

David-Engel reviewed

View reviewed changes

src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java Outdated

src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java Outdated

Jeffery-Wasty commented

View reviewed changes

src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java Outdated

tkyc previously approved these changes

View reviewed changes

Jeffery-Wasty dismissed tkyc’s stale review via

e5d5fea

May 25, 2023 15:57

lilgreenbird reviewed

View reviewed changes

src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java Outdated

lilgreenbird reviewed

View reviewed changes

src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java Outdated

lilgreenbird reviewed

View reviewed changes

src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java Outdated

lilgreenbird reviewed

View reviewed changes

src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java Outdated

Jeffery-Wasty added 11 commits

May 31, 2023 09:51


          Close a resource leak to prevent thread spawn.

fa1ca47


          Add resource leak fix.

433ac4c


          Added test for thread count during find socket using threading.

599be26


          Changed threshold to 5x

caa7d31


          Changing servername from 'localhost' to prevent test failures.

61502f5


          Replace explicit close for socket, with try

236e85c


          Delete test.java that shouldn't have been pushed

0e3be63


          Playing with the threadPoolExecutor thread limit.

fb15594

..

283ef05


          Increase threshold.

b287bcc


          Removed TODO

a09ce52

Jeffery-Wasty added 23 commits

May 31, 2023 09:51


          Added condition to force multithreading.

4acccc9


          More debugging earlier on

df47b5f


          Edit test

f4690e7


          More debugging.

39678c9


          Added print statements further up in the call stack.

230b21b


          for some reason, we're not logging

a55fd97


          More debug

e63515d


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

ab4ec0c


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

d478453


          Cleanup

607bdb0


          Final cleanup before review.

c51e682


          Test cleanup.


          We can't ignore the InterruptedException. If we catch, log, and inter…

3a52c67

…rupt the thread.


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

0271ba3


          A different try with reflection.

66d6467


          One more try with reflection.

ae567c0


          Exclude junit test from DB and DW.

c44c8d0


          whitespace

160f33c


          Remove localhost?

770f5e7


          Moved back in localhost, otherwise the correct condition is not reach…

9f39702

…ed, and the multithread route is not used.


          Check for localhost + PR comments

dd57d60

...

233100b


          Resolve failures that came up after inserting assertTrue() statement

9f3c78c

Jeffery-Wasty force-pushed the infiniteThreads branch from 73dabb4 to 9f3c78c Compare

May 31, 2023 17:14


          Cleanup again

4f5dc39

tkyc approved these changes

View reviewed changes

David-Engel approved these changes

View reviewed changes

lilgreenbird approved these changes

View reviewed changes

Jeffery-Wasty merged commit 0eec249 into main

Jeffery-Wasty deleted the infiniteThreads branch

June 7, 2023 17:58

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet