diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index 601271755..5d01955f2 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -2031,7 +2031,7 @@ Connection connect(Properties propsIn, SQLServerPooledConnection pooledConnectio if (0 == connectRetryCount) { // connection retry disabled throw e; - } else if (connectRetryAttempt++ > connectRetryCount) { + } else if (connectRetryAttempt++ >= connectRetryCount) { // maximum connection retry count reached if (connectionlogger.isLoggable(Level.FINE)) { connectionlogger.fine("Connection failed. Maximum connection retry count " @@ -2064,7 +2064,10 @@ Connection connect(Properties propsIn, SQLServerPooledConnection pooledConnectio + connectRetryInterval + ")s before retry."); } - sleepForInterval(TimeUnit.SECONDS.toMillis(connectRetryInterval)); + if (connectRetryAttempt > 1) { + // We do not sleep for first retry; first retry is immediate + sleepForInterval(TimeUnit.SECONDS.toMillis(connectRetryInterval)); + } } } } diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java index 3b7fe1dc7..d4a2ebe6a 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java @@ -456,17 +456,26 @@ public void testConnectionPoolGetTwice() throws SQLException { } } + /** + * Runs the `testConnectCountInLoginAndCorrectRetryCount` test several times with different values of + * connectRetryCount. + */ + @Test + public void testConnectCountInLoginAndCorrectRetryCountForMultipleValues() { + testConnectCountInLoginAndCorrectRetryCount(0); + testConnectCountInLoginAndCorrectRetryCount(1); + testConnectCountInLoginAndCorrectRetryCount(2); + } + /** * Tests whether connectRetryCount and connectRetryInterval are properly respected in the login loop. As well, tests * that connection is retried the proper number of times. */ - @Test - public void testConnectCountInLoginAndCorrectRetryCount() { + private void testConnectCountInLoginAndCorrectRetryCount(int connectRetryCount) { long timerStart = 0; - int connectRetryCount = 0; int connectRetryInterval = 60; - int longLoginTimeout = loginTimeOutInSeconds * 3; // 90 seconds + int longLoginTimeout = loginTimeOutInSeconds * 9; // 90 seconds try { SQLServerDataSource ds = new SQLServerDataSource(); @@ -492,6 +501,15 @@ public void testConnectCountInLoginAndCorrectRetryCount() { // Maximum is unknown, but is needs to be less than longLoginTimeout or else this is an issue. assertTrue(totalTime < (longLoginTimeout * 1000L), TestResource.getResource("R_executionTooLong")); + + // We should at least take as long as the retry interval between all retries past the first. + // Of the above acceptable errors (R_cannotOpenDatabase, R_loginFailedMI, R_MInotAvailable), only + // R_cannotOpenDatabase is transient, and can be used to measure multiple retries with retry interval. The + // others will exit before they have a chance to wait, and min will be too low. + if (e.getMessage().contains(TestResource.getResource("R_cannotOpenDatabase"))) { + int minTimeInSecs = connectRetryInterval * (connectRetryCount - 1); + assertTrue(totalTime > (minTimeInSecs * 1000L), TestResource.getResource("R_executionNotLong")); + } } } @@ -802,9 +820,9 @@ public void testIncorrectDatabase() throws SQLException { } catch (Exception e) { assertTrue( e.getMessage().contains(TestResource.getResource("R_cannotOpenDatabase")) - || (TestUtils.getProperty(connectionString, "msiClientId") != null + || (TestUtils.getProperty(connectionString, "msiClientId") != null && e.getMessage().toLowerCase() - .contains(TestResource.getResource("R_loginFailedMI").toLowerCase())), + .contains(TestResource.getResource("R_loginFailedMI").toLowerCase())), e.getMessage()); timerEnd = System.currentTimeMillis(); } @@ -835,9 +853,9 @@ public void testIncorrectUserName() throws SQLException { } catch (Exception e) { assertTrue( e.getMessage().contains(TestResource.getResource("R_loginFailed")) - || (TestUtils.getProperty(connectionString, "msiClientId") != null + || (TestUtils.getProperty(connectionString, "msiClientId") != null && e.getMessage().toLowerCase() - .contains(TestResource.getResource("R_loginFailedMI").toLowerCase())), + .contains(TestResource.getResource("R_loginFailedMI").toLowerCase())), e.getMessage()); timerEnd = System.currentTimeMillis(); } @@ -869,8 +887,8 @@ public void testIncorrectPassword() throws SQLException { assertTrue( e.getMessage().contains(TestResource.getResource("R_loginFailed")) || (TestUtils.getProperty(connectionString, "msiClientId") != null - && e.getMessage().toLowerCase() - .contains(TestResource.getResource("R_loginFailedMI").toLowerCase())), + && e.getMessage().toLowerCase() + .contains(TestResource.getResource("R_loginFailedMI").toLowerCase())), e.getMessage()); timerEnd = System.currentTimeMillis(); } @@ -1049,8 +1067,8 @@ public void run() { ds.setURL(connectionString); ds.setServerName("invalidServerName" + UUID.randomUUID()); ds.setLoginTimeout(30); - ds.setConnectRetryCount(3); - ds.setConnectRetryInterval(10); + ds.setConnectRetryCount(6); + ds.setConnectRetryInterval(20); try (Connection con = ds.getConnection()) {} catch (SQLException e) {} } }; diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/connection/TimeoutTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/connection/TimeoutTest.java index d7290b262..ed33104e6 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/connection/TimeoutTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/connection/TimeoutTest.java @@ -172,7 +172,9 @@ public void testDMLoginTimeoutNotApplied() { } } - // Test connect retry set to 0 (disabled) + /** + * Test connect retry set to 0 (disabled) + */ @Test public void testConnectRetryDisable() { long totalTime = 0; @@ -180,10 +182,10 @@ public void testConnectRetryDisable() { int interval = defaultTimeout; // long interval so we can tell if there was a retry long timeout = defaultTimeout * 2; // long loginTimeout to accommodate the long interval - // non existent server with long loginTimeout, should return fast if no retries at all + // non-existent server with long loginTimeout, should return fast if no retries at all try (Connection con = PrepUtil.getConnection( "jdbc:sqlserver://" + randomServer + ";transparentNetworkIPResolution=false;loginTimeout=" + timeout - + ";connectRetryCount=0;connectInterval=" + interval)) { + + ";connectRetryCount=0;connectRetryInterval=" + interval)) { fail(TestResource.getResource("R_shouldNotConnect")); } catch (Exception e) { totalTime = System.currentTimeMillis() - timerStart; @@ -230,7 +232,9 @@ public void testConnectRetryBadServer() { "total time: " + totalTime + " loginTimeout: " + TimeUnit.SECONDS.toMillis(timeout)); } - // Test connect retry for database error + /** + * Test connect retry, with one retry interval, for database error + */ @Test public void testConnectRetryServerError() { String auth = TestUtils.getProperty(connectionString, "authentication"); @@ -242,10 +246,10 @@ public void testConnectRetryServerError() { int interval = defaultTimeout; // long interval so we can tell if there was a retry long timeout = defaultTimeout * 2; // long loginTimeout to accommodate the long interval - // non existent database with interval < loginTimeout this will generate a 4060 transient error and retry 1 time + // non-existent database with interval < loginTimeout this will generate a 4060 transient error and retry 2 times try (Connection con = PrepUtil.getConnection( TestUtils.addOrOverrideProperty(connectionString, "database", RandomUtil.getIdentifier("database")) - + ";loginTimeout=" + timeout + ";connectRetryCount=" + 1 + ";connectRetryInterval=" + interval + + ";loginTimeout=" + timeout + ";connectRetryCount=" + 2 + ";connectRetryInterval=" + interval + ";transparentNetworkIPResolution=false")) { fail(TestResource.getResource("R_shouldNotConnect")); } catch (Exception e) { @@ -261,14 +265,16 @@ public void testConnectRetryServerError() { e.getMessage()); } - // 1 retry should be at least 1 interval long but < 2 intervals + // 2 retries should be at least 1 interval long but < 2 intervals (no interval between initial attempt and retry 1) assertTrue(TimeUnit.SECONDS.toMillis(interval) < totalTime, "interval: " + TimeUnit.SECONDS.toMillis(interval) + " total time: " + totalTime); assertTrue(totalTime < TimeUnit.SECONDS.toMillis(2 * interval), "total time: " + totalTime + " 2 * interval: " + TimeUnit.SECONDS.toMillis(interval)); } - // Test connect retry for database error using Datasource + /** + * Test connect retry, with one retry interval, for database error using Datasource + */ @Test public void testConnectRetryServerErrorDS() { String auth = TestUtils.getProperty(connectionString, "authentication"); @@ -280,10 +286,10 @@ public void testConnectRetryServerErrorDS() { int interval = defaultTimeout; // long interval so we can tell if there was a retry long loginTimeout = defaultTimeout * 2; // long loginTimeout to accommodate the long interval - // non existent database with interval < loginTimeout this will generate a 4060 transient error and retry 1 time + // non-existent database with interval < loginTimeout this will generate a 4060 transient error and retry 2 times SQLServerDataSource ds = new SQLServerDataSource(); String connectStr = TestUtils.addOrOverrideProperty(connectionString, "database", - RandomUtil.getIdentifier("database")) + ";logintimeout=" + loginTimeout + ";connectRetryCount=1" + RandomUtil.getIdentifier("database")) + ";loginTimeout=" + loginTimeout + ";connectRetryCount=2" + ";connectRetryInterval=" + interval; updateDataSource(connectStr, ds); @@ -301,7 +307,7 @@ public void testConnectRetryServerErrorDS() { totalTime = System.currentTimeMillis() - timerStart; } - // 1 retry should be at least 1 interval long but < 2 intervals + // 2 retries should be at least 1 interval long but < 2 intervals (no interval between initial attempt and retry 1) assertTrue(TimeUnit.SECONDS.toMillis(interval) < totalTime, "interval: " + TimeUnit.SECONDS.toMillis(interval) + " total time: " + totalTime); assertTrue(totalTime < TimeUnit.SECONDS.toMillis(2 * interval), diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/BatchExecutionTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/BatchExecutionTest.java index 5a436547c..ba3f6e70c 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/BatchExecutionTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/BatchExecutionTest.java @@ -246,7 +246,6 @@ public void testSqlServerBulkCopyCachingConnectionLevelMultiThreaded() throws Ex TimerTask task = new TimerTask() { public void run() { ((HashMap) bulkcopyCache).clear(); - fail(TestResource.getResource("R_executionTooLong")); } }; Timer timer = new Timer("Timer"); @@ -348,7 +347,7 @@ public void testValidTimezoneForTimestampBatchInsertWithBulkCopy() throws Except public void testValidTimezonesDstTimestampBatchInsertWithBulkCopy() throws Exception { Calendar gmtCal = Calendar.getInstance(TimeZone.getTimeZone("GMT")); - for (String tzId: TimeZone.getAvailableIDs()) { + for (String tzId : TimeZone.getAvailableIDs()) { TimeZone.setDefault(TimeZone.getTimeZone(tzId)); long ms = 1696127400000L; // DST