Fix to allow connection retries to be disabled by setting connectRetryCount to 0

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

@@ -3222,6 +3222,21 @@ else if (0 == requestedPacketSize)
         return this;
     }
 
+    // log open connection failures
+    private void logConnectFailure(int attemptNumber, SQLServerException e, SQLServerError sqlServerError) {
+        loggerResiliency.finer(toString() + " Connection open - connection failed on attempt: " + attemptNumber + ".");
+
+        if (e != null) {
+            loggerResiliency.finer(
+                    toString() + " Connection open - connection failure. Driver error code: " + e.getDriverErrorCode());
+        }
+
+        if (null != sqlServerError && !sqlServerError.getErrorMessage().isEmpty()) {
+            loggerResiliency.finer(toString() + " Connection open - connection failure. SQL Server error : "
+                    + sqlServerError.getErrorMessage());
+        }
+    }
+
     /**
      * This function is used by non failover and failover cases. Even when we make a standard connection the server can
      * provide us with its FO partner. If no FO information is available a standard connection is made. If the server
@@ -3237,6 +3252,7 @@ private void login(String primary, String primaryInstanceName, int primaryPortNu
         int fedauthRetryInterval = BACKOFF_INTERVAL; // milliseconds to sleep (back off) between attempts.
 
         long timeoutUnitInterval;
+        long time1stTry = 0; // time it took to do 1st try
 
         boolean useFailoverHost = false;
         FailoverInfo tempFailover = null;
@@ -3434,50 +3450,32 @@ private void login(String primary, String primaryInstanceName, int primaryPortNu
                                     + connectRetryCount + " reached.");
                 }
 
-                int errorCode = e.getErrorCode();
-                int driverErrorCode = e.getDriverErrorCode();
+                // estimate time it took to do 1 try
+                if (attemptNumber == 0) {
+                    time1stTry = (System.currentTimeMillis() - timerStart);
+                }
+
                 sqlServerError = e.getSQLServerError();
 
-                if (SQLServerException.LOGON_FAILED == errorCode // logon failed, ie bad password
-                        || SQLServerException.PASSWORD_EXPIRED == errorCode // password expired
-                        || SQLServerException.USER_ACCOUNT_LOCKED == errorCode // user account locked
-                        || SQLServerException.DRIVER_ERROR_INVALID_TDS == driverErrorCode // invalid TDS
-                        || SQLServerException.DRIVER_ERROR_SSL_FAILED == driverErrorCode // SSL failure
-                        || SQLServerException.DRIVER_ERROR_INTERMITTENT_TLS_FAILED == driverErrorCode // TLS1.2 failure
-                        || SQLServerException.DRIVER_ERROR_UNSUPPORTED_CONFIG == driverErrorCode // unsupported config
-                                                                                                 // (eg Sphinx, invalid
-                                                                                                 // packetsize, etc)
-                        || (SQLServerException.ERROR_SOCKET_TIMEOUT == driverErrorCode // socket timeout
-                                && (!isDBMirroring || attemptNumber > 0)) // If mirroring, only close after failover has been tried (attempt >= 1)
-                        || timerHasExpired(timerExpire)
-                // for non-dbmirroring cases, do not retry after tcp socket connection succeeds
+                if (isFatalError(e) // do not retry on fatal errors
+                        || timerHasExpired(timerExpire) // no time left
+                        || (timerRemaining(timerExpire) < TimeUnit.SECONDS.toMillis(connectRetryInterval)
+                                + 2 * time1stTry) // not enough time for another retry
+                        || (connectRetryCount == 0 && !isDBMirroring && !useTnir) // retries disabled
+                        // retry at least once for TNIR and failover
+                        || (connectRetryCount == 0 && (isDBMirroring || useTnir) && attemptNumber > 0)
+                        || (connectRetryCount != 0 && attemptNumber > connectRetryCount) // no retries left
                 ) {
                     if (loggerResiliency.isLoggable(Level.FINER)) {
-                        loggerResiliency.finer(
-                                toString() + " Connection open - connection failed on attempt: " + attemptNumber + ".");
-                        loggerResiliency.finer(toString() + " Connection open - connection failure. Driver error code: "
-                                + driverErrorCode);
-                        if (null != sqlServerError && !sqlServerError.getErrorMessage().isEmpty()) {
-                            loggerResiliency
-                                    .finer(toString() + " Connection open - connection failure. SQL Server error : "
-                                            + sqlServerError.getErrorMessage());
-                        }
+                        logConnectFailure(attemptNumber, e, sqlServerError);
                     }
 
                     // close the connection and throw the error back
                     close();
                     throw e;
                 } else {
                     if (loggerResiliency.isLoggable(Level.FINER)) {
-                        loggerResiliency.finer(
-                                toString() + " Connection open - connection failed on attempt: " + attemptNumber + ".");
-                        loggerResiliency.finer(toString() + " Connection open - connection failure. Driver error code: "
-                                + driverErrorCode);
-                        if (null != sqlServerError && !sqlServerError.getErrorMessage().isEmpty()) {
-                            loggerResiliency
-                                    .finer(toString() + " Connection open - connection failure. SQL Server error : "
-                                            + sqlServerError.getErrorMessage());
-                        }
+                        logConnectFailure(attemptNumber, e, sqlServerError);
                     }
 
                     // Close the TDS channel from the failed connection attempt so that we don't
@@ -3496,15 +3494,7 @@ private void login(String primary, String primaryInstanceName, int primaryPortNu
                     if (remainingMilliseconds <= fedauthRetryInterval) {
 
                         if (loggerResiliency.isLoggable(Level.FINER)) {
-                            loggerResiliency.finer(toString() + " Connection open - connection failed on attempt: "
-                                    + attemptNumber + ".");
-                            loggerResiliency.finer(toString()
-                                    + " Connection open - connection failure. Driver error code: " + driverErrorCode);
-                            if (null != sqlServerError && !sqlServerError.getErrorMessage().isEmpty()) {
-                                loggerResiliency
-                                        .finer(toString() + " Connection open - connection failure. SQL Server error : "
-                                                + sqlServerError.getErrorMessage());
-                            }
+                            logConnectFailure(attemptNumber, e, sqlServerError);
                         }
 
                         throw e;
@@ -3515,21 +3505,25 @@ private void login(String primary, String primaryInstanceName, int primaryPortNu
             // We only get here when we failed to connect, but are going to re-try
             // After trying to connect to both servers fails, sleep for a bit to prevent clogging
             // the network with requests, then update sleep interval for next iteration (max 1 second interval)
-            // We have to sleep for every attempt in case of non-dbMirroring scenarios (including multisubnetfailover),
+            // We have to sleep for every attempt in case of non-dbMirroring scenarios (including multi subnetfailover),
             // Whereas for dbMirroring, we sleep for every two attempts as each attempt is to a different server.
-            if (!isDBMirroring || (1 == attemptNumber % 2)) {
-                if (loggerResiliency.isLoggable(Level.FINER)) {
-                    loggerResiliency.finer(toString() + " Connection open - sleeping milisec: " + connectRetryInterval);
-                }
-                if (loggerResiliency.isLoggable(Level.FINER)) {
-                    loggerResiliency.finer(toString() + " Connection open - connection failed on transient error "
-                            + (sqlServerError != null ? sqlServerError.getErrorNumber() : "")
-                            + ". Wait for connectRetryInterval(" + connectRetryInterval + ")s before retry #"
-                            + attemptNumber);
-                }
+            // Make sure there's enough time to do another retry
+            if (!isDBMirroring && (0 == attemptNumber % 2) || (isDBMirroring && attemptNumber < 1)
+                    || (attemptNumber < connectRetryCount && connectRetryCount != 0) && timerRemaining(
+                            timerExpire) > (TimeUnit.SECONDS.toMillis(connectRetryInterval) + 2 * time1stTry))
+
+            {
+                // don't wait for TNIR
+                if (!(useTnir && attemptNumber == 0)) {
+                    if (loggerResiliency.isLoggable(Level.FINER)) {
+                        loggerResiliency.finer(toString() + " Connection open - connection failed on transient error "
+                                + (sqlServerError != null ? sqlServerError.getErrorNumber() : "")
+                                + ". Wait for connectRetryInterval(" + connectRetryInterval + ")s before retry #"
+                                + attemptNumber);
+                    }
 
-                sleepForInterval(fedauthRetryInterval);
-                fedauthRetryInterval = (fedauthRetryInterval < 500) ? fedauthRetryInterval * 2 : 1000;
+                    sleepForInterval(TimeUnit.SECONDS.toMillis(connectRetryInterval));
+                }
             }
 
             // Update timeout interval (but no more than the point where we're supposed to fail: timerExpire)
@@ -3622,31 +3616,22 @@ private void login(String primary, String primaryInstanceName, int primaryPortNu
         }
     }
 
+    // non recoverable or retryable fatal errors
     boolean isFatalError(SQLServerException e) {
         /*
          * NOTE: If these conditions are modified, consider modification to conditions in SQLServerConnection::login()
          * and Reconnect::run()
          */
+        int errorCode = e.getErrorCode();
+        int driverErrorCode = e.getDriverErrorCode();
 
-        // actual logon failed (e.g. bad password)
-        if ((SQLServerException.LOGON_FAILED == e.getErrorCode())
-                // actual logon failed (e.g. password expired)
-                || (SQLServerException.PASSWORD_EXPIRED == e.getErrorCode())
-                // actual logon failed (e.g. user account locked)
-                || (SQLServerException.USER_ACCOUNT_LOCKED == e.getErrorCode())
-                // invalid TDS received from server
-                || (SQLServerException.DRIVER_ERROR_INVALID_TDS == e.getDriverErrorCode())
-                // failure negotiating SSL
-                || (SQLServerException.DRIVER_ERROR_SSL_FAILED == e.getDriverErrorCode())
-                // failure TLS1.2
-                || (SQLServerException.DRIVER_ERROR_INTERMITTENT_TLS_FAILED == e.getDriverErrorCode())
-                // unsupported configuration (e.g. Sphinx, invalid packet size, etc.)
-                || (SQLServerException.DRIVER_ERROR_UNSUPPORTED_CONFIG == e.getDriverErrorCode())
-                // no more time to try again
-                || (SQLServerException.ERROR_SOCKET_TIMEOUT == e.getDriverErrorCode()))
-            return true;
-        else
-            return false;
+        return ((SQLServerException.LOGON_FAILED == errorCode) // logon failed (eg bad password)
+                || (SQLServerException.PASSWORD_EXPIRED == errorCode) // password expired
+                || (SQLServerException.USER_ACCOUNT_LOCKED == errorCode) // user account locked
+                || (SQLServerException.DRIVER_ERROR_INVALID_TDS == driverErrorCode) // invalid TDS from server
+                || (SQLServerException.DRIVER_ERROR_SSL_FAILED == driverErrorCode) // SSL failure
+                || (SQLServerException.DRIVER_ERROR_INTERMITTENT_TLS_FAILED == driverErrorCode) // TLS1.2 failure
+                || (SQLServerException.DRIVER_ERROR_UNSUPPORTED_CONFIG == driverErrorCode)); // unsupported config (eg Sphinx, invalid packet size ,etc)
     }
 
     // reset all params that could have been changed due to ENVCHANGE tokens to defaults,
@@ -3707,7 +3692,7 @@ static boolean timerHasExpired(long timerExpire) {
     }
 
     /**
-     * Get time remaining to timer expiry
+     * Get time remaining to timer expiry (in ms)
      * 
      * @param timerExpire
      * @return remaining time to expiry
@@ -5979,7 +5964,7 @@ private SqlAuthenticationToken getFedAuthToken(SqlFedAuthInfo fedAuthInfo) throw
         String user = activeConnectionProperties.getProperty(SQLServerDriverStringProperty.USER.toString());
 
         // No of milliseconds to sleep for the initial back off.
-        int sleepInterval = BACKOFF_INTERVAL;
+        int fedauthSleepInterval = BACKOFF_INTERVAL;
 
         if (!msalContextExists()
                 && !authenticationString.equalsIgnoreCase(SqlAuthentication.ACTIVE_DIRECTORY_INTEGRATED.toString())) {
@@ -6078,7 +6063,7 @@ private SqlAuthenticationToken getFedAuthToken(SqlFedAuthInfo fedAuthInfo) throw
 
                         int millisecondsRemaining = timerRemaining(timerExpire);
                         if (ActiveDirectoryAuthentication.GET_ACCESS_TOKEN_TRANSIENT_ERROR != errorCategory
-                                || timerHasExpired(timerExpire) || (sleepInterval >= millisecondsRemaining)) {
+                                || timerHasExpired(timerExpire) || (fedauthSleepInterval >= millisecondsRemaining)) {
 
                             String errorStatus = Integer.toHexString(adalException.getStatus());
 
@@ -6102,13 +6087,14 @@ private SqlAuthenticationToken getFedAuthToken(SqlFedAuthInfo fedAuthInfo) throw
 
                         if (connectionlogger.isLoggable(Level.FINER)) {
                             connectionlogger.fine(toString() + " SQLServerConnection.getFedAuthToken sleeping: "
-                                    + sleepInterval + " milliseconds.");
+                                    + fedauthSleepInterval + " milliseconds.");
                             connectionlogger.fine(toString() + " SQLServerConnection.getFedAuthToken remaining: "
                                     + millisecondsRemaining + " milliseconds.");
                         }
 
-                        sleepForInterval(sleepInterval);
-                        sleepInterval = sleepInterval * 2;
+                        sleepForInterval(fedauthSleepInterval);
+                        fedauthSleepInterval = (fedauthSleepInterval < 500) ? fedauthSleepInterval * 2 : 1000;
+
                     }
                 }
                 // else choose MSAL4J for integrated authentication. This option is supported for both windows and unix,

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

@@ -395,16 +395,17 @@ static String generateStateCode(SQLServerConnection con, int errNum, Integer dat
     static String checkAndAppendClientConnId(String errMsg, SQLServerConnection conn) {
         if (null != conn && conn.isConnected()) {
             UUID clientConnId = conn.getClientConIdInternal();
-            assert null != clientConnId;
-            StringBuilder sb = new StringBuilder(errMsg);
-            // This syntax of adding connection id is matched in a retry logic. If anything changes here, make
-            // necessary changes to enableSSL() function's exception handling mechanism.
-            sb.append(LOG_CLIENT_CONNECTION_ID_PREFIX);
-            sb.append(clientConnId.toString());
-            return sb.toString();
-        } else {
-            return errMsg;
+            if (null != clientConnId) {
+                StringBuilder sb = (errMsg != null) ? new StringBuilder(errMsg) : new StringBuilder();
+                // This syntax of adding connection id is matched in a retry logic. If anything changes here, make
+                // necessary changes to enableSSL() function's exception handling mechanism.
+                sb.append(LOG_CLIENT_CONNECTION_ID_PREFIX);
+                sb.append(clientConnId.toString());
+                return sb.toString();
+            }
         }
+        return (errMsg != null) ? errMsg : "";
+
     }
 
     static void throwNotSupportedException(SQLServerConnection con, Object obj) throws SQLServerException {

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

@@ -53,6 +53,7 @@ public class SQLServerConnectionTest extends AbstractTest {
     // If no retry is done, the function should at least exit in 5 seconds
     static int threshHoldForNoRetryInMilliseconds = 5000;
     static int loginTimeOutInSeconds = 10;
+    static String tnirHost = getConfiguredProperty("tnirHost");
 
     String randomServer = RandomUtil.getIdentifier("Server");
 
@@ -489,6 +490,44 @@ public void testConnectCountInLoginAndCorrectRetryCount() {
         }
     }
 
+    // Test connect retry 0 but should still connect to TNIR
+    @Test
+    @Tag(Constants.xAzureSQLDW)
+    @Tag(Constants.xAzureSQLDB)
+    @Tag(Constants.reqExternalSetup)
+    public void testConnectTnir() {
+        org.junit.Assume.assumeTrue(isWindows);
+
+        // no retries but should connect to TNIR (this assumes host is defined in host file
+        try (Connection con = PrepUtil
+                .getConnection(connectionString + ";transparentNetworkIPResolution=true;conneectRetry=0;serverName="
+                        + tnirHost);) {} catch (Exception e) {
+            fail(e.getMessage());
+        }
+    }
+
+    // Test connect retry 0 and TNIR disabled
+    @Test
+    @Tag(Constants.xAzureSQLDW)
+    @Tag(Constants.xAzureSQLDB)
+    @Tag(Constants.reqExternalSetup)
+    public void testConnectNoTnir() {
+        org.junit.Assume.assumeTrue(isWindows);
+
+        // no retries no TNIR should fail even tho host is defined in host file
+        try (Connection con = PrepUtil.getConnection(
+                connectionString + ";transparentNetworkIPResolution=false;conneectRetry=0;serverName=" + tnirHost);) {
+            assertTrue(con == null, TestResource.getResource("R_shouldNotConnect"));
+        } catch (Exception e) {
+            assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_tcpipConnectionFailed"))
+                    || ((isSqlAzure() || isSqlAzureDW())
+                                                         ? e.getMessage().contains(
+                                                                 TestResource.getResource("R_connectTimedOut"))
+                                                         : false),
+                    e.getMessage());
+        }
+    }
+
     @Test
     @Tag(Constants.xAzureSQLDW)
     @Tag(Constants.xAzureSQLDB)
@@ -981,7 +1020,9 @@ public void run() {
 
                 ds.setURL(connectionString);
                 ds.setServerName("invalidServerName" + UUID.randomUUID());
-                ds.setLoginTimeout(5);
+                ds.setLoginTimeout(30);
+                ds.setConnectRetryCount(3);
+                ds.setConnectRetryInterval(10);
                 try (Connection con = ds.getConnection()) {} catch (SQLException e) {}
             }
         };