From 4ec4d3b4b6bf42a5c9f3d5b2e6a3743ef7df5506 Mon Sep 17 00:00:00 2001 From: Jeff Wasty Date: Mon, 23 Sep 2024 14:20:28 -0700 Subject: [PATCH 01/14] Configurable Retry Logic I - Statement Retry (#2396) * Trying on a new branch b/c I keep getting build failures and I have no idea why... * Adding back retryExec * More * Missing null check * Next to final * Removed mssql-jdbc.properties * Set up should start fresh + remove passwords to pass on pipeline * Minor cleanup * Minor cleanup * Another missing null check * Fix for timeout tests * Added timing tests + test comments * Formatting * Added a multiple rules test * Trying on a new branch b/c I keep getting build failures and I have no idea why... * More changes * Undo LimitEscapeTest changes * Remove redundant files * Final? * Remove mssql-jdpc.properties file * sync --> lock * Remove problematic test * Since error is unclear, try removing last test * Adding back connection test * I need debugging * Fix for MI * if condition for min time assertion * Leftover debug code, cleanup * Mistaken changes committed * More liberal time windows * Remove connection part * Missed some parts where connection retry was still included. * Forgot one more part * Added (most) PR comment revisions. * Add comments for specified and public facing methods * Added a missing test * More tests * Added more missing tests * Resolve retryCount test failure * Remove eaten exceptions * Removed the file not found exception as we read for file in all cases, not just when using CRL * Added a proper file read * Delete mssql-jdbc.properties * Added more coverage and minor fixes, ready for review again * Fixed read file test * Addressed recent pr comments * Remove double locking * Remove unneeded variable * Revisions after PR review * PR review update * Rename R_AKVURLInvalid as its use is no longer AKV specific * Add back logging * Typo * Removed unneeded comment * Make static variables thread-safe * Timing * JavaDoc cleanup. --- .../jdbc/ConfigurableRetryLogic.java | 292 +++++++++++ .../sqlserver/jdbc/ConfigurableRetryRule.java | 262 ++++++++++ .../sqlserver/jdbc/ISQLServerDataSource.java | 16 + ...ColumnEncryptionAzureKeyVaultProvider.java | 2 +- .../sqlserver/jdbc/SQLServerConnection.java | 31 ++ .../sqlserver/jdbc/SQLServerDataSource.java | 10 + .../sqlserver/jdbc/SQLServerDriver.java | 5 +- .../jdbc/SQLServerPreparedStatement.java | 1 + .../sqlserver/jdbc/SQLServerResource.java | 6 +- .../sqlserver/jdbc/SQLServerStatement.java | 74 ++- .../JDBCEncryptionDecryptionTest.java | 2 +- .../jdbc/SQLServerConnectionTest.java | 4 +- .../ConfigurableRetryLogicTest.java | 472 ++++++++++++++++++ .../RequestBoundaryMethodsTest.java | 2 + 14 files changed, 1160 insertions(+), 19 deletions(-) create mode 100644 src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryLogic.java create mode 100644 src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryRule.java create mode 100644 src/test/java/com/microsoft/sqlserver/jdbc/configurableretry/ConfigurableRetryLogicTest.java diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryLogic.java b/src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryLogic.java new file mode 100644 index 000000000..ed5591034 --- /dev/null +++ b/src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryLogic.java @@ -0,0 +1,292 @@ +/* + * Microsoft JDBC Driver for SQL Server Copyright(c) Microsoft Corporation All rights reserved. This program is made + * available under the terms of the MIT License. See the LICENSE file in the project root for more information. + */ + +package com.microsoft.sqlserver.jdbc; + +import java.io.BufferedReader; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.FileReader; +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.text.MessageFormat; +import java.util.Collections; +import java.util.Date; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + + +/** + * Allows configurable statement retry through the use of the 'retryExec' connection property. Each rule read in is + * converted to ConfigRetryRule objects, which are stored and referenced during statement retry. + */ +public class ConfigurableRetryLogic { + private static final int INTERVAL_BETWEEN_READS_IN_MS = 30000; + private static final String DEFAULT_PROPS_FILE = "mssql-jdbc.properties"; + private static final Lock CRL_LOCK = new ReentrantLock(); + private static final java.util.logging.Logger CONFIGURABLE_RETRY_LOGGER = java.util.logging.Logger + .getLogger("com.microsoft.sqlserver.jdbc.ConfigurableRetryLogic"); + private static final String SEMI_COLON = ";"; + private static final String COMMA = ","; + private static final String FORWARD_SLASH = "/"; + private static final String EQUALS_SIGN = "="; + private static final String RETRY_EXEC = "retryExec"; + /** + * The time the properties file was last modified. + */ + private static final AtomicLong timeLastModified = new AtomicLong(0); + /** + * The time we last read the properties file. + */ + private static final AtomicLong timeLastRead = new AtomicLong(0); + /** + * The last query executed (used when rule is process-dependent). + */ + private static final AtomicReference lastQuery = new AtomicReference<>(""); + /** + * The previously read rules from the connection string. + */ + private static final AtomicReference prevRulesFromConnectionString = new AtomicReference<>(""); + /** + * The list of statement retry rules. + */ + private static final AtomicReference> stmtRules = new AtomicReference<>( + new HashMap<>()); + private static ConfigurableRetryLogic singleInstance; + + /** + * Constructs the ConfigurableRetryLogic object reading rules from available sources. + * + * @throws SQLServerException + * if unable to construct + */ + private ConfigurableRetryLogic() throws SQLServerException { + timeLastRead.compareAndSet(0, new Date().getTime()); + setUpRules(null); + } + + /** + * Fetches the static instance of ConfigurableRetryLogic, instantiating it if it hasn't already been. Each time the + * instance is fetched, we check if a re-read is needed, and do so if properties should be re-read. + * + * @return the static instance of ConfigurableRetryLogic + * @throws SQLServerException + * an exception + */ + public static ConfigurableRetryLogic getInstance() throws SQLServerException { + if (singleInstance == null) { + CRL_LOCK.lock(); + try { + if (singleInstance == null) { + singleInstance = new ConfigurableRetryLogic(); + } else { + refreshRuleSet(); + } + } finally { + CRL_LOCK.unlock(); + } + } else { + refreshRuleSet(); + } + + return singleInstance; + } + + /** + * If it has been INTERVAL_BETWEEN_READS_IN_MS (30 secs) since last read, see if we last did a file read, if so + * only reread if the file has been modified. If no file read, set up rules using the prev. connection string rules. + * + * @throws SQLServerException + * when an exception occurs + */ + private static void refreshRuleSet() throws SQLServerException { + long currentTime = new Date().getTime(); + + if ((currentTime - timeLastRead.get()) >= INTERVAL_BETWEEN_READS_IN_MS) { + timeLastRead.set(currentTime); + if (timeLastModified.get() != 0) { + // If timeLastModified is set, we previously read from file, so we setUpRules also reading from file + File f = new File(getCurrentClassPath()); + if (f.lastModified() != timeLastModified.get()) { + setUpRules(null); + } + } else { + setUpRules(prevRulesFromConnectionString.get()); + } + } + } + + /** + * Sets rules given from connection string. + * + * @param newRules + * the new rules to use + * @throws SQLServerException + * when an exception occurs + */ + void setFromConnectionString(String newRules) throws SQLServerException { + prevRulesFromConnectionString.set(newRules); + setUpRules(prevRulesFromConnectionString.get()); + } + + /** + * Stores last query executed. + * + * @param newQueryToStore + * the new query to store + */ + void storeLastQuery(String newQueryToStore) { + lastQuery.set(newQueryToStore.toLowerCase()); + } + + /** + * Gets last query. + * + * @return the last query + */ + String getLastQuery() { + return lastQuery.get(); + } + + /** + * Sets up rules based on either connection string option or file read. + * + * @param cxnStrRules + * if null, rules are constructed from file, else, this parameter is used to construct rules + * @throws SQLServerException + * if an exception occurs + */ + private static void setUpRules(String cxnStrRules) throws SQLServerException { + LinkedList temp; + + stmtRules.set(new HashMap<>()); + lastQuery.set(""); + + if (cxnStrRules == null || cxnStrRules.isEmpty()) { + temp = readFromFile(); + } else { + temp = new LinkedList<>(); + Collections.addAll(temp, cxnStrRules.split(SEMI_COLON)); + } + createRules(temp); + } + + /** + * Creates and stores rules based on the inputted list of rules. + * + * @param listOfRules + * the list of rules, as a String LinkedList + * @throws SQLServerException + * if unable to create rules from the inputted list + */ + private static void createRules(LinkedList listOfRules) throws SQLServerException { + stmtRules.set(new HashMap<>()); + + for (String potentialRule : listOfRules) { + ConfigurableRetryRule rule = new ConfigurableRetryRule(potentialRule); + + if (rule.getError().contains(COMMA)) { + String[] arr = rule.getError().split(COMMA); + + for (String retryError : arr) { + ConfigurableRetryRule splitRule = new ConfigurableRetryRule(retryError, rule); + stmtRules.get().put(Integer.parseInt(splitRule.getError()), splitRule); + } + } else { + stmtRules.get().put(Integer.parseInt(rule.getError()), rule); + } + } + } + + /** + * Gets the current class path (for use in file reading). + * + * @return the current class path, as a String + * @throws SQLServerException + * if unable to retrieve the current class path + */ + private static String getCurrentClassPath() throws SQLServerException { + String location = ""; + String className = ""; + + try { + className = new Object() {}.getClass().getEnclosingClass().getName(); + location = Class.forName(className).getProtectionDomain().getCodeSource().getLocation().getPath(); + location = location.substring(0, location.length() - 16); + URI uri = new URI(location + FORWARD_SLASH); + return uri.getPath() + DEFAULT_PROPS_FILE; // For now, we only allow "mssql-jdbc.properties" as file name. + } catch (URISyntaxException e) { + MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_URLInvalid")); + Object[] msgArgs = {location + FORWARD_SLASH}; + throw new SQLServerException(form.format(msgArgs), null, 0, e); + } catch (ClassNotFoundException e) { + MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_UnableToFindClass")); + Object[] msgArgs = {className}; + throw new SQLServerException(form.format(msgArgs), null, 0, e); + } + } + + /** + * Attempts to read rules from the properties file. + * + * @return the list of rules as a LinkedList + * @throws SQLServerException + * if unable to read from the file + */ + private static LinkedList readFromFile() throws SQLServerException { + String filePath = getCurrentClassPath(); + LinkedList list = new LinkedList<>(); + + try { + File f = new File(filePath); + try (BufferedReader buffer = new BufferedReader(new FileReader(f))) { + String readLine; + while ((readLine = buffer.readLine()) != null) { + if (readLine.startsWith(RETRY_EXEC)) { + String value = readLine.split(EQUALS_SIGN)[1]; + Collections.addAll(list, value.split(SEMI_COLON)); + } + } + } + timeLastModified.set(f.lastModified()); + } catch (FileNotFoundException e) { + // If the file is not found either A) We're not using CRL OR B) the path is wrong. Do not error out, instead + // log a message. + if (CONFIGURABLE_RETRY_LOGGER.isLoggable(java.util.logging.Level.FINER)) { + CONFIGURABLE_RETRY_LOGGER.finest("File not found at path - \"" + filePath + "\""); + } + } catch (IOException e) { + MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_errorReadingStream")); + Object[] msgArgs = {e.getMessage() + ", from path - \"" + filePath + "\""}; + throw new SQLServerException(form.format(msgArgs), null, 0, e); + } + return list; + } + + /** + * Searches rule set for the given rule. + * + * @param ruleToSearchFor + * the rule to search for + * @return the configurable retry rule + * @throws SQLServerException + * when an exception occurs + */ + ConfigurableRetryRule searchRuleSet(int ruleToSearchFor) throws SQLServerException { + refreshRuleSet(); + for (Map.Entry entry : stmtRules.get().entrySet()) { + if (entry.getKey() == ruleToSearchFor) { + return entry.getValue(); + } + } + return null; + } +} diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryRule.java b/src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryRule.java new file mode 100644 index 000000000..f52df8d8d --- /dev/null +++ b/src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryRule.java @@ -0,0 +1,262 @@ +/* + * Microsoft JDBC Driver for SQL Server Copyright(c) Microsoft Corporation All rights reserved. This program is made + * available under the terms of the MIT License. See the LICENSE file in the project root for more information. + */ + +package com.microsoft.sqlserver.jdbc; + +import java.text.MessageFormat; +import java.util.ArrayList; + + +/** + * The ConfigRetryRule object is what is used by the ConfigurableRetryLogic class to handle statement retries. Each + * ConfigRetryRule object allows for one rule. + * + */ +class ConfigurableRetryRule { + private final String PLUS_SIGN = "+"; + private final String MULTIPLICATION_SIGN = "*"; + private final String COMMA = ","; + private final String ZERO = "0"; + private String operand = "+"; + private int initialRetryTime = 0; + private int retryChange = 2; + private int retryCount = 1; + private String retryQueries = ""; + private String retryError; + + private ArrayList waitTimes = new ArrayList<>(); + + /** + * Construct a ConfigurableRetryRule object from a String rule. + * + * @param rule + * the rule used to construct the ConfigRetryRule object + * @throws SQLServerException + * if there is a problem parsing the rule + */ + ConfigurableRetryRule(String rule) throws SQLServerException { + addElements(removeExtraElementsAndSplitRule(rule)); + calculateWaitTimes(); + } + + /** + * Allows constructing a ConfigRetryRule object from another ConfigRetryRule object. Used when the first object has + * multiple errors provided. We pass in the multi-error object and create 1 new object for each error in the initial + * object. + * + * @param newRule + * the rule used to construct the ConfigRetryRule object + * @param baseRule + * the ConfigRetryRule object to base the new objects off of + */ + ConfigurableRetryRule(String newRule, ConfigurableRetryRule baseRule) { + copyFromRule(baseRule); + this.retryError = newRule; + } + + /** + * Copy elements from the base rule to this rule. + * + * @param baseRule + * the rule to copy elements from + */ + private void copyFromRule(ConfigurableRetryRule baseRule) { + this.retryError = baseRule.retryError; + this.operand = baseRule.operand; + this.initialRetryTime = baseRule.initialRetryTime; + this.retryChange = baseRule.retryChange; + this.retryCount = baseRule.retryCount; + this.retryQueries = baseRule.retryQueries; + this.waitTimes = baseRule.waitTimes; + } + + /** + * Removes extra elements in the rule (e.g. '{') and splits the rule based on ':' (colon). + * + * @param rule + * the rule to format and split + * @return the split rule as a string array + */ + private String[] removeExtraElementsAndSplitRule(String rule) { + if (rule.endsWith(":")) { + rule = rule + ZERO; // Add a zero to make below parsing easier + } + + rule = rule.replace("{", ""); + rule = rule.replace("}", ""); + rule = rule.trim(); + + return rule.split(":"); // Split on colon + } + + /** + * Checks if the value passed in is numeric. In the case where the value contains a comma, the value must be a + * multi-error value, e.g. 2714,2716. This must be separated, and each error checked separately. + * + * @param value + * the value to be checked + * @throws SQLServerException + * if a non-numeric value is passed in + */ + private void checkParameter(String value) throws SQLServerException { + if (!StringUtils.isNumeric(value)) { + String[] arr = value.split(COMMA); + for (String error : arr) { + if (!StringUtils.isNumeric(error)) { + MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_invalidParameterNumber")); + Object[] msgArgs = {error}; + throw new SQLServerException(null, form.format(msgArgs), null, 0, true); + } + } + } + } + + /** + * Parses the passed in string array, containing all elements from the original rule, and assigns the information + * to the class variables. The logic is as follows: + *

+ *

+ * The rule array, which was created by splitting the rule string based on ":", must be of length 2 or 3. If not + * there are too many parts, and an error is thrown. + *

+ *

+ * If it is of length 2 or 3, the first part is always the retry error (the error to retry on). We check if its + * numeric, and if so, assign it to the class variable. The second part are the retry timings, which include + * retry count (mandatory), initial retry time (optional), operand (optional), and retry change (optional). A + * parameter can only be included, if ALL parameters prior to it are included. Thus, these are the only valid rule + * formats for rules of length 2: + * error; count + * error; count, initial retry time + * error; count, initial retry time [OPERAND] + * error; count, initial retry time [OPERAND] retry change + *

+ *

+ * Next, the second part of the rule is parsed based on "," and each part checked. The retry count is mandatory + * and must be numeric and greater than 0, else an error is thrown. + *

+ *

+ * If there is a second part to the retry timings, it includes any of the parameters mentioned above: initial retry + * time, operand, and retry change. We first check if there is an operand, if not, then only initial retry time has + * been given, and it is assigned. If there is an operand, we split this second part based on the operand. + * Whatever was before the operand was the initial retry time, and if there was something after the operand, this + * is the retry change. If there are more than 2 parts to the timing, i.e. more than 2 commas, throw an error. + *

+ *

+ * Finally, if the rule has 3 parts, it includes a query specifier, parse this and assign it. + * + * @param rule + * the passed in rule, as a string array + * @throws SQLServerException + * if a rule or parameter has invalid inputs + */ + private void addElements(String[] rule) throws SQLServerException { + if (rule.length == 2 || rule.length == 3) { + checkParameter(rule[0]); + retryError = rule[0]; + String[] timings = rule[1].split(COMMA); + checkParameter(timings[0]); + retryCount = Integer.parseInt(timings[0]); + + if (timings.length == 2) { + if (timings[1].contains(MULTIPLICATION_SIGN)) { + String[] initialAndChange = timings[1].split("\\*"); + checkParameter(initialAndChange[0]); + + initialRetryTime = Integer.parseInt(initialAndChange[0]); + operand = MULTIPLICATION_SIGN; + if (initialAndChange.length > 1) { + checkParameter(initialAndChange[1]); + retryChange = Integer.parseInt(initialAndChange[1]); + } else { + retryChange = initialRetryTime; + } + } else if (timings[1].contains(PLUS_SIGN)) { + String[] initialAndChange = timings[1].split("\\+"); + checkParameter(initialAndChange[0]); + + initialRetryTime = Integer.parseInt(initialAndChange[0]); + operand = PLUS_SIGN; + if (initialAndChange.length > 1) { + checkParameter(initialAndChange[1]); + retryChange = Integer.parseInt(initialAndChange[1]); + } else { + retryChange = 2; + } + } else { + checkParameter(timings[1]); + initialRetryTime = Integer.parseInt(timings[1]); + } + } else if (timings.length > 2) { + MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_invalidParameterNumber")); + Object[] msgArgs = {rule[1]}; + throw new SQLServerException(null, form.format(msgArgs), null, 0, true); + } + + if (rule.length == 3) { + retryQueries = (rule[2].equals(ZERO) ? "" : rule[2].toLowerCase()); + } + } else { + MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_InvalidRuleFormat")); + Object[] msgArgs = {rule.length}; + throw new SQLServerException(null, form.format(msgArgs), null, 0, true); + } + } + + /** + * Calculates all the 'wait times', i.e. how long the driver waits between re-execution of statement, based + * on the parameters in the rule. Saves all these times in "waitTimes" to be referenced during statement re-execution. + */ + private void calculateWaitTimes() { + for (int i = 0; i < retryCount; ++i) { + int waitTime = initialRetryTime; + if (operand.equals(PLUS_SIGN)) { + for (int j = 0; j < i; ++j) { + waitTime += retryChange; + } + } else if (operand.equals(MULTIPLICATION_SIGN)) { + for (int k = 0; k < i; ++k) { + waitTime *= retryChange; + } + } + waitTimes.add(waitTime); + } + } + + /** + * Returns the retry error for this ConfigRetryRule object. + * + * @return the retry error + */ + String getError() { + return retryError; + } + + /** + * Returns the retry count (amount of times to retry) for this ConfigRetryRule object. + * + * @return the retry count + */ + int getRetryCount() { + return retryCount; + } + + /** + * Returns the retry query specifier for this ConfigRetryRule object. + * + * @return the retry query specifier + */ + String getRetryQueries() { + return retryQueries; + } + + /** + * Returns an array listing the waiting times between each retry, for this ConfigRetryRule object. + * + * @return the list of waiting times + */ + ArrayList getWaitTimes() { + return waitTimes; + } +} diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerDataSource.java b/src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerDataSource.java index b9ab8027f..66fc33744 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerDataSource.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerDataSource.java @@ -1346,6 +1346,22 @@ public interface ISQLServerDataSource extends javax.sql.CommonDataSource { * @return cacheBulkCopyMetadata boolean value */ boolean getcacheBulkCopyMetadata(); + + /** + * Returns value of 'retryExec' from Connection String. + * + * @param retryExec + * Set of rules used for statement (execution) retry + */ + void setRetryExec(String retryExec); + + /** + * Sets the value for 'retryExec' property + * + * @return retryExec + * String value + */ + String getRetryExec(); /** * useFlexibleCallableStatements is temporarily removed. This is meant as a no-op. diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerColumnEncryptionAzureKeyVaultProvider.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerColumnEncryptionAzureKeyVaultProvider.java index 42281688a..082028f2c 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerColumnEncryptionAzureKeyVaultProvider.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerColumnEncryptionAzureKeyVaultProvider.java @@ -645,7 +645,7 @@ private void validateNonEmptyAKVPath(String masterKeyPath) throws SQLServerExcep } } } catch (URISyntaxException e) { - MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_AKVURLInvalid")); + MessageFormat form = new MessageFormat(SQLServerException.getErrString("R_URLInvalid")); Object[] msgArgs = {masterKeyPath}; throw new SQLServerException(form.format(msgArgs), null, 0, e); } diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index d81e3da9e..903adc0b0 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -1064,6 +1064,28 @@ public void setCalcBigDecimalPrecision(boolean calcBigDecimalPrecision) { this.calcBigDecimalPrecision = calcBigDecimalPrecision; } + private String retryExec = SQLServerDriverStringProperty.RETRY_EXEC.getDefaultValue(); + + /** + * Returns the set of configurable statement retry rules set in retryExec + * + * @return + * A string containing statement retry rules. + */ + public String getRetryExec() { + return retryExec; + } + + /** + * Sets the list of configurable statement retry rules, for the given connection, in retryExec. + * + * @param retryExec + * The list of retry rules to set, as a string. + */ + public void setRetryExec(String retryExec) { + this.retryExec = retryExec; + } + /** Session Recovery Object */ private transient IdleConnectionResiliency sessionRecovery = new IdleConnectionResiliency(this); @@ -2338,6 +2360,15 @@ Connection connectInternal(Properties propsIn, IPAddressPreference.valueOfString(sPropValue).toString()); } + sPropKey = SQLServerDriverStringProperty.RETRY_EXEC.toString(); + sPropValue = activeConnectionProperties.getProperty(sPropKey); + if (null == sPropValue) { + sPropValue = SQLServerDriverStringProperty.RETRY_EXEC.getDefaultValue(); + activeConnectionProperties.setProperty(sPropKey, sPropValue); + } + retryExec = sPropValue; + ConfigurableRetryLogic.getInstance().setFromConnectionString(sPropValue); + sPropKey = SQLServerDriverBooleanProperty.CALC_BIG_DECIMAL_PRECISION.toString(); sPropValue = activeConnectionProperties.getProperty(sPropKey); if (null == sPropValue) { diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java index e27845a0d..6136cffa0 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java @@ -1383,6 +1383,16 @@ public boolean getCalcBigDecimalPrecision() { SQLServerDriverBooleanProperty.CALC_BIG_DECIMAL_PRECISION.getDefaultValue()); } + @Override + public void setRetryExec(String retryExec) { + setStringProperty(connectionProps, SQLServerDriverStringProperty.RETRY_EXEC.toString(), retryExec); + } + + @Override + public String getRetryExec() { + return getStringProperty(connectionProps, SQLServerDriverStringProperty.RETRY_EXEC.toString(), null); + } + /** * Sets a property string value. * diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java index 0e9689216..1c9abc000 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java @@ -610,7 +610,8 @@ enum SQLServerDriverStringProperty { ENCRYPT("encrypt", EncryptOption.TRUE.toString()), SERVER_CERTIFICATE("serverCertificate", ""), DATETIME_DATATYPE("datetimeParameterType", DatetimeType.DATETIME2.toString()), - ACCESS_TOKEN_CALLBACK_CLASS("accessTokenCallbackClass", ""); + ACCESS_TOKEN_CALLBACK_CLASS("accessTokenCallbackClass", ""), + RETRY_EXEC("retryExec", ""); private final String name; private final String defaultValue; @@ -852,6 +853,8 @@ public final class SQLServerDriver implements java.sql.Driver { SQLServerDriverObjectProperty.ACCESS_TOKEN_CALLBACK.getDefaultValue(), false, null), new SQLServerDriverPropertyInfo(SQLServerDriverStringProperty.ACCESS_TOKEN_CALLBACK_CLASS.toString(), SQLServerDriverStringProperty.ACCESS_TOKEN_CALLBACK_CLASS.getDefaultValue(), false, null), + new SQLServerDriverPropertyInfo(SQLServerDriverStringProperty.RETRY_EXEC.toString(), + SQLServerDriverStringProperty.RETRY_EXEC.getDefaultValue(), false, null), new SQLServerDriverPropertyInfo(SQLServerDriverBooleanProperty.REPLICATION.toString(), Boolean.toString(SQLServerDriverBooleanProperty.REPLICATION.getDefaultValue()), false, TRUE_FALSE), new SQLServerDriverPropertyInfo(SQLServerDriverBooleanProperty.SEND_TIME_AS_DATETIME.toString(), diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java index a5f50b14a..2c2bbcb9a 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java @@ -567,6 +567,7 @@ public boolean execute() throws SQLServerException, SQLTimeoutException { loggerExternal.finer(toString() + ACTIVITY_ID + ActivityCorrelator.getCurrent().toString()); } checkClosed(); + ConfigurableRetryLogic.getInstance().storeLastQuery(this.userSQL); connection.unprepareUnreferencedPreparedStatementHandles(false); executeStatement(new PrepStmtExecCmd(this, EXECUTE)); loggerExternal.exiting(getClassNameLogging(), "execute", null != resultSet); diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java index 64e140534..55a5a5fc0 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java @@ -370,8 +370,8 @@ protected Object[][] getContents() { {"R_ForceEncryptionTrue_HonorAEFalseRS", "Cannot set Force Encryption to true for parameter {0} because encryption is not enabled for the statement or procedure."}, {"R_ForceEncryptionTrue_HonorAETrue_UnencryptedColumnRS", "Cannot execute update because Force Encryption was set as true for parameter {0} and the database expects this parameter to be sent as plaintext. This may be due to a configuration error."}, {"R_NullValue", "{0} cannot be null."}, + {"R_URLInvalid", "Invalid URL specified: {0}."}, {"R_AKVPathNull", "Azure Key Vault key path cannot be null."}, - {"R_AKVURLInvalid", "Invalid URL specified: {0}."}, {"R_AKVMasterKeyPathInvalid", "Invalid Azure Key Vault key path specified: {0}."}, {"R_ManagedIdentityInitFail", "Failed to initialize package to get Managed Identity token for Azure Key Vault."}, {"R_EmptyCEK", "Empty column encryption key specified."}, @@ -517,6 +517,7 @@ protected Object[][] getContents() { {"R_InvalidCSVQuotes", "Failed to parse the CSV file, verify that the fields are correctly enclosed in double quotes."}, {"R_TokenRequireUrl", "Token credentials require a URL using the HTTPS protocol scheme."}, {"R_calcBigDecimalPrecisionPropertyDescription", "Indicates whether the driver should calculate precision for big decimal values."}, + {"R_retryExecPropertyDescription", "List of rules to follow for configurable retry logic."}, {"R_maxResultBufferPropertyDescription", "Determines maximum amount of bytes that can be read during retrieval of result set"}, {"R_maxResultBufferInvalidSyntax", "Invalid syntax: {0} in maxResultBuffer parameter."}, {"R_maxResultBufferNegativeParameterValue", "MaxResultBuffer must have positive value: {0}."}, @@ -545,6 +546,9 @@ protected Object[][] getContents() { {"R_InvalidSqlQuery", "Invalid SQL Query: {0}"}, {"R_InvalidScale", "Scale of input value is larger than the maximum allowed by SQL Server."}, {"R_colCountNotMatchColTypeCount", "Number of provided columns {0} does not match the column data types definition {1}."}, + {"R_InvalidRuleFormat", "Wrong number of parameters supplied to rule. Number of parameters: {0}, expected: 2 or 3."}, + {"R_InvalidRetryInterval", "Current retry interval: {0}, is longer than queryTimeout: {1}."}, + {"R_UnableToFindClass", "Unable to locate specified class: {0}"}, }; } // @formatter:on diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java index b599856a7..883320640 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java @@ -21,6 +21,7 @@ import java.util.Stack; import java.util.StringTokenizer; import java.util.Vector; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -241,22 +242,66 @@ final void executeStatement(TDSCommand newStmtCmd) throws SQLServerException, SQ execProps = new ExecuteProperties(this); - try { - // (Re)execute this Statement with the new command - executeCommand(newStmtCmd); - } catch (SQLServerException e) { - if (e.getDriverErrorCode() == SQLServerException.ERROR_QUERY_TIMEOUT) { - if (e.getCause() == null) { - throw new SQLTimeoutException(e.getMessage(), e.getSQLState(), e.getErrorCode(), e); + boolean cont; + int retryAttempt = 0; + ConfigurableRetryLogic crl = ConfigurableRetryLogic.getInstance(); + + do { + cont = false; + try { + // (Re)execute this Statement with the new command + executeCommand(newStmtCmd); + } catch (SQLServerException e) { + SQLServerError sqlServerError = e.getSQLServerError(); + ConfigurableRetryRule rule = null; + + if (null != sqlServerError) { + rule = crl.searchRuleSet(e.getSQLServerError().getErrorNumber()); + } + + // If there is a rule for this error AND we still have retries remaining THEN we can proceed, otherwise + // first check for query timeout, and then throw the error if queryTimeout was not reached + if (null != rule && retryAttempt < rule.getRetryCount()) { + + // Also check if the last executed statement matches the query constraint passed in for the rule. + // Defaults to true, changed to false if the query does NOT match. + boolean matchesDefinedQuery = true; + if (!(rule.getRetryQueries().isEmpty())) { + + matchesDefinedQuery = rule.getRetryQueries().contains(crl.getLastQuery().split(" ")[0]); + } + + if (matchesDefinedQuery) { + int timeToWait = rule.getWaitTimes().get(retryAttempt); + int queryTimeout = connection.getQueryTimeoutSeconds(); + if (queryTimeout >= 0 && timeToWait > queryTimeout) { + MessageFormat form = new MessageFormat( + SQLServerException.getErrString("R_InvalidRetryInterval")); + Object[] msgArgs = {timeToWait, queryTimeout}; + throw new SQLServerException(null, form.format(msgArgs), null, 0, true); + } + try { + Thread.sleep(TimeUnit.SECONDS.toMillis(timeToWait)); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + } + cont = true; + retryAttempt++; + } + } else if (e.getDriverErrorCode() == SQLServerException.ERROR_QUERY_TIMEOUT) { + if (e.getCause() == null) { + throw new SQLTimeoutException(e.getMessage(), e.getSQLState(), e.getErrorCode(), e); + } + throw new SQLTimeoutException(e.getMessage(), e.getSQLState(), e.getErrorCode(), e.getCause()); + } else { + throw e; + } + } finally { + if (newStmtCmd.wasExecuted()) { + lastStmtExecCmd = newStmtCmd; } - throw new SQLTimeoutException(e.getMessage(), e.getSQLState(), e.getErrorCode(), e.getCause()); - } else { - throw e; } - } finally { - if (newStmtCmd.wasExecuted()) - lastStmtExecCmd = newStmtCmd; - } + } while (cont); } /** @@ -793,6 +838,7 @@ public boolean execute(String sql) throws SQLServerException, SQLTimeoutExceptio loggerExternal.finer(toString() + ACTIVITY_ID + ActivityCorrelator.getCurrent().toString()); } checkClosed(); + ConfigurableRetryLogic.getInstance().storeLastQuery(sql); executeStatement(new StmtExecCmd(this, sql, EXECUTE, NO_GENERATED_KEYS)); loggerExternal.exiting(getClassNameLogging(), "execute", null != resultSet); return null != resultSet; diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/AlwaysEncrypted/JDBCEncryptionDecryptionTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/AlwaysEncrypted/JDBCEncryptionDecryptionTest.java index 7376baca7..6f3502933 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/AlwaysEncrypted/JDBCEncryptionDecryptionTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/AlwaysEncrypted/JDBCEncryptionDecryptionTest.java @@ -313,7 +313,7 @@ public void testAkvDecryptColumnEncryptionKey(String serverName, String url, Str akv.decryptColumnEncryptionKey("http:///^[!#$&-;=?-[]_a-", "", null); fail(TestResource.getResource("R_expectedExceptionNotThrown")); } catch (SQLServerException e) { - assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_AKVURLInvalid")), e.getMessage()); + assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_URLInvalid")), e.getMessage()); } // null encryptedColumnEncryptionKey diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java index 1ed9749cc..3b7fe1dc7 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java @@ -5,9 +5,9 @@ package com.microsoft.sqlserver.jdbc; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; -import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.IOException; import java.io.Reader; @@ -209,6 +209,8 @@ public void testDataSource() throws SQLServerException { ds.setCalcBigDecimalPrecision(booleanPropValue); assertEquals(booleanPropValue, ds.getCalcBigDecimalPrecision(), TestResource.getResource("R_valuesAreDifferent")); + ds.setRetryExec(stringPropValue); + assertEquals(stringPropValue, ds.getRetryExec(), TestResource.getResource("R_valuesAreDifferent")); ds.setServerCertificate(stringPropValue); assertEquals(stringPropValue, ds.getServerCertificate(), TestResource.getResource("R_valuesAreDifferent")); diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/configurableretry/ConfigurableRetryLogicTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/configurableretry/ConfigurableRetryLogicTest.java new file mode 100644 index 000000000..169986a0a --- /dev/null +++ b/src/test/java/com/microsoft/sqlserver/jdbc/configurableretry/ConfigurableRetryLogicTest.java @@ -0,0 +1,472 @@ +/* + * Microsoft JDBC Driver for SQL Server Copyright(c) Microsoft Corporation All rights reserved. This program is made + * available under the terms of the MIT License. See the LICENSE file in the project root for more information. + */ + +package com.microsoft.sqlserver.jdbc.configurableretry; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +import java.io.File; +import java.io.FileWriter; +import java.sql.CallableStatement; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import com.microsoft.sqlserver.jdbc.RandomUtil; +import com.microsoft.sqlserver.jdbc.SQLServerConnection; +import com.microsoft.sqlserver.jdbc.SQLServerException; +import com.microsoft.sqlserver.jdbc.TestResource; +import com.microsoft.sqlserver.jdbc.TestUtils; +import com.microsoft.sqlserver.testframework.AbstractSQLGenerator; +import com.microsoft.sqlserver.testframework.AbstractTest; + + +/** + * Test statement retry for configurable retry logic. + */ +public class ConfigurableRetryLogicTest extends AbstractTest { + /** + * The table used throughout the tests. + */ + private static final String CRLTestTable = AbstractSQLGenerator + .escapeIdentifier(RandomUtil.getIdentifier("crlTestTable")); + + /** + * Sets up tests. + * + * @throws Exception + * if an exception occurs + */ + @BeforeAll + public static void setupTests() throws Exception { + setConnection(); + } + + /** + * Test that the SQLServerConnection methods getRetryExec and setRetryExec correctly get the existing retryExec, and + * set the retryExec connection parameter respectively. + * + * @throws Exception + * if an exception occurs + */ + @Test + public void testRetryExecConnectionStringOption() throws Exception { + try (SQLServerConnection conn = (SQLServerConnection) DriverManager.getConnection(connectionString); + Statement s = conn.createStatement()) { + String test = conn.getRetryExec(); + assertTrue(test.isEmpty()); + conn.setRetryExec("{2714:3,2*2:CREATE;2715:1,3}"); + try { + PreparedStatement ps = conn.prepareStatement("create table " + CRLTestTable + " (c1 int null);"); + createTable(s); + ps.execute(); + Assertions.fail(TestResource.getResource("R_expectedFailPassed")); + } catch (SQLServerException e) { + assertTrue(e.getMessage().startsWith("There is already an object"), + TestResource.getResource("R_unexpectedExceptionContent") + ": " + e.getMessage()); + } finally { + dropTable(s); + } + } + } + + /** + * Tests that statement retry with prepared statements correctly retries given the provided retryExec rule. + * + * @throws Exception + * if unable to connect or execute against db + */ + @Test + public void testStatementRetryPreparedStatement() throws Exception { + try (Connection conn = DriverManager.getConnection( + TestUtils.addOrOverrideProperty(connectionString, "retryExec", "{2714:3,2*2:CREATE;2715:1,3}")); + Statement s = conn.createStatement(); + PreparedStatement ps = conn.prepareStatement("create table " + CRLTestTable + " (c1 int null);")) { + try { + createTable(s); + ps.execute(); + Assertions.fail(TestResource.getResource("R_expectedFailPassed")); + } catch (SQLServerException e) { + assertTrue(e.getMessage().startsWith("There is already an object"), + TestResource.getResource("R_unexpectedExceptionContent") + ": " + e.getMessage()); + } finally { + dropTable(s); + } + } + } + + /** + * Tests that statement retry with callable statements correctly retries given the provided retryExec rule. + * + * @throws Exception + * if unable to connect or execute against db + */ + @Test + public void testStatementRetryCallableStatement() throws Exception { + try (Connection conn = DriverManager.getConnection( + TestUtils.addOrOverrideProperty(connectionString, "retryExec", "{2714:3,2*2:CREATE;2715:1,3}")); + Statement s = conn.createStatement(); + CallableStatement cs = conn.prepareCall("create table " + CRLTestTable + " (c1 int null);")) { + try { + createTable(s); + cs.execute(); + fail(TestResource.getResource("R_expectedFailPassed")); + } catch (SQLServerException e) { + assertTrue(e.getMessage().startsWith("There is already an object"), + TestResource.getResource("R_unexpectedExceptionContent") + ": " + e.getMessage()); + } finally { + dropTable(s); + } + } + } + + /** + * Tests that statement retry with SQL server statements correctly retries given the provided retryExec rule. + * + * @throws Exception + * if unable to connect or execute against db + */ + public void testStatementRetry(String addedRetryParams) throws Exception { + try (Connection conn = DriverManager.getConnection(connectionString + addedRetryParams); + Statement s = conn.createStatement()) { + try { + createTable(s); + s.execute("create table " + CRLTestTable + " (c1 int null);"); + fail(TestResource.getResource("R_expectedFailPassed")); + } catch (SQLServerException e) { + assertTrue(e.getMessage().startsWith("There is already an object"), + TestResource.getResource("R_unexpectedExceptionContent") + ": " + e.getMessage()); + } finally { + dropTable(s); + } + } + } + + /** + * Tests that statement retry with SQL server statements correctly attempts to retry, but eventually cancels due + * to the retry wait interval being longer than queryTimeout. + * + * @throws Exception + * if unable to connect or execute against db + */ + public void testStatementRetryWithShortQueryTimeout(String addedRetryParams) throws Exception { + try (Connection conn = DriverManager.getConnection(connectionString + addedRetryParams); + Statement s = conn.createStatement()) { + try { + createTable(s); + s.execute("create table " + CRLTestTable + " (c1 int null);"); + fail(TestResource.getResource("R_expectedFailPassed")); + } finally { + dropTable(s); + } + } + } + + /** + * Tests that the correct number of retries are happening for all statement scenarios. Tests are expected to take + * a minimum of the sum of whatever has been defined for the waiting intervals, and maximum of the previous sum + * plus some amount of time to account for test environment slowness. + */ + @Test + public void statementTimingTests() { + long totalTime; + long timerStart = System.currentTimeMillis(); + + // A single retry immediately + try { + testStatementRetry("retryExec={2714:1;};"); + } catch (Exception e) { + Assertions.fail(TestResource.getResource("R_unexpectedException")); + } finally { + totalTime = System.currentTimeMillis() - timerStart; + assertTrue(totalTime < TimeUnit.SECONDS.toMillis(10), + "total time: " + totalTime + ", expected time: " + TimeUnit.SECONDS.toMillis(10)); + } + + timerStart = System.currentTimeMillis(); + + // A single retry waiting 5 seconds + try { + testStatementRetry("retryExec={2714:1,5;};"); + } catch (Exception e) { + Assertions.fail(TestResource.getResource("R_unexpectedException")); + } finally { + totalTime = System.currentTimeMillis() - timerStart; + assertTrue(totalTime > TimeUnit.SECONDS.toMillis(5), + "total time: " + totalTime + ", expected minimum time: " + TimeUnit.SECONDS.toMillis(5)); + assertTrue(totalTime < TimeUnit.SECONDS.toMillis(15), + "total time: " + totalTime + ", expected maximum time: " + TimeUnit.SECONDS.toMillis(15)); + } + + timerStart = System.currentTimeMillis(); + + // Two retries. The first after 2 seconds, the next after 6 + try { + testStatementRetry("retryExec={2714,2716:2,2*3:CREATE};"); + } catch (Exception e) { + Assertions.fail(TestResource.getResource("R_unexpectedException")); + } finally { + totalTime = System.currentTimeMillis() - timerStart; + assertTrue(totalTime > TimeUnit.SECONDS.toMillis(2), + "total time: " + totalTime + ", expected minimum time: " + TimeUnit.SECONDS.toMillis(8)); + assertTrue(totalTime < TimeUnit.SECONDS.toMillis(18), + "total time: " + totalTime + ", expected maximum time: " + TimeUnit.SECONDS.toMillis(18)); + } + + timerStart = System.currentTimeMillis(); + + // Two retries. The first after 3 seconds, the next after 7 + try { + testStatementRetry("retryExec={2714,2716:2,3+4:CREATE};"); + } catch (Exception e) { + Assertions.fail(TestResource.getResource("R_unexpectedException")); + } finally { + totalTime = System.currentTimeMillis() - timerStart; + assertTrue(totalTime > TimeUnit.SECONDS.toMillis(3), + "total time: " + totalTime + ", expected minimum time: " + TimeUnit.SECONDS.toMillis(10)); + assertTrue(totalTime < TimeUnit.SECONDS.toMillis(20), + "total time: " + totalTime + ", expected maximum time: " + TimeUnit.SECONDS.toMillis(20)); + } + } + + /** + * Tests that configurable retry logic correctly parses, and retries using, multiple rules provided at once. + */ + @Test + public void multipleRules() { + try { + testStatementRetry("retryExec={2716:1,2*2:CREATE;2714:1,2*2:CREATE};"); + } catch (Exception e) { + Assertions.fail(TestResource.getResource("R_unexpectedException")); + } + } + + /** + * Tests that CRL is able to read from a properties file, in the event the connection property is not used. + */ + @Test + public void readFromFile() { + File propsFile = null; + try { + propsFile = new File("mssql-jdbc.properties"); + FileWriter propFileWriter = new FileWriter(propsFile); + propFileWriter.write("retryExec={2716:1,2*2:CREATE;2714:1,2*2:CREATE};"); + propFileWriter.close(); + testStatementRetry(""); + + } catch (Exception e) { + Assertions.fail(TestResource.getResource("R_unexpectedException")); + } finally { + if (propsFile != null && !propsFile.delete()) { // If unable to delete, fail test + Assertions.fail(TestResource.getResource("R_unexpectedException")); + } + } + } + + /** + * Ensure that CRL properly re-reads rules after INTERVAL_BETWEEN_READS_IN_MS (30 secs). + */ + @Test + public void rereadAfterInterval() { + try { + testStatementRetry("retryExec={2716:1,2*2:CREATE;};"); + Thread.sleep(30000); // Sleep to ensure it has been INTERVAL_BETWEEN_READS_IN_MS between reads + testStatementRetry("retryExec={2714:1,2*2:CREATE;};"); + } catch (Exception e) { + Assertions.fail(TestResource.getResource("R_unexpectedException")); + } + } + + /** + * Tests that rules of the correct length, and containing valid values, pass. + */ + @Test + public void testCorrectlyFormattedRules() { + // Correctly formatted rules + try { + // Empty rule set + testStatementRetry("retryExec={};"); + testStatementRetry("retryExec={;};"); + + // Test length 1 + testStatementRetry("retryExec={2714:1;};"); + + // Test length 2 + testStatementRetry("retryExec={2714:1,3;};"); + + // Test length 2, with operand, but no initial-retry-time + testStatementRetry("retryExec={2714:1,3+;};"); + testStatementRetry("retryExec={2714:1,3*;};"); + + // Test length 3, but query is empty + testStatementRetry("retryExec={2714:1,3:;};"); + + // Test length 3, also multiple statement errors + testStatementRetry("retryExec={2714,2716:1,2*2:CREATE};"); + + // Same as above but using + operator + testStatementRetry("retryExec={2714,2716:1,2+2:CREATE};"); + testStatementRetry("retryExec={2714,2716:1,2+2};"); + + } catch (Exception e) { + Assertions.fail(TestResource.getResource("R_unexpectedException")); + } + + // Test length >3 + try { + testStatementRetry("retryExec={2714,2716:1,2*2:CREATE:4};"); + } catch (SQLServerException e) { + assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_InvalidRuleFormat"))); + } catch (Exception e) { + Assertions.fail(TestResource.getResource("R_unexpectedException")); + } + } + + /** + * Tests that too many timing parameters (>2) causes InvalidParameterFormat Exception. + */ + @Test + public void testTooManyTimings() { + try { + testStatementRetry("retryExec={2714,2716:1,2*2,1:CREATE};"); + } catch (SQLServerException e) { + assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_invalidParameterNumber"))); + } catch (Exception e) { + Assertions.fail(TestResource.getResource("R_unexpectedException")); + } + } + + /** + * Tests that rules with an invalid retry error correctly fail. + * + * @throws Exception + * for the invalid parameter + */ + @Test + public void testRetryError() throws Exception { + // Test incorrect format (NaN) + try { + testStatementRetry("retryExec={TEST:TEST};"); + } catch (SQLServerException e) { + assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_invalidParameterNumber"))); + } + + // Test empty error + try { + testStatementRetry("retryExec={:1,2*2:CREATE};"); + } catch (SQLServerException e) { + assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_invalidParameterNumber"))); + } + } + + /** + * Tests that rules with an invalid retry count correctly fail. + * + * @throws Exception + * for the invalid parameter + */ + @Test + public void testRetryCount() throws Exception { + // Test min + try { + testStatementRetry("retryExec={2714,2716:-1,2+2:CREATE};"); + } catch (SQLServerException e) { + assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_invalidParameterNumber"))); + } + + // Test max (query timeout) + try { + testStatementRetryWithShortQueryTimeout("queryTimeout=3;retryExec={2714,2716:11,2+2:CREATE};"); + } catch (SQLServerException e) { + assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_InvalidRetryInterval"))); + } + } + + /** + * Tests that rules with an invalid initial retry time correctly fail. + * + * @throws Exception + * for the invalid parameter + */ + @Test + public void testInitialRetryTime() throws Exception { + // Test min + try { + testStatementRetry("retryExec={2714,2716:4,-1+1:CREATE};"); + } catch (SQLServerException e) { + assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_invalidParameterNumber"))); + } + + // Test max + try { + testStatementRetryWithShortQueryTimeout("queryTimeout=3;retryExec={2714,2716:4,100+1:CREATE};"); + } catch (SQLServerException e) { + assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_InvalidRetryInterval"))); + } + } + + /** + * Tests that rules with an invalid operand correctly fail. + * + * @throws Exception + * for the invalid parameter + */ + @Test + public void testOperand() throws Exception { + try { + testStatementRetry("retryExec={2714,2716:1,2AND2:CREATE};"); + } catch (SQLServerException e) { + assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_invalidParameterNumber"))); + } + } + + /** + * Tests that rules with an invalid retry change correctly fail. + * + * @throws Exception + * for the invalid parameter + */ + @Test + public void testRetryChange() throws Exception { + try { + testStatementRetry("retryExec={2714,2716:1,2+2:CREATE};"); + } catch (SQLServerException e) { + assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_invalidParameterNumber"))); + } + } + + /** + * Creates table for use in ConfigurableRetryLogic tests. + * + * @param stmt + * the SQL statement to use to create the table + * @throws SQLException + * if unable to execute statement + */ + private static void createTable(Statement stmt) throws SQLException { + String sql = "create table " + CRLTestTable + " (c1 int null);"; + stmt.execute(sql); + } + + /** + * Drops the table used in ConfigurableRetryLogic tests. + * + * @param stmt + * the SQL statement to use to drop the table + * @throws SQLException + * if unable to execute statement + */ + private static void dropTable(Statement stmt) throws SQLException { + TestUtils.dropTableIfExists(CRLTestTable, stmt); + } +} diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/connection/RequestBoundaryMethodsTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/connection/RequestBoundaryMethodsTest.java index 0ae93feb3..8d3a62788 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/connection/RequestBoundaryMethodsTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/connection/RequestBoundaryMethodsTest.java @@ -521,6 +521,8 @@ private List getVerifiedMethodNames() { verifiedMethodNames.add("setCalcBigDecimalPrecision"); verifiedMethodNames.add("registerBeforeReconnectListener"); verifiedMethodNames.add("removeBeforeReconnectListener"); + verifiedMethodNames.add("getRetryExec"); + verifiedMethodNames.add("setRetryExec"); verifiedMethodNames.add("getUseFlexibleCallableStatements"); verifiedMethodNames.add("setUseFlexibleCallableStatements"); return verifiedMethodNames; From e03b19684dc6c3e0066ea5b1a8d2a5e0808e63e5 Mon Sep 17 00:00:00 2001 From: lilgreenbird Date: Mon, 23 Sep 2024 14:49:17 -0700 Subject: [PATCH 02/14] jdk 23 (#2515) --- README.md | 4 ++-- build.gradle | 8 ++++---- pom.xml | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 687962432..7423adff1 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ To build the jar files, you must use minimum version of Java 11 with Maven. You * Maven: 1. If you have not already done so, add the environment variable `mssql_jdbc_test_connection_properties` in your system with the connection properties for your SQL Server or SQL DB instance. 2. Run one of the commands below to build a JRE 11 and newer versions compatible jar or JRE 8 compatible jar in the `\target` directory. - * Run `mvn install -Pjre22`. This creates JRE 22 compatible jar in `\target` directory which is JDBC 4.3 compliant (Build with JDK 22). + * Run `mvn install -Pjre23`. This creates JRE 23 compatible jar in `\target` directory which is JDBC 4.3 compliant (Build with JDK 23). * Run `mvn install -Pjre21`. This creates JRE 21 compatible jar in `\target` directory which is JDBC 4.3 compliant (Build with JDK 21+). * Run `mvn install -Pjre17`. This creates JRE 17 compatible jar in `\target` directory which is JDBC 4.3 compliant (Build with JDK 17+). * Run `mvn install -Pjre11`. This creates JRE 11 compatible jar in `\target` directory which is JDBC 4.3 compliant (Build with JDK 11+). @@ -55,7 +55,7 @@ To build the jar files, you must use minimum version of Java 11 with Maven. You * Gradle: 1. If you have not already done so, add the environment variable `mssql_jdbc_test_connection_properties` in your system with the connection properties for your SQL Server or SQL DB instance. 2. Run one of the commands below to build a JRE 11 and newer versions compatible jar or JRE 8 compatible jar in the `\build\libs` directory. - * Run `gradle build -PbuildProfile=jre22`. This creates JRE 22 compatible jar in `\build\libs` directory which is JDBC 4.3 compliant (Build with JDK 22). + * Run `gradle build -PbuildProfile=jre23`. This creates JRE 23 compatible jar in `\build\libs` directory which is JDBC 4.3 compliant (Build with JDK 23). * Run `gradle build -PbuildProfile=jre21`. This creates JRE 21 compatible jar in `\build\libs` directory which is JDBC 4.3 compliant (Build with JDK 21+). * Run `gradle build -PbuildProfile=jre17`. This creates JRE 17 compatible jar in `\build\libs` directory which is JDBC 4.3 compliant (Build with JDK 17+). * Run `gradle build -PbuildProfile=jre11`. This creates JRE 11 compatible jar in `\build\libs` directory which is JDBC 4.3 compliant (Build with JDK 11+). diff --git a/build.gradle b/build.gradle index 5ee38e905..3e5d87a9e 100644 --- a/build.gradle +++ b/build.gradle @@ -33,17 +33,17 @@ test { } } -if (!hasProperty('buildProfile') || (hasProperty('buildProfile') && buildProfile == "jre22")) { +if (!hasProperty('buildProfile') || (hasProperty('buildProfile') && buildProfile == "jre23")) { - jreVersion = "jre22" + jreVersion = "jre23" excludedFile = 'com/microsoft/sqlserver/jdbc/SQLServerJdbc42.java' jar { manifest { attributes 'Automatic-Module-Name': 'com.microsoft.sqlserver.jdbc' } } - sourceCompatibility = 22 - targetCompatibility = 22 + sourceCompatibility = 23 + targetCompatibility = 23 } if (hasProperty('buildProfile') && buildProfile == "jre21") { diff --git a/pom.xml b/pom.xml index 32fe5f812..026144f46 100644 --- a/pom.xml +++ b/pom.xml @@ -403,12 +403,12 @@ - jre22 + jre23 true - ${project.artifactId}-${project.version}.jre22${releaseExt} + ${project.artifactId}-${project.version}.jre23${releaseExt} org.apache.maven.plugins @@ -418,8 +418,8 @@ **/com/microsoft/sqlserver/jdbc/SQLServerJdbc42.java - 22 - 22 + 23 + 23 From ffa5e1fa9cc1a490ed10d86b39a7c8b887488d89 Mon Sep 17 00:00:00 2001 From: Jeff Wasty Date: Wed, 25 Sep 2024 10:27:45 -0700 Subject: [PATCH 03/14] Update changelog (#2520) --- CHANGELOG.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c007584a..9f5d2883a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,19 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) +## [12.9.0] Preview Release +### Added +- Added statement portion of configurable retry logic feature [#2396](https://github.com/microsoft/mssql-jdbc/pull/2396) +- Added JDK 23 support [#2515](https://github.com/microsoft/mssql-jdbc/pull/2515) + +### Changed +- Reverted "Execute Stored Procedures Directly" feature, as well as subsequent changes related to the feature [#2488](https://github.com/microsoft/mssql-jdbc/pull/2488) +- Changed MSAL logging from FINER to FINEST [#2489](https://github.com/microsoft/mssql-jdbc/pull/2489) + +### Fixed issues +- Changed driver behavior to allow prepared statement objects to be reused, preventing a "multiple queries are not allowed" error [#2482](https://github.com/microsoft/mssql-jdbc/pull/2482) +- Adjusted DESTINATION_COL_METADATA_LOCK, in SQLServerBulkCopy, so that is properly released in all cases [#2484](https://github.com/microsoft/mssql-jdbc/pull/2484) + ## [12.8.0] Stable Release ### Fixed issues - Fixed regression with specifying argument names in callable statement syntax [#2480](https://github.com/microsoft/mssql-jdbc/pull/2480) From 362d3a43c52a30284247222719459f09a9f3f285 Mon Sep 17 00:00:00 2001 From: lilgreenbird Date: Thu, 7 Nov 2024 11:42:05 -0800 Subject: [PATCH 04/14] javadocs (#2521) --- .../sqlserver/jdbc/KerbAuthentication.java | 4 +-- .../PersistentTokenCacheAccessAspect.java | 7 ++++++ .../sqlserver/jdbc/ReconnectListener.java | 3 +++ .../sqlserver/jdbc/SQLServerConnection.java | 15 +++++++++++ .../sqlserver/jdbc/SQLServerException.java | 6 +++++ .../jdbc/SQLServerPreparedStatement.java | 25 ++++++++++++++----- .../sqlserver/jdbc/SQLServerSortOrder.java | 3 +++ .../sqlserver/jdbc/SQLServerWarning.java | 5 +++- 8 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/KerbAuthentication.java b/src/main/java/com/microsoft/sqlserver/jdbc/KerbAuthentication.java index 3c21e7711..6f8f30a29 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/KerbAuthentication.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/KerbAuthentication.java @@ -5,8 +5,6 @@ package com.microsoft.sqlserver.jdbc; -import java.security.AccessControlContext; -import java.security.AccessController; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; import java.text.MessageFormat; @@ -96,7 +94,7 @@ private void initAuthInit() throws SQLServerException { Subject currentSubject; KerbCallback callback = new KerbCallback(con); try { - AccessControlContext context = AccessController.getContext(); + java.security.AccessControlContext context = java.security.AccessController.getContext(); currentSubject = Subject.getSubject(context); if (null == currentSubject) { if (useDefaultJaas) { diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/PersistentTokenCacheAccessAspect.java b/src/main/java/com/microsoft/sqlserver/jdbc/PersistentTokenCacheAccessAspect.java index 24458a7f3..5dca14899 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/PersistentTokenCacheAccessAspect.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/PersistentTokenCacheAccessAspect.java @@ -28,6 +28,13 @@ public class PersistentTokenCacheAccessAspect implements ITokenCacheAccessAspect static final long TIME_TO_LIVE = 86400000L; // Token cache time to live (24 hrs). private long expiryTime; + /** + * default constructor + */ + public PersistentTokenCacheAccessAspect() { + // default constructor + } + static PersistentTokenCacheAccessAspect getInstance() { if (instance == null) { instance = new PersistentTokenCacheAccessAspect(); diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/ReconnectListener.java b/src/main/java/com/microsoft/sqlserver/jdbc/ReconnectListener.java index c56045dfd..5fc1dfff8 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/ReconnectListener.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/ReconnectListener.java @@ -10,6 +10,9 @@ @FunctionalInterface public interface ReconnectListener { + /** + * called before reconnect + */ void beforeReconnect(); } diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index 903adc0b0..601271755 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -1064,6 +1064,9 @@ public void setCalcBigDecimalPrecision(boolean calcBigDecimalPrecision) { this.calcBigDecimalPrecision = calcBigDecimalPrecision; } + /** + * Retry exec + */ private String retryExec = SQLServerDriverStringProperty.RETRY_EXEC.getDefaultValue(); /** @@ -1783,10 +1786,22 @@ SQLServerPooledConnection getPooledConnectionParent() { */ private List reconnectListeners = new ArrayList<>(); + /** + * Register before reconnect listener + * + * @param reconnectListener + * reconnect listener + */ public void registerBeforeReconnectListener(ReconnectListener reconnectListener) { reconnectListeners.add(reconnectListener); } + /** + * Remove before reconnect listener + * + * @param reconnectListener + * reconnect listener + */ public void removeBeforeReconnectListener(ReconnectListener reconnectListener) { reconnectListeners.remove(reconnectListener); } diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerException.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerException.java index 57387bb03..ba35b84a2 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerException.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerException.java @@ -179,6 +179,12 @@ static String getErrString(String errCode) { logException(obj, errText, bStack); } + /** + * Constructs a new SQLServerException from SQL Server error + * + * @param sqlServerError + * SQL Server error + */ public SQLServerException(SQLServerError sqlServerError) { super(sqlServerError.getErrorMessage(), generateStateCode(null, sqlServerError.getErrorNumber(), sqlServerError.getErrorState()), diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java index 2c2bbcb9a..2128f9cef 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java @@ -132,8 +132,20 @@ private void setPreparedStatementHandle(int handle) { * For caching data related to batch insert with bulkcopy */ private SQLServerBulkCopy bcOperation = null; + + /** + * Bulkcopy operation table name + */ private String bcOperationTableName = null; + + /** + * Bulkcopy operation column list + */ private ArrayList bcOperationColumnList = null; + + /** + * Bulkcopy operation value list + */ private ArrayList bcOperationValueList = null; /** Returns the prepared statement SQL */ @@ -450,13 +462,13 @@ private String buildParamTypeDefinitions(Parameter[] params, boolean renewDefini return ""; // Output looks like @P0 timestamp, @P1 varchar - int stringLen = nCols * 2; // @P - stringLen += nCols; // spaces - stringLen += nCols -1; // commas + int stringLen = nCols * 2; // @P + stringLen += nCols; // spaces + stringLen += nCols - 1; // commas if (nCols > 10) - stringLen += 10 + ((nCols - 10) * 2); // @P{0-99} Numbers after p + stringLen += 10 + ((nCols - 10) * 2); // @P{0-99} Numbers after p else - stringLen += nCols; // @P{0-9} Numbers after p less than 10 + stringLen += nCols; // @P{0-9} Numbers after p less than 10 // Computing the type definitions up front, so we can get exact string lengths needed for the string builder. String[] typeDefinitions = new String[nCols]; @@ -2358,7 +2370,8 @@ public long[] executeLargeBatch() throws SQLServerException, BatchUpdateExceptio if (rs.getColumnCount() != bcOperationValueList.size()) { MessageFormat form = new MessageFormat( SQLServerException.getErrString("R_colNotMatchTable")); - Object[] msgArgs = {bcOperationColumnList!= null ? bcOperationColumnList.size() : 0, bcOperationValueList.size()}; + Object[] msgArgs = {bcOperationColumnList != null ? bcOperationColumnList.size() : 0, + bcOperationValueList.size()}; throw new IllegalArgumentException(form.format(msgArgs)); } } diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerSortOrder.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerSortOrder.java index 16ea49043..756515674 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerSortOrder.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerSortOrder.java @@ -11,8 +11,11 @@ * */ public enum SQLServerSortOrder { + /** ascending order */ ASCENDING(0), + /** descending order */ DESCENDING(1), + /** unspecified order */ UNSPECIFIED(-1); final int value; diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerWarning.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerWarning.java index de6df2a42..dffefdcb8 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerWarning.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerWarning.java @@ -22,8 +22,11 @@ public class SQLServerWarning extends SQLWarning { /** SQL server error */ private SQLServerError sqlServerError; - /* + /** * Create a SQLWarning from an SQLServerError object + * + * @param sqlServerError + * SQL Server error */ public SQLServerWarning(SQLServerError sqlServerError) { super(sqlServerError.getErrorMessage(), SQLServerException.generateStateCode(null, From b27b0c1ed7f1db74347b2ad8f3d0ae68dcac7384 Mon Sep 17 00:00:00 2001 From: Jeff Wasty Date: Thu, 7 Nov 2024 14:43:07 -0800 Subject: [PATCH 05/14] Fix retries when `connectRetryCount` is set to 1 or greater (#2513) * Fix for connectRetryCount > 0 * Cleanup * Added test to SQLServerConnectionTest + cleanup * Fix test * Testing * ok.. * Grasping at straws * more debugs, yum * focus on transient error * Fix for azure * Cleanup * More cleanup * More cleanup * Trying to resolve thread interrupt test * a different try * add more time to interrupt * Removed sleep mistakenly * Added more variants * Remove unneeded tests * Remove problem line + add fix for uncaught fail for BatchExecutionTest * Fail fixes * Added back the timer to the BatchExecutionTest; adjusted timeout time * More code cleanup * Fixed mistake * Formatting * Trying to resolve code cov * Update timeout value * Remove stray fail * Revert changes to login retry interval. * Remove added line. --- .../sqlserver/jdbc/SQLServerConnection.java | 7 +++- .../jdbc/SQLServerConnectionTest.java | 42 +++++++++++++------ .../jdbc/connection/TimeoutTest.java | 28 ++++++++----- .../unit/statement/BatchExecutionTest.java | 3 +- 4 files changed, 53 insertions(+), 27 deletions(-) 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 From ef6b7709c09f24365e832f2bfddc941f1a8eea48 Mon Sep 17 00:00:00 2001 From: Terry Chow <32403408+tkyc@users.noreply.github.com> Date: Wed, 13 Nov 2024 09:34:13 -0800 Subject: [PATCH 06/14] Updated fedauth error tests (#2538) --- .../sqlserver/jdbc/fedauth/ErrorMessageTest.java | 9 ++++++--- .../microsoft/sqlserver/jdbc/fedauth/FedauthCommon.java | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/fedauth/ErrorMessageTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/fedauth/ErrorMessageTest.java index 936855573..dd4d95239 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/fedauth/ErrorMessageTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/fedauth/ErrorMessageTest.java @@ -397,7 +397,8 @@ public void testADPasswordWrongPasswordWithConnectionStringUserName() throws SQL && e.getCause().getCause().getMessage().toLowerCase().contains("invalid username or password") || e.getCause().getCause().getMessage().contains(ERR_MSG_SIGNIN_TOO_MANY) || e.getCause().getCause().getMessage().contains(ERR_FAULT_ID3342) - || e.getMessage().contains(ERR_MSG_REQUEST_THROTTLED)); + || e.getMessage().contains(ERR_MSG_REQUEST_THROTTLED) + || e.getMessage().contains(ERR_FAULT_AUTH_FAIL)); } } @@ -424,7 +425,8 @@ public void testADPasswordWrongPasswordWithDatasource() throws SQLException { && e.getCause().getCause().getMessage().toLowerCase().contains("invalid username or password") || e.getCause().getCause().getMessage().contains(ERR_MSG_SIGNIN_TOO_MANY) || e.getCause().getCause().getMessage().contains(ERR_FAULT_ID3342) - || e.getMessage().contains(ERR_MSG_REQUEST_THROTTLED)); + || e.getMessage().contains(ERR_MSG_REQUEST_THROTTLED) + || e.getMessage().contains(ERR_FAULT_AUTH_FAIL)); } } @@ -445,7 +447,8 @@ public void testADPasswordWrongPasswordWithConnectionStringUser() throws SQLExce && e.getCause().getCause().getMessage().toLowerCase().contains("invalid username or password") || e.getCause().getCause().getMessage().contains(ERR_MSG_SIGNIN_TOO_MANY) || e.getCause().getCause().getMessage().contains(ERR_FAULT_ID3342) - || e.getMessage().contains(ERR_MSG_REQUEST_THROTTLED)); + || e.getMessage().contains(ERR_MSG_REQUEST_THROTTLED) + || e.getMessage().contains(ERR_FAULT_AUTH_FAIL)); } } diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/fedauth/FedauthCommon.java b/src/test/java/com/microsoft/sqlserver/jdbc/fedauth/FedauthCommon.java index bbacf1ecc..1270b1c94 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/fedauth/FedauthCommon.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/fedauth/FedauthCommon.java @@ -128,6 +128,7 @@ public class FedauthCommon extends AbstractTest { static final String ERR_MSG_HAS_BEEN_CLOSED = TestResource.getResource("R_hasBeenClosed"); static final String ERR_MSG_SIGNIN_TOO_MANY = TestResource.getResource("R_signinTooManyTimes"); static final String ERR_FAULT_ID3342 = "FaultMessage: ID3242"; + static final String ERR_FAULT_AUTH_FAIL = "FaultMessage: Authentication Failure"; static final String ERR_MSG_NOT_AUTH_AND_IS = TestUtils.R_BUNDLE .getString("R_SetAuthenticationWhenIntegratedSecurityTrue"); static final String ERR_MSG_NOT_AUTH_AND_USER_PASSWORD = TestUtils.R_BUNDLE From 38564d9ef6d5833f598547df0122876608d68b6e Mon Sep 17 00:00:00 2001 From: Jeff Wasty Date: Wed, 20 Nov 2024 13:09:46 -0800 Subject: [PATCH 07/14] Configurable Retry Logic II - Connection Retry (#2519) * Added back changes for connection part of CRL, which only affect exis 0296179 ting files. To allow for PR to be created. * added more * Added tests * Add another test; see if I can still push to GitHub * Added more * Decoupled statement and connection parts to ensure tests pass. * Cleanup. * Changed tests to account for new changes to retry logic. --- .../jdbc/ConfigurableRetryLogic.java | 119 ++++++++++++--- .../sqlserver/jdbc/ConfigurableRetryRule.java | 21 ++- .../sqlserver/jdbc/ISQLServerDataSource.java | 16 +++ .../sqlserver/jdbc/SQLServerConnection.java | 50 ++++++- .../sqlserver/jdbc/SQLServerDataSource.java | 32 +++++ .../sqlserver/jdbc/SQLServerDriver.java | 5 +- .../sqlserver/jdbc/SQLServerResource.java | 3 +- .../sqlserver/jdbc/SQLServerStatement.java | 2 +- .../jdbc/SQLServerConnectionTest.java | 3 + .../ConfigurableRetryLogicTest.java | 136 +++++++++++++++++- .../RequestBoundaryMethodsTest.java | 2 + 11 files changed, 361 insertions(+), 28 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryLogic.java b/src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryLogic.java index ed5591034..df9e6b956 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryLogic.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryLogic.java @@ -39,6 +39,9 @@ public class ConfigurableRetryLogic { private static final String FORWARD_SLASH = "/"; private static final String EQUALS_SIGN = "="; private static final String RETRY_EXEC = "retryExec"; + private static final String RETRY_CONN = "retryConn"; + private static final String STATEMENT = "statement"; + private static boolean replaceFlag = false; // Are we replacing the list of transient errors? /** * The time the properties file was last modified. */ @@ -52,14 +55,23 @@ public class ConfigurableRetryLogic { */ private static final AtomicReference lastQuery = new AtomicReference<>(""); /** - * The previously read rules from the connection string. + * The previously read statement rules from the connection string. */ - private static final AtomicReference prevRulesFromConnectionString = new AtomicReference<>(""); + private static final AtomicReference prevStmtRulesFromConnString = new AtomicReference<>(""); + /** + * The previously read connection rules from the connection string. + */ + private static final AtomicReference prevConnRulesFromConnString = new AtomicReference<>(""); /** * The list of statement retry rules. */ private static final AtomicReference> stmtRules = new AtomicReference<>( new HashMap<>()); + /** + * The list of connection retry rules. + */ + private static final AtomicReference> connRules = new AtomicReference<>( + new HashMap<>()); private static ConfigurableRetryLogic singleInstance; /** @@ -70,7 +82,8 @@ public class ConfigurableRetryLogic { */ private ConfigurableRetryLogic() throws SQLServerException { timeLastRead.compareAndSet(0, new Date().getTime()); - setUpRules(null); + setUpStatementRules(null); + setUpConnectionRules(null); } /** @@ -102,7 +115,8 @@ public static ConfigurableRetryLogic getInstance() throws SQLServerException { /** * If it has been INTERVAL_BETWEEN_READS_IN_MS (30 secs) since last read, see if we last did a file read, if so - * only reread if the file has been modified. If no file read, set up rules using the prev. connection string rules. + * only reread if the file has been modified. If no file read, set up rules using the previous connection + * string (statement and connection) rules * * @throws SQLServerException * when an exception occurs @@ -116,25 +130,40 @@ private static void refreshRuleSet() throws SQLServerException { // If timeLastModified is set, we previously read from file, so we setUpRules also reading from file File f = new File(getCurrentClassPath()); if (f.lastModified() != timeLastModified.get()) { - setUpRules(null); + setUpStatementRules(null); + setUpConnectionRules(null); } } else { - setUpRules(prevRulesFromConnectionString.get()); + setUpStatementRules(prevStmtRulesFromConnString.get()); + setUpConnectionRules(prevConnRulesFromConnString.get()); } } } /** - * Sets rules given from connection string. + * Sets statement rules given from connection string. + * + * @param newRules + * the new rules to use + * @throws SQLServerException + * when an exception occurs + */ + void setStatementRulesFromConnectionString(String newRules) throws SQLServerException { + prevStmtRulesFromConnString.set(newRules); + setUpStatementRules(prevStmtRulesFromConnString.get()); + } + + /** + * Sets connection rules given from connection string. * * @param newRules * the new rules to use * @throws SQLServerException * when an exception occurs */ - void setFromConnectionString(String newRules) throws SQLServerException { - prevRulesFromConnectionString.set(newRules); - setUpRules(prevRulesFromConnectionString.get()); + void setConnectionRulesFromConnectionString(String newRules) throws SQLServerException { + prevConnRulesFromConnString.set(newRules); + setUpConnectionRules(prevConnRulesFromConnString.get()); } /** @@ -164,19 +193,34 @@ String getLastQuery() { * @throws SQLServerException * if an exception occurs */ - private static void setUpRules(String cxnStrRules) throws SQLServerException { + private static void setUpStatementRules(String cxnStrRules) throws SQLServerException { LinkedList temp; stmtRules.set(new HashMap<>()); lastQuery.set(""); if (cxnStrRules == null || cxnStrRules.isEmpty()) { - temp = readFromFile(); + temp = readFromFile(RETRY_EXEC); } else { temp = new LinkedList<>(); Collections.addAll(temp, cxnStrRules.split(SEMI_COLON)); } - createRules(temp); + createStatementRules(temp); + } + + private static void setUpConnectionRules(String cxnStrRules) throws SQLServerException { + LinkedList temp; + + connRules.set(new HashMap<>()); + lastQuery.set(""); + + if (cxnStrRules == null || cxnStrRules.isEmpty()) { + temp = readFromFile(RETRY_CONN); + } else { + temp = new LinkedList<>(); + Collections.addAll(temp, cxnStrRules.split(SEMI_COLON)); + } + createConnectionRules(temp); } /** @@ -187,7 +231,7 @@ private static void setUpRules(String cxnStrRules) throws SQLServerException { * @throws SQLServerException * if unable to create rules from the inputted list */ - private static void createRules(LinkedList listOfRules) throws SQLServerException { + private static void createStatementRules(LinkedList listOfRules) throws SQLServerException { stmtRules.set(new HashMap<>()); for (String potentialRule : listOfRules) { @@ -206,6 +250,29 @@ private static void createRules(LinkedList listOfRules) throws SQLServer } } + private static void createConnectionRules(LinkedList listOfRules) throws SQLServerException { + connRules.set(new HashMap<>()); + replaceFlag = false; + + for (String potentialRule : listOfRules) { + ConfigurableRetryRule rule = new ConfigurableRetryRule(potentialRule); + if (rule.replaceExisting) { + replaceFlag = true; + } + + if (rule.getError().contains(COMMA)) { + String[] arr = rule.getError().split(COMMA); + + for (String retryError : arr) { + ConfigurableRetryRule splitRule = new ConfigurableRetryRule(retryError, rule); + connRules.get().put(Integer.parseInt(splitRule.getError()), splitRule); + } + } else { + connRules.get().put(Integer.parseInt(rule.getError()), rule); + } + } + } + /** * Gets the current class path (for use in file reading). * @@ -241,7 +308,7 @@ private static String getCurrentClassPath() throws SQLServerException { * @throws SQLServerException * if unable to read from the file */ - private static LinkedList readFromFile() throws SQLServerException { + private static LinkedList readFromFile(String connectionStringProperty) throws SQLServerException { String filePath = getCurrentClassPath(); LinkedList list = new LinkedList<>(); @@ -250,7 +317,7 @@ private static LinkedList readFromFile() throws SQLServerException { try (BufferedReader buffer = new BufferedReader(new FileReader(f))) { String readLine; while ((readLine = buffer.readLine()) != null) { - if (readLine.startsWith(RETRY_EXEC)) { + if (readLine.startsWith(connectionStringProperty)) { // Either "retryExec" or "retryConn" String value = readLine.split(EQUALS_SIGN)[1]; Collections.addAll(list, value.split(SEMI_COLON)); } @@ -280,13 +347,25 @@ private static LinkedList readFromFile() throws SQLServerException { * @throws SQLServerException * when an exception occurs */ - ConfigurableRetryRule searchRuleSet(int ruleToSearchFor) throws SQLServerException { + ConfigurableRetryRule searchRuleSet(int ruleToSearchFor, String ruleSet) throws SQLServerException { refreshRuleSet(); - for (Map.Entry entry : stmtRules.get().entrySet()) { - if (entry.getKey() == ruleToSearchFor) { - return entry.getValue(); + if (ruleSet.equals(STATEMENT)) { + for (Map.Entry entry : stmtRules.get().entrySet()) { + if (entry.getKey() == ruleToSearchFor) { + return entry.getValue(); + } + } + } else { + for (Map.Entry entry : connRules.get().entrySet()) { + if (entry.getKey() == ruleToSearchFor) { + return entry.getValue(); + } } } return null; } + + boolean getReplaceFlag() { + return replaceFlag; + } } diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryRule.java b/src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryRule.java index f52df8d8d..518a111be 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryRule.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/ConfigurableRetryRule.java @@ -25,6 +25,8 @@ class ConfigurableRetryRule { private int retryCount = 1; private String retryQueries = ""; private String retryError; + boolean isConnection = false; + boolean replaceExisting = false; private ArrayList waitTimes = new ArrayList<>(); @@ -70,6 +72,18 @@ private void copyFromRule(ConfigurableRetryRule baseRule) { this.retryCount = baseRule.retryCount; this.retryQueries = baseRule.retryQueries; this.waitTimes = baseRule.waitTimes; + this.isConnection = baseRule.isConnection; + } + + private String appendOrReplace(String retryError) { + if (retryError.startsWith(PLUS_SIGN)) { + replaceExisting = false; + StringUtils.isNumeric(retryError.substring(1)); + return retryError.substring(1); + } else { + replaceExisting = true; + return retryError; + } } /** @@ -152,7 +166,12 @@ private void checkParameter(String value) throws SQLServerException { * if a rule or parameter has invalid inputs */ private void addElements(String[] rule) throws SQLServerException { - if (rule.length == 2 || rule.length == 3) { + if (rule.length == 1) { + String errorWithoutOptionalPrefix = appendOrReplace(rule[0]); + checkParameter(errorWithoutOptionalPrefix); + isConnection = true; + retryError = errorWithoutOptionalPrefix; + } else if (rule.length == 2 || rule.length == 3) { checkParameter(rule[0]); retryError = rule[0]; String[] timings = rule[1].split(COMMA); diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerDataSource.java b/src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerDataSource.java index 66fc33744..ec7067220 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerDataSource.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerDataSource.java @@ -1363,6 +1363,22 @@ public interface ISQLServerDataSource extends javax.sql.CommonDataSource { */ String getRetryExec(); + /** + * Returns value of 'retryConn' from Connection String. + * + * @param retryConn + * Set of rules used for connection retry + */ + void setRetryConn(String retryConn); + + /** + * Sets the value for 'retryConn' property + * + * @return retryConn + * String value + */ + String getRetryConn(); + /** * useFlexibleCallableStatements is temporarily removed. This is meant as a no-op. * diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index 5d01955f2..1571f5bec 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -1089,6 +1089,28 @@ public void setRetryExec(String retryExec) { this.retryExec = retryExec; } + private String retryConn = SQLServerDriverStringProperty.RETRY_CONN.getDefaultValue(); + + /** + * Returns the set of configurable connection retry rules set in retryConn + * + * @return + * A string containing statement retry rules. + */ + public String getRetryConn() { + return retryConn; + } + + /** + * Sets the list of configurable connection retry rules, for the given connection, in retryConn. + * + * @param retryConn + * The list of retry rules to set, as a string. + */ + public void setRetryConn(String retryConn) { + this.retryConn = retryConn; + } + /** Session Recovery Object */ private transient IdleConnectionResiliency sessionRecovery = new IdleConnectionResiliency(this); @@ -2039,10 +2061,23 @@ Connection connect(Properties propsIn, SQLServerPooledConnection pooledConnectio } throw e; } else { - // only retry if transient error + // Only retry if matches configured CRL rules, or transient error (if CRL is not in use) SQLServerError sqlServerError = e.getSQLServerError(); - if (!TransientError.isTransientError(sqlServerError)) { + if (null == sqlServerError) { throw e; + } else { + ConfigurableRetryRule rule = ConfigurableRetryLogic.getInstance() + .searchRuleSet(sqlServerError.getErrorNumber(), "connection"); + + if (null == rule) { + if (ConfigurableRetryLogic.getInstance().getReplaceFlag()) { + throw e; + } else { + if (!TransientError.isTransientError(sqlServerError)) { + throw e; + } + } + } } // check if there's time to retry, no point to wait if no time left @@ -2385,7 +2420,16 @@ Connection connectInternal(Properties propsIn, activeConnectionProperties.setProperty(sPropKey, sPropValue); } retryExec = sPropValue; - ConfigurableRetryLogic.getInstance().setFromConnectionString(sPropValue); + ConfigurableRetryLogic.getInstance().setStatementRulesFromConnectionString(sPropValue); + + sPropKey = SQLServerDriverStringProperty.RETRY_CONN.toString(); + sPropValue = activeConnectionProperties.getProperty(sPropKey); + if (null == sPropValue) { + sPropValue = SQLServerDriverStringProperty.RETRY_CONN.getDefaultValue(); + activeConnectionProperties.setProperty(sPropKey, sPropValue); + } + retryConn = sPropValue; + ConfigurableRetryLogic.getInstance().setConnectionRulesFromConnectionString(sPropValue); sPropKey = SQLServerDriverBooleanProperty.CALC_BIG_DECIMAL_PRECISION.toString(); sPropValue = activeConnectionProperties.getProperty(sPropKey); diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java index 6136cffa0..da7688e60 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java @@ -1383,16 +1383,48 @@ public boolean getCalcBigDecimalPrecision() { SQLServerDriverBooleanProperty.CALC_BIG_DECIMAL_PRECISION.getDefaultValue()); } + /** + * Sets the 'retryExec' setting. + * + * @param retryExec + * String property giving the custom statement retry rules to use for configurable retry logic + */ @Override public void setRetryExec(String retryExec) { setStringProperty(connectionProps, SQLServerDriverStringProperty.RETRY_EXEC.toString(), retryExec); } + /** + * Returns the value for 'retryExec'. + * + * @return retryExec String value + */ @Override public String getRetryExec() { return getStringProperty(connectionProps, SQLServerDriverStringProperty.RETRY_EXEC.toString(), null); } + /** + * Sets the 'retryConn' setting. + * + * @param retryConn + * String property giving the custom connection retry rules to use for configurable retry logic + */ + @Override + public void setRetryConn(String retryConn) { + setStringProperty(connectionProps, SQLServerDriverStringProperty.RETRY_CONN.toString(), retryConn); + } + + /** + * Returns the value for 'retryConn'. + * + * @return retryConn String value + */ + @Override + public String getRetryConn() { + return getStringProperty(connectionProps, SQLServerDriverStringProperty.RETRY_CONN.toString(), null); + } + /** * Sets a property string value. * diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java index 1c9abc000..e4b1d59ee 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java @@ -611,7 +611,8 @@ enum SQLServerDriverStringProperty { SERVER_CERTIFICATE("serverCertificate", ""), DATETIME_DATATYPE("datetimeParameterType", DatetimeType.DATETIME2.toString()), ACCESS_TOKEN_CALLBACK_CLASS("accessTokenCallbackClass", ""), - RETRY_EXEC("retryExec", ""); + RETRY_EXEC("retryExec", ""), + RETRY_CONN("retryConn", ""); private final String name; private final String defaultValue; @@ -855,6 +856,8 @@ public final class SQLServerDriver implements java.sql.Driver { SQLServerDriverStringProperty.ACCESS_TOKEN_CALLBACK_CLASS.getDefaultValue(), false, null), new SQLServerDriverPropertyInfo(SQLServerDriverStringProperty.RETRY_EXEC.toString(), SQLServerDriverStringProperty.RETRY_EXEC.getDefaultValue(), false, null), + new SQLServerDriverPropertyInfo(SQLServerDriverStringProperty.RETRY_CONN.toString(), + SQLServerDriverStringProperty.RETRY_CONN.getDefaultValue(), false, null), new SQLServerDriverPropertyInfo(SQLServerDriverBooleanProperty.REPLICATION.toString(), Boolean.toString(SQLServerDriverBooleanProperty.REPLICATION.getDefaultValue()), false, TRUE_FALSE), new SQLServerDriverPropertyInfo(SQLServerDriverBooleanProperty.SEND_TIME_AS_DATETIME.toString(), diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java index 55a5a5fc0..c9d875e58 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java @@ -517,7 +517,8 @@ protected Object[][] getContents() { {"R_InvalidCSVQuotes", "Failed to parse the CSV file, verify that the fields are correctly enclosed in double quotes."}, {"R_TokenRequireUrl", "Token credentials require a URL using the HTTPS protocol scheme."}, {"R_calcBigDecimalPrecisionPropertyDescription", "Indicates whether the driver should calculate precision for big decimal values."}, - {"R_retryExecPropertyDescription", "List of rules to follow for configurable retry logic."}, + {"R_retryExecPropertyDescription", "List of statement retry rules to follow for configurable retry logic."}, + {"R_retryConnPropertyDescription", "List of connection retry rules to follow for configurable retry logic."}, {"R_maxResultBufferPropertyDescription", "Determines maximum amount of bytes that can be read during retrieval of result set"}, {"R_maxResultBufferInvalidSyntax", "Invalid syntax: {0} in maxResultBuffer parameter."}, {"R_maxResultBufferNegativeParameterValue", "MaxResultBuffer must have positive value: {0}."}, diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java index 883320640..fef51bc6a 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java @@ -256,7 +256,7 @@ final void executeStatement(TDSCommand newStmtCmd) throws SQLServerException, SQ ConfigurableRetryRule rule = null; if (null != sqlServerError) { - rule = crl.searchRuleSet(e.getSQLServerError().getErrorNumber()); + rule = crl.searchRuleSet(e.getSQLServerError().getErrorNumber(), "statement"); } // If there is a rule for this error AND we still have retries remaining THEN we can proceed, otherwise diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java index d4a2ebe6a..916aa419f 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/SQLServerConnectionTest.java @@ -212,6 +212,9 @@ public void testDataSource() throws SQLServerException { ds.setRetryExec(stringPropValue); assertEquals(stringPropValue, ds.getRetryExec(), TestResource.getResource("R_valuesAreDifferent")); + ds.setRetryConn(stringPropValue); + assertEquals(stringPropValue, ds.getRetryConn(), TestResource.getResource("R_valuesAreDifferent")); + ds.setServerCertificate(stringPropValue); assertEquals(stringPropValue, ds.getServerCertificate(), TestResource.getResource("R_valuesAreDifferent")); diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/configurableretry/ConfigurableRetryLogicTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/configurableretry/ConfigurableRetryLogicTest.java index 169986a0a..3f522080d 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/configurableretry/ConfigurableRetryLogicTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/configurableretry/ConfigurableRetryLogicTest.java @@ -5,6 +5,7 @@ package com.microsoft.sqlserver.jdbc.configurableretry; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -32,7 +33,7 @@ /** - * Test statement retry for configurable retry logic. + * Test connection and statement retry for configurable retry logic. */ public class ConfigurableRetryLogicTest extends AbstractTest { /** @@ -66,6 +67,31 @@ public void testRetryExecConnectionStringOption() throws Exception { String test = conn.getRetryExec(); assertTrue(test.isEmpty()); conn.setRetryExec("{2714:3,2*2:CREATE;2715:1,3}"); + test = conn.getRetryExec(); + assertFalse(test.isEmpty()); + try { + PreparedStatement ps = conn.prepareStatement("create table " + CRLTestTable + " (c1 int null);"); + createTable(s); + ps.execute(); + Assertions.fail(TestResource.getResource("R_expectedFailPassed")); + } catch (SQLServerException e) { + assertTrue(e.getMessage().startsWith("There is already an object"), + TestResource.getResource("R_unexpectedExceptionContent") + ": " + e.getMessage()); + } finally { + dropTable(s); + } + } + } + + @Test + public void testRetryConnConnectionStringOption() throws Exception { + try (SQLServerConnection conn = (SQLServerConnection) DriverManager.getConnection(connectionString); + Statement s = conn.createStatement()) { + String test = conn.getRetryConn(); + assertTrue(test.isEmpty()); + conn.setRetryConn("{4060}"); + test = conn.getRetryConn(); + assertFalse(test.isEmpty()); try { PreparedStatement ps = conn.prepareStatement("create table " + CRLTestTable + " (c1 int null);"); createTable(s); @@ -172,6 +198,29 @@ public void testStatementRetryWithShortQueryTimeout(String addedRetryParams) thr } } + /** + * Tests connection retry. Used in other tests. + * + * @throws Exception + * if unable to connect or execute against db + */ + public void testConnectionRetry(String replacedDbName, String addedRetryParams) throws Exception { + String cxnString = connectionString + addedRetryParams; + cxnString = TestUtils.addOrOverrideProperty(cxnString, "database", replacedDbName); + + try (Connection conn = DriverManager.getConnection(cxnString); Statement s = conn.createStatement()) { + try { + fail(TestResource.getResource("R_expectedFailPassed")); + } catch (Exception e) { + System.out.println("blah"); + assertTrue(e.getMessage().startsWith("There is already an object"), + TestResource.getResource("R_unexpectedExceptionContent") + ": " + e.getMessage()); + } finally { + dropTable(s); + } + } + } + /** * Tests that the correct number of retries are happening for all statement scenarios. Tests are expected to take * a minimum of the sum of whatever has been defined for the waiting intervals, and maximum of the previous sum @@ -445,6 +494,91 @@ public void testRetryChange() throws Exception { } } + /** + * Tests that the correct number of retries are happening for all connection scenarios. Tests are expected to take + * a minimum of the sum of whatever has been defined for the waiting intervals, and maximum of the previous sum + * plus some amount of time to account for test environment slowness. + */ + @Test + public void connectionTimingTest() { + long totalTime; + long timerStart = System.currentTimeMillis(); + long expectedMaxTime = 10; + + // No retries since CRL rules override, expected time ~1 second + try { + testConnectionRetry("blah", "retryConn={9999};"); + } catch (Exception e) { + assertTrue( + (e.getMessage().toLowerCase() + .contains(TestResource.getResource("R_cannotOpenDatabase").toLowerCase())) + || (TestUtils.getProperty(connectionString, "msiClientId") != null && e.getMessage() + .toLowerCase().contains(TestResource.getResource("R_loginFailedMI").toLowerCase())) + || ((isSqlAzure() || isSqlAzureDW()) && e.getMessage().toLowerCase() + .contains(TestResource.getResource("R_connectTimedOut").toLowerCase())), + e.getMessage()); + + if (e.getMessage().toLowerCase().contains(TestResource.getResource("R_cannotOpenDatabase").toLowerCase())) { + // Only check the timing if the correct error, "cannot open database", is returned. + totalTime = System.currentTimeMillis() - timerStart; + assertTrue(totalTime < TimeUnit.SECONDS.toMillis(expectedMaxTime), + "total time: " + totalTime + ", expected time: " + TimeUnit.SECONDS.toMillis(expectedMaxTime)); + } + } + + timerStart = System.currentTimeMillis(); + long expectedMinTime = 10; + expectedMaxTime = 35; + + // (0s attempt + 0s attempt + 10s wait + 0s attempt) = expected 10s execution time + try { + testConnectionRetry("blah", "retryConn={4060,4070};connectRetryCount=2;connectRetryInterval=10"); + } catch (Exception e) { + assertTrue( + (e.getMessage().toLowerCase() + .contains(TestResource.getResource("R_cannotOpenDatabase").toLowerCase())) + || (TestUtils.getProperty(connectionString, "msiClientId") != null && e.getMessage() + .toLowerCase().contains(TestResource.getResource("R_loginFailedMI").toLowerCase())) + || ((isSqlAzure() || isSqlAzureDW()) && e.getMessage().toLowerCase() + .contains(TestResource.getResource("R_connectTimedOut").toLowerCase())), + e.getMessage()); + + if (e.getMessage().toLowerCase().contains(TestResource.getResource("R_cannotOpenDatabase").toLowerCase())) { + // Only check the timing if the correct error, "cannot open database", is returned. + totalTime = System.currentTimeMillis() - timerStart; + assertTrue(totalTime < TimeUnit.SECONDS.toMillis(expectedMaxTime), "total time: " + totalTime + + ", expected max time: " + TimeUnit.SECONDS.toMillis(expectedMaxTime)); + assertTrue(totalTime > TimeUnit.SECONDS.toMillis(expectedMinTime), "total time: " + totalTime + + ", expected min time: " + TimeUnit.SECONDS.toMillis(expectedMinTime)); + } + } + + timerStart = System.currentTimeMillis(); + + // Append should work the same way + try { + testConnectionRetry("blah", "retryConn={+4060,4070};connectRetryCount=2;connectRetryInterval=10"); + } catch (Exception e) { + assertTrue( + (e.getMessage().toLowerCase() + .contains(TestResource.getResource("R_cannotOpenDatabase").toLowerCase())) + || (TestUtils.getProperty(connectionString, "msiClientId") != null && e.getMessage() + .toLowerCase().contains(TestResource.getResource("R_loginFailedMI").toLowerCase())) + || ((isSqlAzure() || isSqlAzureDW()) && e.getMessage().toLowerCase() + .contains(TestResource.getResource("R_connectTimedOut").toLowerCase())), + e.getMessage()); + + if (e.getMessage().toLowerCase().contains(TestResource.getResource("R_cannotOpenDatabase").toLowerCase())) { + // Only check the timing if the correct error, "cannot open database", is returned. + totalTime = System.currentTimeMillis() - timerStart; + assertTrue(totalTime < TimeUnit.SECONDS.toMillis(expectedMaxTime), "total time: " + totalTime + + ", expected max time: " + TimeUnit.SECONDS.toMillis(expectedMaxTime)); + assertTrue(totalTime > TimeUnit.SECONDS.toMillis(expectedMinTime), "total time: " + totalTime + + ", expected min time: " + TimeUnit.SECONDS.toMillis(expectedMinTime)); + } + } + } + /** * Creates table for use in ConfigurableRetryLogic tests. * diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/connection/RequestBoundaryMethodsTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/connection/RequestBoundaryMethodsTest.java index 8d3a62788..402cccc8c 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/connection/RequestBoundaryMethodsTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/connection/RequestBoundaryMethodsTest.java @@ -523,6 +523,8 @@ private List getVerifiedMethodNames() { verifiedMethodNames.add("removeBeforeReconnectListener"); verifiedMethodNames.add("getRetryExec"); verifiedMethodNames.add("setRetryExec"); + verifiedMethodNames.add("getRetryConn"); + verifiedMethodNames.add("setRetryConn"); verifiedMethodNames.add("getUseFlexibleCallableStatements"); verifiedMethodNames.add("setUseFlexibleCallableStatements"); return verifiedMethodNames; From 52979870969698841fcb20099a17de41b41aa3ee Mon Sep 17 00:00:00 2001 From: Terry Chow <32403408+tkyc@users.noreply.github.com> Date: Wed, 20 Nov 2024 13:31:47 -0800 Subject: [PATCH 08/14] Deprecation of Security Manager fix (#2539) --- .../sqlserver/jdbc/KerbAuthentication.java | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/KerbAuthentication.java b/src/main/java/com/microsoft/sqlserver/jdbc/KerbAuthentication.java index 6f8f30a29..d02238196 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/KerbAuthentication.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/KerbAuthentication.java @@ -5,6 +5,8 @@ package com.microsoft.sqlserver.jdbc; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; import java.text.MessageFormat; @@ -94,8 +96,22 @@ private void initAuthInit() throws SQLServerException { Subject currentSubject; KerbCallback callback = new KerbCallback(con); try { - java.security.AccessControlContext context = java.security.AccessController.getContext(); - currentSubject = Subject.getSubject(context); + + try { + java.security.AccessControlContext context = java.security.AccessController.getContext(); + currentSubject = Subject.getSubject(context); + + } catch (UnsupportedOperationException ue) { + if (authLogger.isLoggable(Level.FINE)) { + authLogger.fine("JDK version does not support Subject.getSubject(), " + + "falling back to Subject.current() : " + ue.getMessage()); + } + + Method current = Subject.class.getDeclaredMethod("current"); + current.setAccessible(true); + currentSubject = (Subject) current.invoke(null); + } + if (null == currentSubject) { if (useDefaultJaas) { lc = new LoginContext(configName, null, callback, new JaasConfiguration(null)); @@ -159,6 +175,12 @@ private void initAuthInit() throws SQLServerException { } con.terminate(SQLServerException.DRIVER_ERROR_NONE, SQLServerException.getErrString("R_integratedAuthenticationFailed"), ge); + } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException ex) { + if (authLogger.isLoggable(Level.FINER)) { + authLogger.finer(toString() + "initAuthInit failed reflection exception:-" + ex); + } + con.terminate(SQLServerException.DRIVER_ERROR_NONE, + SQLServerException.getErrString("R_integratedAuthenticationFailed"), ex); } } From edbc7fc3be2da13e2b24d7785c22632ef0ccae6c Mon Sep 17 00:00:00 2001 From: Jeff Wasty Date: Fri, 22 Nov 2024 12:01:58 -0800 Subject: [PATCH 09/14] Update CHANGELOG.md (#2543) * Update CHANGELOG.md Updated changelog to reflect changes to 12.9.0 since last changelog was published. * Update CHANGELOG.md * Update CHANGELOG.md * Update CHANGELOG.md --- CHANGELOG.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f5d2883a..d0a42e759 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,16 +5,20 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) ## [12.9.0] Preview Release ### Added -- Added statement portion of configurable retry logic feature [#2396](https://github.com/microsoft/mssql-jdbc/pull/2396) +- Added configurable retry logic feature, supporting both statement, and connection, retry [#2396](https://github.com/microsoft/mssql-jdbc/pull/2396)[#2519](https://github.com/microsoft/mssql-jdbc/pull/2519) - Added JDK 23 support [#2515](https://github.com/microsoft/mssql-jdbc/pull/2515) ### Changed - Reverted "Execute Stored Procedures Directly" feature, as well as subsequent changes related to the feature [#2488](https://github.com/microsoft/mssql-jdbc/pull/2488) -- Changed MSAL logging from FINER to FINEST [#2489](https://github.com/microsoft/mssql-jdbc/pull/2489) +- Changed MSAL logging from FINEST to FINER [#2489](https://github.com/microsoft/mssql-jdbc/pull/2489) +- Updated project pom file to pull dependencies from public Azure Artifacts Feed [#2504](https://github.com/microsoft/mssql-jdbc/pull/2504) +- Changed how Kerberos authentication acquires subject to provide compatibility for Kerberos with Java 23 and above [#2539](https://github.com/microsoft/mssql-jdbc/pull/2539) ### Fixed issues - Changed driver behavior to allow prepared statement objects to be reused, preventing a "multiple queries are not allowed" error [#2482](https://github.com/microsoft/mssql-jdbc/pull/2482) - Adjusted DESTINATION_COL_METADATA_LOCK, in SQLServerBulkCopy, so that is properly released in all cases [#2484](https://github.com/microsoft/mssql-jdbc/pull/2484) +- Fixed connection retry behavior when `connectRetryCount` is set to a value greater than 1 [#2513](https://github.com/microsoft/mssql-jdbc/pull/2513) +- Resolved JavaDoc warnings that would appear during project build [#2521](https://github.com/microsoft/mssql-jdbc/pull/2521) ## [12.8.0] Stable Release ### Fixed issues From fc0cfabec0d52360953dc3f44bf7294ca7e02fa8 Mon Sep 17 00:00:00 2001 From: Jeff Wasty Date: Fri, 22 Nov 2024 12:30:36 -0800 Subject: [PATCH 10/14] Remove maximum time testing from CRL (#2545) * Remove maximum testing as this is very dependent on pipeline behavior. * Formatting * Undo formatting to satisfy code cov --- .../ConfigurableRetryLogicTest.java | 27 +++---------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/configurableretry/ConfigurableRetryLogicTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/configurableretry/ConfigurableRetryLogicTest.java index 3f522080d..b4f8fe4c7 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/configurableretry/ConfigurableRetryLogicTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/configurableretry/ConfigurableRetryLogicTest.java @@ -223,8 +223,8 @@ public void testConnectionRetry(String replacedDbName, String addedRetryParams) /** * Tests that the correct number of retries are happening for all statement scenarios. Tests are expected to take - * a minimum of the sum of whatever has been defined for the waiting intervals, and maximum of the previous sum - * plus some amount of time to account for test environment slowness. + * a minimum of the sum of whatever has been defined for the waiting intervals. Maximum is not tested due to the + * unpredictable factor of slowness that can be applied to these tests. */ @Test public void statementTimingTests() { @@ -253,8 +253,6 @@ public void statementTimingTests() { totalTime = System.currentTimeMillis() - timerStart; assertTrue(totalTime > TimeUnit.SECONDS.toMillis(5), "total time: " + totalTime + ", expected minimum time: " + TimeUnit.SECONDS.toMillis(5)); - assertTrue(totalTime < TimeUnit.SECONDS.toMillis(15), - "total time: " + totalTime + ", expected maximum time: " + TimeUnit.SECONDS.toMillis(15)); } timerStart = System.currentTimeMillis(); @@ -268,8 +266,6 @@ public void statementTimingTests() { totalTime = System.currentTimeMillis() - timerStart; assertTrue(totalTime > TimeUnit.SECONDS.toMillis(2), "total time: " + totalTime + ", expected minimum time: " + TimeUnit.SECONDS.toMillis(8)); - assertTrue(totalTime < TimeUnit.SECONDS.toMillis(18), - "total time: " + totalTime + ", expected maximum time: " + TimeUnit.SECONDS.toMillis(18)); } timerStart = System.currentTimeMillis(); @@ -283,8 +279,6 @@ public void statementTimingTests() { totalTime = System.currentTimeMillis() - timerStart; assertTrue(totalTime > TimeUnit.SECONDS.toMillis(3), "total time: " + totalTime + ", expected minimum time: " + TimeUnit.SECONDS.toMillis(10)); - assertTrue(totalTime < TimeUnit.SECONDS.toMillis(20), - "total time: " + totalTime + ", expected maximum time: " + TimeUnit.SECONDS.toMillis(20)); } } @@ -496,14 +490,13 @@ public void testRetryChange() throws Exception { /** * Tests that the correct number of retries are happening for all connection scenarios. Tests are expected to take - * a minimum of the sum of whatever has been defined for the waiting intervals, and maximum of the previous sum - * plus some amount of time to account for test environment slowness. + * a minimum of the sum of whatever has been defined for the waiting intervals. Maximum is not tested due to the + * unpredictable factor of slowness that can be applied to these tests. */ @Test public void connectionTimingTest() { long totalTime; long timerStart = System.currentTimeMillis(); - long expectedMaxTime = 10; // No retries since CRL rules override, expected time ~1 second try { @@ -517,18 +510,10 @@ public void connectionTimingTest() { || ((isSqlAzure() || isSqlAzureDW()) && e.getMessage().toLowerCase() .contains(TestResource.getResource("R_connectTimedOut").toLowerCase())), e.getMessage()); - - if (e.getMessage().toLowerCase().contains(TestResource.getResource("R_cannotOpenDatabase").toLowerCase())) { - // Only check the timing if the correct error, "cannot open database", is returned. - totalTime = System.currentTimeMillis() - timerStart; - assertTrue(totalTime < TimeUnit.SECONDS.toMillis(expectedMaxTime), - "total time: " + totalTime + ", expected time: " + TimeUnit.SECONDS.toMillis(expectedMaxTime)); - } } timerStart = System.currentTimeMillis(); long expectedMinTime = 10; - expectedMaxTime = 35; // (0s attempt + 0s attempt + 10s wait + 0s attempt) = expected 10s execution time try { @@ -546,8 +531,6 @@ public void connectionTimingTest() { if (e.getMessage().toLowerCase().contains(TestResource.getResource("R_cannotOpenDatabase").toLowerCase())) { // Only check the timing if the correct error, "cannot open database", is returned. totalTime = System.currentTimeMillis() - timerStart; - assertTrue(totalTime < TimeUnit.SECONDS.toMillis(expectedMaxTime), "total time: " + totalTime - + ", expected max time: " + TimeUnit.SECONDS.toMillis(expectedMaxTime)); assertTrue(totalTime > TimeUnit.SECONDS.toMillis(expectedMinTime), "total time: " + totalTime + ", expected min time: " + TimeUnit.SECONDS.toMillis(expectedMinTime)); } @@ -571,8 +554,6 @@ public void connectionTimingTest() { if (e.getMessage().toLowerCase().contains(TestResource.getResource("R_cannotOpenDatabase").toLowerCase())) { // Only check the timing if the correct error, "cannot open database", is returned. totalTime = System.currentTimeMillis() - timerStart; - assertTrue(totalTime < TimeUnit.SECONDS.toMillis(expectedMaxTime), "total time: " + totalTime - + ", expected max time: " + TimeUnit.SECONDS.toMillis(expectedMaxTime)); assertTrue(totalTime > TimeUnit.SECONDS.toMillis(expectedMinTime), "total time: " + totalTime + ", expected min time: " + TimeUnit.SECONDS.toMillis(expectedMinTime)); } From cb990f4d789baac0b127652c5a59973d9bb95451 Mon Sep 17 00:00:00 2001 From: Mahendra Chavan Date: Wed, 27 Nov 2024 23:11:23 +0530 Subject: [PATCH 11/14] issue#2537 - Fix for SQLServerConnection infinite loop (#2547) * issue#2537 - Fix for SQLServerConnection infinite loop * Added removeOpenStatement in finally block. * Removed volatile for openStatements and added it for requestStarted --- .../microsoft/sqlserver/jdbc/SQLServerConnection.java | 10 +++++++--- .../microsoft/sqlserver/jdbc/SQLServerStatement.java | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index 1571f5bec..18a27cc95 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -7598,7 +7598,7 @@ public T unwrap(Class iface) throws SQLException { } /** request started flag */ - private boolean requestStarted = false; + private volatile boolean requestStarted = false; /** original database autocommit mode */ private boolean originalDatabaseAutoCommitMode; @@ -7740,9 +7740,13 @@ void endRequestInternal() throws SQLException { sqlWarnings = originalSqlWarnings; if (null != openStatements) { while (!openStatements.isEmpty()) { - try (Statement st = openStatements.get(0)) {} + Statement st = openStatements.get(0); + try { + st.close(); + } finally { + removeOpenStatement((SQLServerStatement) st); + } } - openStatements.clear(); } requestStarted = false; } diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java index fef51bc6a..ab9e9fbe2 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java @@ -541,7 +541,7 @@ boolean onDone(TDSReader tdsReader) throws SQLServerException { /** * True is the statement is closed */ - boolean bIsClosed; + volatile boolean bIsClosed; /** * True if the user requested to driver to generate insert keys From ae462f95cd3a568158084406364ac81f305dd3f4 Mon Sep 17 00:00:00 2001 From: Ananya Garg Date: Sat, 30 Nov 2024 01:18:44 +0530 Subject: [PATCH 12/14] Remove exception and validation for user/password with AccessTokenCallback (#2549) * Removed the if statement, that throws exception R_AccessTokenCallbackWithUserPassword. This logic may be too restrictive and may be removed * Updated testDSPooledConnectionAccessTokenCallbackClassExceptions test case in PooledConnectionTest.java * Updated testDSPooledConnectionAccessTokenCallbackClassExceptions test case. * Added import statement. --- .../sqlserver/jdbc/SQLServerConnection.java | 7 ------ .../jdbc/fedauth/PooledConnectionTest.java | 23 +++++-------------- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index 18a27cc95..6874deab4 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -2802,13 +2802,6 @@ Connection connectInternal(Properties propsIn, && !activeConnectionProperties .getProperty(SQLServerDriverStringProperty.ACCESS_TOKEN_CALLBACK_CLASS.toString()) .isEmpty(); - if ((null != accessTokenCallback || hasAccessTokenCallbackClass) && (!activeConnectionProperties - .getProperty(SQLServerDriverStringProperty.USER.toString()).isEmpty() - || !activeConnectionProperties.getProperty(SQLServerDriverStringProperty.PASSWORD.toString()) - .isEmpty())) { - throw new SQLServerException( - SQLServerException.getErrString("R_AccessTokenCallbackWithUserPassword"), null); - } sPropKey = SQLServerDriverStringProperty.ACCESS_TOKEN_CALLBACK_CLASS.toString(); sPropValue = activeConnectionProperties.getProperty(sPropKey); diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/fedauth/PooledConnectionTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/fedauth/PooledConnectionTest.java index a7bf0f8c4..85a232fd0 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/fedauth/PooledConnectionTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/fedauth/PooledConnectionTest.java @@ -8,6 +8,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.assertNotNull; import java.lang.reflect.Field; import java.sql.Connection; @@ -435,27 +436,15 @@ public void testDSPooledConnectionAccessTokenCallbackClassExceptions() throws Ex // User/password is not required for access token callback AbstractTest.updateDataSource(accessTokenCallbackConnectionString, ds); + ds.setAccessTokenCallbackClass(AccessTokenCallbackClass.class.getName()); ds.setUser("user"); - SQLServerPooledConnection pc; - - // Should fail with user set - try { - pc = (SQLServerPooledConnection) ds.getPooledConnection(); - fail(TestResource.getResource("R_expectedFailPassed")); - } catch (SQLServerException e) { - assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_AccessTokenCallbackWithUserPassword"))); - } - - ds.setUser(""); ds.setPassword(UUID.randomUUID().toString()); + SQLServerPooledConnection pc; - // Should fail with password set - try { - pc = (SQLServerPooledConnection) ds.getPooledConnection(); - fail(TestResource.getResource("R_expectedFailPassed")); - } catch (SQLServerException e) { - assertTrue(e.getMessage().matches(TestUtils.formatErrorMsg("R_AccessTokenCallbackWithUserPassword"))); + pc = (SQLServerPooledConnection) ds.getPooledConnection(); + try (Connection conn1 = pc.getConnection()) { + assertNotNull(conn1); } // Should fail with invalid accessTokenCallbackClass value From 1967348b1a1f6f0ebc0bf4234f349986812fbaef Mon Sep 17 00:00:00 2001 From: Terry Chow <32403408+tkyc@users.noreply.github.com> Date: Mon, 2 Dec 2024 10:19:37 -0800 Subject: [PATCH 13/14] Updated changelog for 12.9.0 preview (#2552) --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0a42e759..552526457 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,12 +13,14 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) - Changed MSAL logging from FINEST to FINER [#2489](https://github.com/microsoft/mssql-jdbc/pull/2489) - Updated project pom file to pull dependencies from public Azure Artifacts Feed [#2504](https://github.com/microsoft/mssql-jdbc/pull/2504) - Changed how Kerberos authentication acquires subject to provide compatibility for Kerberos with Java 23 and above [#2539](https://github.com/microsoft/mssql-jdbc/pull/2539) +- Removed user and password check for AccessTokenCallback [#2549](https://github.com/microsoft/mssql-jdbc/pull/2549) ### Fixed issues - Changed driver behavior to allow prepared statement objects to be reused, preventing a "multiple queries are not allowed" error [#2482](https://github.com/microsoft/mssql-jdbc/pull/2482) - Adjusted DESTINATION_COL_METADATA_LOCK, in SQLServerBulkCopy, so that is properly released in all cases [#2484](https://github.com/microsoft/mssql-jdbc/pull/2484) - Fixed connection retry behavior when `connectRetryCount` is set to a value greater than 1 [#2513](https://github.com/microsoft/mssql-jdbc/pull/2513) - Resolved JavaDoc warnings that would appear during project build [#2521](https://github.com/microsoft/mssql-jdbc/pull/2521) +- Fixed infinite loop when removing open statement [#2547](https://github.com/microsoft/mssql-jdbc/pull/2547) ## [12.8.0] Stable Release ### Fixed issues From 6829848c355a73277d60697d41cc35879dfad62f Mon Sep 17 00:00:00 2001 From: David Engel Date: Tue, 10 Dec 2024 10:23:26 -0800 Subject: [PATCH 14/14] Fix ISQLServerConnection java doc reference (#2560) --- .../java/com/microsoft/sqlserver/jdbc/ISQLServerConnection.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerConnection.java index bd22c1fc9..52f1b67e6 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/ISQLServerConnection.java @@ -12,7 +12,7 @@ /** - * Provides an interface to the {@link SQLServerConnection} and {@link SQLServerConnectionPoolProxy} classes. + * Provides an interface to the {@link SQLServerConnection} class. */ public interface ISQLServerConnection extends java.sql.Connection {