Skip to content

Commit ebf4d78

Browse files
authored
Revert PR 2247 (#2267)
1 parent a7b4dbc commit ebf4d78

File tree

8 files changed

+210
-178
lines changed

8 files changed

+210
-178
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,8 @@ final InetSocketAddress open(String host, int port, int timeoutMillis, boolean u
727727
setSocketOptions(tcpSocket, this);
728728

729729
// set SO_TIMEOUT
730-
tcpSocket.setSoTimeout(con.getSocketTimeoutMilliseconds());
730+
int socketTimeout = con.getSocketTimeoutMilliseconds();
731+
tcpSocket.setSoTimeout(socketTimeout);
731732

732733
inputStream = tcpInputStream = new ProxyInputStream(tcpSocket.getInputStream());
733734
outputStream = tcpOutputStream = tcpSocket.getOutputStream();

src/main/java/com/microsoft/sqlserver/jdbc/IdleConnectionResiliency.java

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import java.lang.Thread.State;
99
import java.util.concurrent.ScheduledFuture;
10-
import java.util.concurrent.TimeUnit;
1110
import java.util.concurrent.atomic.AtomicInteger;
1211
import java.util.logging.Level;
1312

@@ -167,10 +166,7 @@ int getLoginTimeoutSeconds() {
167166

168167
void reconnect(TDSCommand cmd) throws InterruptedException {
169168
reconnectErrorReceived = null;
170-
connectRetryCount = this.connection.getRetryCount();
171-
if (connectRetryCount > 0) {
172-
reconnectThread = new ReconnectThread(this.connection, cmd);
173-
}
169+
reconnectThread = new ReconnectThread(this.connection, cmd);
174170
reconnectThread.start();
175171
reconnectThread.join();
176172
reconnectErrorReceived = reconnectThread.getException();
@@ -457,9 +453,8 @@ public void run() {
457453
}
458454

459455
boolean keepRetrying = true;
460-
long connectRetryInterval = TimeUnit.SECONDS.toMillis(con.getRetryInterval());
461456

462-
while ((connectRetryCount >= 0) && (!stopRequested) && keepRetrying) {
457+
while ((connectRetryCount > 0) && (!stopRequested) && keepRetrying) {
463458
if (loggerResiliency.isLoggable(Level.FINER)) {
464459
loggerResiliency.finer("Idle connection resiliency - running reconnect for command: "
465460
+ command.toString() + " ; connectRetryCount = " + connectRetryCount);
@@ -497,7 +492,7 @@ public void run() {
497492
} else {
498493
try {
499494
if (connectRetryCount > 1) {
500-
Thread.sleep(connectRetryInterval);
495+
Thread.sleep((long) (con.getRetryInterval()) * 1000);
501496
}
502497
} catch (InterruptedException ie) {
503498
if (loggerResiliency.isLoggable(Level.FINER)) {
@@ -530,7 +525,7 @@ public void run() {
530525
}
531526
}
532527

533-
if ((connectRetryCount <= 0) && (keepRetrying)) {
528+
if ((connectRetryCount == 0) && (keepRetrying)) {
534529
eReceived = new SQLServerException(SQLServerException.getErrString("R_crClientAllRecoveryAttemptsFailed"),
535530
eReceived);
536531
}

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java

Lines changed: 186 additions & 109 deletions
Large diffs are not rendered by default.

src/main/java/com/microsoft/sqlserver/jdbc/SQLServerException.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ static String generateStateCode(SQLServerConnection con, int errNum, Integer dat
393393
* original error string.
394394
*/
395395
static String checkAndAppendClientConnId(String errMsg, SQLServerConnection conn) {
396-
if (null != conn && conn.isConnected()) {
396+
if (null != conn && conn.attachConnId()) {
397397
UUID clientConnId = conn.getClientConIdInternal();
398398
assert null != clientConnId;
399399
StringBuilder sb = new StringBuilder(errMsg);

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

Lines changed: 4 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -463,45 +463,6 @@ public void testConnectionPoolGetTwice() throws SQLException {
463463
public void testConnectCountInLoginAndCorrectRetryCount() {
464464
long timerStart = 0;
465465

466-
int connectRetryCount = 3;
467-
int connectRetryInterval = 1;
468-
int longLoginTimeout = loginTimeOutInSeconds * 4;
469-
470-
try {
471-
SQLServerDataSource ds = new SQLServerDataSource();
472-
ds.setURL(connectionString);
473-
ds.setLoginTimeout(longLoginTimeout);
474-
ds.setConnectRetryCount(connectRetryCount);
475-
ds.setConnectRetryInterval(connectRetryInterval);
476-
ds.setDatabaseName(RandomUtil.getIdentifier("DataBase"));
477-
timerStart = System.currentTimeMillis();
478-
479-
try (Connection con = ds.getConnection()) {
480-
assertTrue(con == null, TestResource.getResource("R_shouldNotConnect"));
481-
}
482-
} catch (Exception e) {
483-
long totalTime = System.currentTimeMillis() - timerStart;
484-
485-
assertTrue(e.getMessage().contains(TestResource.getResource("R_cannotOpenDatabase")), e.getMessage());
486-
int expectedMinimumTimeInMillis = (connectRetryCount * connectRetryInterval) * 1000; // 3 seconds
487-
488-
// Minimum time is 0 seconds per attempt and connectRetryInterval * connectRetryCount seconds of interval.
489-
// Maximum is unknown, but is needs to be less than longLoginTimeout or else this is an issue.
490-
assertTrue(totalTime > expectedMinimumTimeInMillis, TestResource.getResource("R_executionNotLong")
491-
+ " totalTime: " + totalTime + " expectedTime: " + expectedMinimumTimeInMillis);
492-
assertTrue(totalTime < (longLoginTimeout * 1000L), TestResource.getResource("R_executionTooLong")
493-
+ "totalTime: " + totalTime + " expectedTime: " + expectedMinimumTimeInMillis);
494-
}
495-
}
496-
497-
/**
498-
* Tests whether connectRetryCount and connectRetryInterval are properly respected in the login loop. As well, tests
499-
* that connection is retried the proper number of times. This is for cases with zero retries.
500-
*/
501-
@Test
502-
public void testConnectCountInLoginAndCorrectRetryCountWithZeroRetry() {
503-
long timerStart = 0;
504-
505466
int connectRetryCount = 0;
506467
int connectRetryInterval = 60;
507468
int longLoginTimeout = loginTimeOutInSeconds * 3; // 90 seconds
@@ -1016,9 +977,10 @@ public void testThreadInterruptedStatus() throws InterruptedException {
1016977
Runnable runnable = new Runnable() {
1017978
public void run() {
1018979
SQLServerDataSource ds = new SQLServerDataSource();
980+
1019981
ds.setURL(connectionString);
1020-
ds.setDatabaseName("invalidDatabase" + UUID.randomUUID());
1021-
ds.setLoginTimeout(30);
982+
ds.setServerName("invalidServerName" + UUID.randomUUID());
983+
ds.setLoginTimeout(5);
1022984
try (Connection con = ds.getConnection()) {} catch (SQLException e) {}
1023985
}
1024986
};
@@ -1033,8 +995,7 @@ public void run() {
1033995
Thread.sleep(8000);
1034996
executor.shutdownNow();
1035997

1036-
assertTrue(status && future.isCancelled(), TestResource.getResource("R_threadInterruptNotSet") + " status: "
1037-
+ status + " isCancelled: " + future.isCancelled());
998+
assertTrue(status && future.isCancelled(), TestResource.getResource("R_threadInterruptNotSet"));
1038999
}
10391000

10401001
/**

src/test/java/com/microsoft/sqlserver/jdbc/connection/TimeoutTest.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public class TimeoutTest extends AbstractTest {
3939
static String randomServer = RandomUtil.getIdentifier("Server");
4040
static String waitForDelaySPName = RandomUtil.getIdentifier("waitForDelaySP");
4141
static final int waitForDelaySeconds = 10;
42-
static final int defaultTimeout = 60; // loginTimeout default value
42+
static final int defaultTimeout = 15; // loginTimeout default value
4343

4444
@BeforeAll
4545
public static void setupTests() throws Exception {
@@ -55,13 +55,13 @@ public void testDefaultLoginTimeout() {
5555
try (Connection con = PrepUtil.getConnection("jdbc:sqlserver://" + randomServer + "connectRetryCount=0")) {
5656
fail(TestResource.getResource("R_shouldNotConnect"));
5757
} catch (Exception e) {
58-
timerEnd = System.currentTimeMillis();
5958
assertTrue((e.getMessage().contains(TestResource.getResource("R_tcpipConnectionToHost")))
6059
|| ((isSqlAzure() || isSqlAzureDW())
6160
? e.getMessage().contains(
6261
TestResource.getResource("R_connectTimedOut"))
6362
: false),
6463
e.getMessage());
64+
timerEnd = System.currentTimeMillis();
6565
}
6666

6767
verifyTimeout(timerEnd - timerStart, defaultTimeout);
@@ -77,13 +77,13 @@ public void testURLLoginTimeout() {
7777
try (Connection con = PrepUtil.getConnection("jdbc:sqlserver://" + randomServer + ";logintimeout=" + timeout)) {
7878
fail(TestResource.getResource("R_shouldNotConnect"));
7979
} catch (Exception e) {
80-
timerEnd = System.currentTimeMillis();
8180
assertTrue((e.getMessage().contains(TestResource.getResource("R_tcpipConnectionToHost")))
8281
|| ((isSqlAzure() || isSqlAzureDW())
8382
? e.getMessage().contains(
8483
TestResource.getResource("R_connectTimedOut"))
8584
: false),
8685
e.getMessage());
86+
timerEnd = System.currentTimeMillis();
8787
}
8888

8989
verifyTimeout(timerEnd - timerStart, timeout);
@@ -100,13 +100,13 @@ public void testDMLoginTimeoutApplied() {
100100
try (Connection con = PrepUtil.getConnection("jdbc:sqlserver://" + randomServer)) {
101101
fail(TestResource.getResource("R_shouldNotConnect"));
102102
} catch (Exception e) {
103-
timerEnd = System.currentTimeMillis();
104103
assertTrue((e.getMessage().contains(TestResource.getResource("R_tcpipConnectionToHost")))
105104
|| ((isSqlAzure() || isSqlAzureDW())
106105
? e.getMessage().contains(
107106
TestResource.getResource("R_connectTimedOut"))
108107
: false),
109108
e.getMessage());
109+
timerEnd = System.currentTimeMillis();
110110
}
111111

112112
verifyTimeout(timerEnd - timerStart, timeout);
@@ -124,7 +124,6 @@ public void testDMLoginTimeoutNotApplied() {
124124
.getConnection("jdbc:sqlserver://" + randomServer + ";loginTimeout=" + timeout)) {
125125
fail(TestResource.getResource("R_shouldNotConnect"));
126126
} catch (Exception e) {
127-
timerEnd = System.currentTimeMillis();
128127
assertTrue(
129128
(e.getMessage().contains(TestResource.getResource("R_tcpipConnectionToHost")))
130129
|| ((isSqlAzure() || isSqlAzureDW())
@@ -133,6 +132,7 @@ public void testDMLoginTimeoutNotApplied() {
133132
.getResource("R_connectTimedOut"))
134133
: false),
135134
e.getMessage());
135+
timerEnd = System.currentTimeMillis();
136136
}
137137
verifyTimeout(timerEnd - timerStart, timeout);
138138
} finally {
@@ -152,13 +152,13 @@ public void testConnectRetryBadServer() {
152152
.getConnection("jdbc:sqlserver://" + randomServer + ";loginTimeout=" + loginTimeout)) {
153153
fail(TestResource.getResource("R_shouldNotConnect"));
154154
} catch (Exception e) {
155-
timerEnd = System.currentTimeMillis();
156155
assertTrue((e.getMessage().contains(TestResource.getResource("R_tcpipConnectionToHost")))
157156
|| ((isSqlAzure() || isSqlAzureDW())
158157
? e.getMessage().contains(
159158
TestResource.getResource("R_connectTimedOut"))
160159
: false),
161160
e.getMessage());
161+
timerEnd = System.currentTimeMillis();
162162
}
163163

164164
verifyTimeout(timerEnd - timerStart, loginTimeout);
@@ -179,13 +179,13 @@ public void testConnectRetryServerError() {
179179
+ ";connectRetryInterval=" + connectRetryInterval)) {
180180
fail(TestResource.getResource("R_shouldNotConnect"));
181181
} catch (Exception e) {
182-
timerEnd = System.currentTimeMillis();
183182
assertTrue((e.getMessage().contains(TestResource.getResource("R_cannotOpenDatabase")))
184183
|| ((isSqlAzure() || isSqlAzureDW())
185184
? e.getMessage().contains(
186185
TestResource.getResource("R_connectTimedOut"))
187186
: false),
188187
e.getMessage());
188+
timerEnd = System.currentTimeMillis();
189189
}
190190

191191
// connect + all retries should always be <= loginTimeout
@@ -211,13 +211,13 @@ public void testConnectRetryServerErrorDS() {
211211
try (Connection con = PrepUtil.getConnection(connectStr)) {
212212
fail(TestResource.getResource("R_shouldNotConnect"));
213213
} catch (Exception e) {
214-
timerEnd = System.currentTimeMillis();
215214
assertTrue((e.getMessage().contains(TestResource.getResource("R_cannotOpenDatabase")))
216215
|| ((isSqlAzure() || isSqlAzureDW())
217216
? e.getMessage().contains(
218217
TestResource.getResource("R_connectTimedOut"))
219218
: false),
220219
e.getMessage());
220+
timerEnd = System.currentTimeMillis();
221221
}
222222

223223
// connect + all retries should always be <= loginTimeout
@@ -228,8 +228,8 @@ public void testConnectRetryServerErrorDS() {
228228
@Test
229229
public void testConnectRetryTimeout() {
230230
long timerEnd = 0;
231-
int loginTimeout = 2;
232231
long timerStart = System.currentTimeMillis();
232+
int loginTimeout = 2;
233233

234234
// non existent server with very short loginTimeout so there is no time to do all retries
235235
try (Connection con = PrepUtil.getConnection(
@@ -238,13 +238,13 @@ public void testConnectRetryTimeout() {
238238
+ (new Random().nextInt(defaultTimeout - 1) + 1) + ";loginTimeout=" + loginTimeout)) {
239239
fail(TestResource.getResource("R_shouldNotConnect"));
240240
} catch (Exception e) {
241-
timerEnd = System.currentTimeMillis();
242241
assertTrue((e.getMessage().contains(TestResource.getResource("R_cannotOpenDatabase")))
243242
|| ((isSqlAzure() || isSqlAzureDW())
244243
? e.getMessage().contains(
245244
TestResource.getResource("R_connectTimedOut"))
246245
: false),
247246
e.getMessage());
247+
timerEnd = System.currentTimeMillis();
248248
}
249249

250250
verifyTimeout(timerEnd - timerStart, loginTimeout);
@@ -260,13 +260,13 @@ public void testFailoverInstanceResolution() throws SQLException {
260260
+ ";databaseName=FailoverDB_abc;failoverPartner=" + randomServer + "\\foo;user=sa;password=pwd;")) {
261261
fail(TestResource.getResource("R_shouldNotConnect"));
262262
} catch (Exception e) {
263-
timerEnd = System.currentTimeMillis();
264263
assertTrue((e.getMessage().contains(TestResource.getResource("R_tcpipConnectionToHost")))
265264
|| ((isSqlAzure() || isSqlAzureDW())
266265
? e.getMessage().contains(
267266
TestResource.getResource("R_connectTimedOut"))
268267
: false),
269268
e.getMessage());
269+
timerEnd = System.currentTimeMillis();
270270
}
271271

272272
verifyTimeout(timerEnd - timerStart, defaultTimeout * 2);

src/test/java/com/microsoft/sqlserver/jdbc/resiliency/BasicConnectionTest.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public void testBasicConnectionAAD() throws Exception {
6464
basicReconnect("jdbc:sqlserver://" + azureServer + ";database=" + azureDatabase + ";user="
6565
+ azureUserName + ";password=" + azurePassword
6666
+ ";loginTimeout=90;Authentication=ActiveDirectoryPassword");
67-
retry = THROTTLE_RETRY_COUNT + 1;
67+
retry = THROTTLE_RETRY_COUNT + 1;
6868
} catch (Exception e) {
6969
if (e.getMessage().matches(TestUtils.formatErrorMsg("R_crClientAllRecoveryAttemptsFailed"))) {
7070
System.out.println(e.getMessage() + ". Recovery failed, retry #" + retry + " in "
@@ -265,8 +265,7 @@ public void testPooledConnectionDB() throws SQLException {
265265
@Test
266266
public void testPooledConnectionLang() throws SQLException {
267267
SQLServerConnectionPoolDataSource mds = new SQLServerConnectionPoolDataSource();
268-
mds.setURL(connectionString + ";connectRetryCount=1");
269-
268+
mds.setURL(connectionString);
270269
PooledConnection pooledConnection = mds.getPooledConnection();
271270
String lang0 = null, lang1 = null;
272271

src/test/java/com/microsoft/sqlserver/jdbc/resiliency/ReflectiveTests.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ public void testDefaultRetry() throws SQLException {
8585
m.put("loginTimeout", "5");
8686

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

0 commit comments

Comments
 (0)