Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert PR 2247 #2267

Merged
merged 4 commits into from
Nov 28, 2023
Merged

Revert PR 2247 #2267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b38c76a
revert 2247
lilgreenbird Nov 28, 2023
e539b51
fixed merge
lilgreenbird Nov 28, 2023
0d5de25
fix
lilgreenbird Nov 28, 2023
c0c2c2f
test fix
lilgreenbird Nov 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,8 @@ final InetSocketAddress open(String host, int port, int timeoutMillis, boolean u
setSocketOptions(tcpSocket, this);

// set SO_TIMEOUT
tcpSocket.setSoTimeout(con.getSocketTimeoutMilliseconds());
int socketTimeout = con.getSocketTimeoutMilliseconds();
tcpSocket.setSoTimeout(socketTimeout);

inputStream = tcpInputStream = new ProxyInputStream(tcpSocket.getInputStream());
outputStream = tcpOutputStream = tcpSocket.getOutputStream();
Expand Down
13 changes: 4 additions & 9 deletions src/main/java/com/microsoft/sqlserver/jdbc/IdleConnectionResiliency.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import java.lang.Thread.State;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.logging.Level;

Expand Down Expand Up @@ -167,10 +166,7 @@ int getLoginTimeoutSeconds() {

void reconnect(TDSCommand cmd) throws InterruptedException {
reconnectErrorReceived = null;
connectRetryCount = this.connection.getRetryCount();
if (connectRetryCount > 0) {
reconnectThread = new ReconnectThread(this.connection, cmd);
}
reconnectThread = new ReconnectThread(this.connection, cmd);
reconnectThread.start();
reconnectThread.join();
reconnectErrorReceived = reconnectThread.getException();
Expand Down Expand Up @@ -457,9 +453,8 @@ public void run() {
}

boolean keepRetrying = true;
long connectRetryInterval = TimeUnit.SECONDS.toMillis(con.getRetryInterval());

while ((connectRetryCount >= 0) && (!stopRequested) && keepRetrying) {
while ((connectRetryCount > 0) && (!stopRequested) && keepRetrying) {
if (loggerResiliency.isLoggable(Level.FINER)) {
loggerResiliency.finer("Idle connection resiliency - running reconnect for command: "
+ command.toString() + " ; connectRetryCount = " + connectRetryCount);
Expand Down Expand Up @@ -497,7 +492,7 @@ public void run() {
} else {
try {
if (connectRetryCount > 1) {
Thread.sleep(connectRetryInterval);
Thread.sleep((long) (con.getRetryInterval()) * 1000);
}
} catch (InterruptedException ie) {
if (loggerResiliency.isLoggable(Level.FINER)) {
Expand Down Expand Up @@ -530,7 +525,7 @@ public void run() {
}
}

if ((connectRetryCount <= 0) && (keepRetrying)) {
if ((connectRetryCount == 0) && (keepRetrying)) {
eReceived = new SQLServerException(SQLServerException.getErrString("R_crClientAllRecoveryAttemptsFailed"),
eReceived);
}
Expand Down
295 changes: 186 additions & 109 deletions src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/main/java/com/microsoft/sqlserver/jdbc/SQLServerException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ static String generateStateCode(SQLServerConnection con, int errNum, Integer dat
* original error string.
*/
static String checkAndAppendClientConnId(String errMsg, SQLServerConnection conn) {
if (null != conn && conn.isConnected()) {
if (null != conn && conn.attachConnId()) {
UUID clientConnId = conn.getClientConIdInternal();
assert null != clientConnId;
StringBuilder sb = new StringBuilder(errMsg);
Expand Down
47 changes: 4 additions & 43 deletions src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,45 +463,6 @@ public void testConnectionPoolGetTwice() throws SQLException {
public void testConnectCountInLoginAndCorrectRetryCount() {
long timerStart = 0;

int connectRetryCount = 3;
int connectRetryInterval = 1;
int longLoginTimeout = loginTimeOutInSeconds * 4;

try {
SQLServerDataSource ds = new SQLServerDataSource();
ds.setURL(connectionString);
ds.setLoginTimeout(longLoginTimeout);
ds.setConnectRetryCount(connectRetryCount);
ds.setConnectRetryInterval(connectRetryInterval);
ds.setDatabaseName(RandomUtil.getIdentifier("DataBase"));
timerStart = System.currentTimeMillis();

try (Connection con = ds.getConnection()) {
assertTrue(con == null, TestResource.getResource("R_shouldNotConnect"));
}
} catch (Exception e) {
long totalTime = System.currentTimeMillis() - timerStart;

assertTrue(e.getMessage().contains(TestResource.getResource("R_cannotOpenDatabase")), e.getMessage());
int expectedMinimumTimeInMillis = (connectRetryCount * connectRetryInterval) * 1000; // 3 seconds

// Minimum time is 0 seconds per attempt and connectRetryInterval * connectRetryCount seconds of interval.
// Maximum is unknown, but is needs to be less than longLoginTimeout or else this is an issue.
assertTrue(totalTime > expectedMinimumTimeInMillis, TestResource.getResource("R_executionNotLong")
+ " totalTime: " + totalTime + " expectedTime: " + expectedMinimumTimeInMillis);
assertTrue(totalTime < (longLoginTimeout * 1000L), TestResource.getResource("R_executionTooLong")
+ "totalTime: " + totalTime + " expectedTime: " + expectedMinimumTimeInMillis);
}
}

/**
* Tests whether connectRetryCount and connectRetryInterval are properly respected in the login loop. As well, tests
* that connection is retried the proper number of times. This is for cases with zero retries.
*/
@Test
public void testConnectCountInLoginAndCorrectRetryCountWithZeroRetry() {
long timerStart = 0;

int connectRetryCount = 0;
int connectRetryInterval = 60;
int longLoginTimeout = loginTimeOutInSeconds * 3; // 90 seconds
Expand Down Expand Up @@ -1016,9 +977,10 @@ public void testThreadInterruptedStatus() throws InterruptedException {
Runnable runnable = new Runnable() {
public void run() {
SQLServerDataSource ds = new SQLServerDataSource();

ds.setURL(connectionString);
ds.setDatabaseName("invalidDatabase" + UUID.randomUUID());
ds.setLoginTimeout(30);
ds.setServerName("invalidServerName" + UUID.randomUUID());
ds.setLoginTimeout(5);
try (Connection con = ds.getConnection()) {} catch (SQLException e) {}
}
};
Expand All @@ -1033,8 +995,7 @@ public void run() {
Thread.sleep(8000);
executor.shutdownNow();

assertTrue(status && future.isCancelled(), TestResource.getResource("R_threadInterruptNotSet") + " status: "
+ status + " isCancelled: " + future.isCancelled());
assertTrue(status && future.isCancelled(), TestResource.getResource("R_threadInterruptNotSet"));
}

/**
Expand Down
22 changes: 11 additions & 11 deletions src/test/java/com/microsoft/sqlserver/jdbc/connection/TimeoutTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public class TimeoutTest extends AbstractTest {
static String randomServer = RandomUtil.getIdentifier("Server");
static String waitForDelaySPName = RandomUtil.getIdentifier("waitForDelaySP");
static final int waitForDelaySeconds = 10;
static final int defaultTimeout = 60; // loginTimeout default value
static final int defaultTimeout = 15; // loginTimeout default value

@BeforeAll
public static void setupTests() throws Exception {
Expand All @@ -55,13 +55,13 @@ public void testDefaultLoginTimeout() {
try (Connection con = PrepUtil.getConnection("jdbc:sqlserver://" + randomServer + "connectRetryCount=0")) {
fail(TestResource.getResource("R_shouldNotConnect"));
} catch (Exception e) {
timerEnd = System.currentTimeMillis();
assertTrue((e.getMessage().contains(TestResource.getResource("R_tcpipConnectionToHost")))
|| ((isSqlAzure() || isSqlAzureDW())
? e.getMessage().contains(
TestResource.getResource("R_connectTimedOut"))
: false),
e.getMessage());
timerEnd = System.currentTimeMillis();
}

verifyTimeout(timerEnd - timerStart, defaultTimeout);
Expand All @@ -77,13 +77,13 @@ public void testURLLoginTimeout() {
try (Connection con = PrepUtil.getConnection("jdbc:sqlserver://" + randomServer + ";logintimeout=" + timeout)) {
fail(TestResource.getResource("R_shouldNotConnect"));
} catch (Exception e) {
timerEnd = System.currentTimeMillis();
assertTrue((e.getMessage().contains(TestResource.getResource("R_tcpipConnectionToHost")))
|| ((isSqlAzure() || isSqlAzureDW())
? e.getMessage().contains(
TestResource.getResource("R_connectTimedOut"))
: false),
e.getMessage());
timerEnd = System.currentTimeMillis();
}

verifyTimeout(timerEnd - timerStart, timeout);
Expand All @@ -100,13 +100,13 @@ public void testDMLoginTimeoutApplied() {
try (Connection con = PrepUtil.getConnection("jdbc:sqlserver://" + randomServer)) {
fail(TestResource.getResource("R_shouldNotConnect"));
} catch (Exception e) {
timerEnd = System.currentTimeMillis();
assertTrue((e.getMessage().contains(TestResource.getResource("R_tcpipConnectionToHost")))
|| ((isSqlAzure() || isSqlAzureDW())
? e.getMessage().contains(
TestResource.getResource("R_connectTimedOut"))
: false),
e.getMessage());
timerEnd = System.currentTimeMillis();
}

verifyTimeout(timerEnd - timerStart, timeout);
Expand All @@ -124,7 +124,6 @@ public void testDMLoginTimeoutNotApplied() {
.getConnection("jdbc:sqlserver://" + randomServer + ";loginTimeout=" + timeout)) {
fail(TestResource.getResource("R_shouldNotConnect"));
} catch (Exception e) {
timerEnd = System.currentTimeMillis();
assertTrue(
(e.getMessage().contains(TestResource.getResource("R_tcpipConnectionToHost")))
|| ((isSqlAzure() || isSqlAzureDW())
Expand All @@ -133,6 +132,7 @@ public void testDMLoginTimeoutNotApplied() {
.getResource("R_connectTimedOut"))
: false),
e.getMessage());
timerEnd = System.currentTimeMillis();
}
verifyTimeout(timerEnd - timerStart, timeout);
} finally {
Expand All @@ -152,13 +152,13 @@ public void testConnectRetryBadServer() {
.getConnection("jdbc:sqlserver://" + randomServer + ";loginTimeout=" + loginTimeout)) {
fail(TestResource.getResource("R_shouldNotConnect"));
} catch (Exception e) {
timerEnd = System.currentTimeMillis();
assertTrue((e.getMessage().contains(TestResource.getResource("R_tcpipConnectionToHost")))
|| ((isSqlAzure() || isSqlAzureDW())
? e.getMessage().contains(
TestResource.getResource("R_connectTimedOut"))
: false),
e.getMessage());
timerEnd = System.currentTimeMillis();
}

verifyTimeout(timerEnd - timerStart, loginTimeout);
Expand All @@ -179,13 +179,13 @@ public void testConnectRetryServerError() {
+ ";connectRetryInterval=" + connectRetryInterval)) {
fail(TestResource.getResource("R_shouldNotConnect"));
} catch (Exception e) {
timerEnd = System.currentTimeMillis();
assertTrue((e.getMessage().contains(TestResource.getResource("R_cannotOpenDatabase")))
|| ((isSqlAzure() || isSqlAzureDW())
? e.getMessage().contains(
TestResource.getResource("R_connectTimedOut"))
: false),
e.getMessage());
timerEnd = System.currentTimeMillis();
}

// connect + all retries should always be <= loginTimeout
Expand All @@ -211,13 +211,13 @@ public void testConnectRetryServerErrorDS() {
try (Connection con = PrepUtil.getConnection(connectStr)) {
fail(TestResource.getResource("R_shouldNotConnect"));
} catch (Exception e) {
timerEnd = System.currentTimeMillis();
assertTrue((e.getMessage().contains(TestResource.getResource("R_cannotOpenDatabase")))
|| ((isSqlAzure() || isSqlAzureDW())
? e.getMessage().contains(
TestResource.getResource("R_connectTimedOut"))
: false),
e.getMessage());
timerEnd = System.currentTimeMillis();
}

// connect + all retries should always be <= loginTimeout
Expand All @@ -228,8 +228,8 @@ public void testConnectRetryServerErrorDS() {
@Test
public void testConnectRetryTimeout() {
long timerEnd = 0;
int loginTimeout = 2;
long timerStart = System.currentTimeMillis();
int loginTimeout = 2;

// non existent server with very short loginTimeout so there is no time to do all retries
try (Connection con = PrepUtil.getConnection(
Expand All @@ -238,13 +238,13 @@ public void testConnectRetryTimeout() {
+ (new Random().nextInt(defaultTimeout - 1) + 1) + ";loginTimeout=" + loginTimeout)) {
fail(TestResource.getResource("R_shouldNotConnect"));
} catch (Exception e) {
timerEnd = System.currentTimeMillis();
assertTrue((e.getMessage().contains(TestResource.getResource("R_cannotOpenDatabase")))
|| ((isSqlAzure() || isSqlAzureDW())
? e.getMessage().contains(
TestResource.getResource("R_connectTimedOut"))
: false),
e.getMessage());
timerEnd = System.currentTimeMillis();
}

verifyTimeout(timerEnd - timerStart, loginTimeout);
Expand All @@ -260,13 +260,13 @@ public void testFailoverInstanceResolution() throws SQLException {
+ ";databaseName=FailoverDB_abc;failoverPartner=" + randomServer + "\\foo;user=sa;password=pwd;")) {
fail(TestResource.getResource("R_shouldNotConnect"));
} catch (Exception e) {
timerEnd = System.currentTimeMillis();
assertTrue((e.getMessage().contains(TestResource.getResource("R_tcpipConnectionToHost")))
|| ((isSqlAzure() || isSqlAzureDW())
? e.getMessage().contains(
TestResource.getResource("R_connectTimedOut"))
: false),
e.getMessage());
timerEnd = System.currentTimeMillis();
}

verifyTimeout(timerEnd - timerStart, defaultTimeout * 2);
Expand Down
5 changes: 2 additions & 3 deletions src/test/java/com/microsoft/sqlserver/jdbc/resiliency/BasicConnectionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public void testBasicConnectionAAD() throws Exception {
basicReconnect("jdbc:sqlserver://" + azureServer + ";database=" + azureDatabase + ";user="
+ azureUserName + ";password=" + azurePassword
+ ";loginTimeout=90;Authentication=ActiveDirectoryPassword");
retry = THROTTLE_RETRY_COUNT + 1;
retry = THROTTLE_RETRY_COUNT + 1;
} catch (Exception e) {
if (e.getMessage().matches(TestUtils.formatErrorMsg("R_crClientAllRecoveryAttemptsFailed"))) {
System.out.println(e.getMessage() + ". Recovery failed, retry #" + retry + " in "
Expand Down Expand Up @@ -265,8 +265,7 @@ public void testPooledConnectionDB() throws SQLException {
@Test
public void testPooledConnectionLang() throws SQLException {
SQLServerConnectionPoolDataSource mds = new SQLServerConnectionPoolDataSource();
mds.setURL(connectionString + ";connectRetryCount=1");

mds.setURL(connectionString);
PooledConnection pooledConnection = mds.getPooledConnection();
String lang0 = null, lang1 = null;

Expand Down
1 change: 0 additions & 1 deletion src/test/java/com/microsoft/sqlserver/jdbc/resiliency/ReflectiveTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ public void testDefaultRetry() throws SQLException {
m.put("loginTimeout", "5");

// ensure count is not set to something else as this test assumes exactly just 1 retry
m.put("connectRetryCount", "1");
timeoutVariations(m, 6000, Optional.empty());
}

Expand Down