Skip to content

Commit

Permalink
Fix to allow connection retries to be disabled by setting connectRetr…
Browse files Browse the repository at this point in the history 
…yCount to 0 (#2293)
  • Loading branch information
@lilgreenbird
lilgreenbird authored Feb 29, 2024
1 parent cfb018a commit dc191db
Show file tree
Hide file tree
Showing 4 changed files with 264 additions and 256 deletions.
144 changes: 65 additions & 79 deletions src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 timeForFirstTry = 0; // time it took to do 1st try in ms

boolean useFailoverHost = false;
FailoverInfo tempFailover = null;
Expand Down Expand Up @@ -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) {
timeForFirstTry = (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 * timeForFirstTry) // 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
Expand All @@ -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;
Expand All @@ -3517,19 +3507,23 @@ private void login(String primary, String primaryInstanceName, int primaryPortNu
// 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),
// 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 || (isDBMirroring && (0 == attemptNumber % 2))
&& (attemptNumber < connectRetryCount && connectRetryCount != 0)
&& timerRemaining(
timerExpire) > (TimeUnit.SECONDS.toMillis(connectRetryInterval) + 2 * timeForFirstTry)) {

// 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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())) {
Expand Down Expand Up @@ -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());

Expand All @@ -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,
Expand Down
19 changes: 10 additions & 9 deletions src/main/java/com/microsoft/sqlserver/jdbc/SQLServerException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
43 changes: 42 additions & 1 deletion src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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;connectRetryCount=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;connectRetryCount=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)
Expand Down Expand Up @@ -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) {}
}
};
Expand Down
Loading

0 comments on commit dc191db

Please sign in to comment.