Skip to content

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

Merged
lilgreenbird merged 3 commits intomainfrom
tryfix
Feb 29, 2024
Merged

Fix to allow connection retries to be disabled by setting connectRetryCount to 0#2293
lilgreenbird merged 3 commits intomainfrom
tryfix

Conversation

@lilgreenbird
Copy link
Copy Markdown
Contributor

No description provided.

@lilgreenbird lilgreenbird marked this pull request as ready for review January 16, 2024 23:37
@lilgreenbird lilgreenbird added this to the 12.6.0 milestone Jan 16, 2024
Comment thread src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java Outdated
@tkyc
Copy link
Copy Markdown
Contributor

tkyc commented Jan 18, 2024

I did a quick test and I see that it's retrying twice.

FINER: ConnectionID:1 Connection open - connection failed on attempt: 0.
Jan 18, 2024 9:56:19 A.M. com.microsoft.sqlserver.jdbc.SQLServerConnection login
FINER: ConnectionID:1 Connection open - connection failure. Driver error code: 0
Jan 18, 2024 9:56:19 A.M. com.microsoft.sqlserver.jdbc.SQLServerConnection login
FINER: ConnectionID:1 Connection open - sleeping milisec: 10
Jan 18, 2024 9:56:19 A.M. com.microsoft.sqlserver.jdbc.SQLServerConnection login
FINER: ConnectionID:1 Connection open - connection failed on transient error . Wait for connectRetryInterval(10)s before retry #0
Jan 18, 2024 9:56:19 A.M. com.microsoft.sqlserver.jdbc.SQLServerConnection login
FINE: ConnectionID:1 Connection open - connection retry failed on attempt number: 1
Jan 18, 2024 9:56:19 A.M. com.microsoft.sqlserver.jdbc.SQLServerConnection login
FINER: ConnectionID:1 Connection open - connection failed. Maximum connection retry count 0 reached.

Repro app

        String connectionUrl = "jdbc:sqlserver://sqlao1vm1.availability.local\\DEVINSTANCE;connectRetryCount=0;trustServerCertificate=true;databaseName=master;IntegratedSecurity=true;authenticationScheme=NTLM;user=;password=;domain=availability.local;";

        Handler fh = new ConsoleHandler();
        fh.setFormatter(new SimpleFormatter());
        fh.setLevel(Level.FINER);
        Logger.getLogger("").addHandler(fh);
        Logger logger = Logger.getLogger("com.microsoft.sqlserver.jdbc.Resiliency");
        logger.setLevel(Level.FINEST);

        try (Connection con = DriverManager.getConnection(connectionUrl); Statement stmt = con.createStatement();) {
            String SQL = "SELECT GETDATE() as dt";
            ResultSet rs = stmt.executeQuery(SQL);
            // Iterate through the data in the result set and display it.
            while (rs.next()) {
                System.out.println(rs.getString("dt"));
            }
        }

        // Handle any errors that may have occurred.
        catch (SQLException e) {
            e.printStackTrace();
        }

EDIT: Nevermind, it should be twice when TNIR=true.

@Jeffery-Wasty Jeffery-Wasty self-requested a review January 18, 2024 18:17
Jeffery-Wasty
Jeffery-Wasty previously approved these changes Jan 18, 2024
tkyc
tkyc previously approved these changes Jan 18, 2024
@lilgreenbird lilgreenbird dismissed stale reviews from tkyc and Jeffery-Wasty via d9c8db2 January 19, 2024 02:53
@Jeffery-Wasty Jeffery-Wasty modified the milestones: 12.6.0, 12.7.0 Jan 19, 2024
@lilgreenbird lilgreenbird marked this pull request as draft February 15, 2024 01:05
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 23, 2024

Codecov Report

Attention: Patch coverage is 55.26316% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 50.04%. Comparing base (cfb018a) to head (0622787).

❗ Current head 0622787 differs from pull request most recent head fc20b85. Consider uploading reports for the commit fc20b85 to get more accurate results

Files Patch % Lines
.../microsoft/sqlserver/jdbc/SQLServerConnection.java 56.25% 7 Missing and 7 partials ⚠️
...m/microsoft/sqlserver/jdbc/SQLServerException.java 50.00% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##               main    #2293       +/-   ##
=============================================
- Coverage     73.60%   50.04%   -23.56%     
+ Complexity     6082     3786     -2296     
=============================================
  Files           142      143        +1     
  Lines         33123    33128        +5     
  Branches       5623     5628        +5     
=============================================
- Hits          24380    16579     -7801     
- Misses         6234    14171     +7937     
+ Partials       2509     2378      -131     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@microsoft microsoft deleted a comment from azure-pipelines Bot Feb 25, 2024
@lilgreenbird lilgreenbird changed the title Fix for connectRetry=0 Fix to allow connection retries to be disabled by setting connectRetryCount to 0 Feb 25, 2024
@microsoft microsoft deleted a comment from azure-pipelines Bot Feb 26, 2024
@microsoft microsoft deleted a comment from azure-pipelines Bot Feb 26, 2024
@microsoft microsoft deleted a comment from azure-pipelines Bot Feb 26, 2024
@lilgreenbird lilgreenbird marked this pull request as ready for review February 26, 2024 19:45
@lilgreenbird lilgreenbird linked an issue Feb 27, 2024 that may be closed by this pull request
Comment thread src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java Outdated
Comment thread src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed/Merged PRs

Development

Successfully merging this pull request may close these issues.

Exception not immediately thrown after connect failure when connectRetryCount=0 Retry when DatabaseName is wrong

3 participants