Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear prepared statement handle before reconnect #2364

Merged
merged 4 commits into from
Apr 3, 2024

Conversation

lukasaignerrsg
Copy link
Contributor

This fix clears the SQLServerPreparedStatement.prepStmtHandle before reconnecting.
The provided interface can also be used for other objects that need to handle reconnecting.

One test fails: PropertyTest.testRetryCount:54->testInvalidPropertyOverBrokenConnection:46 Unexpected error message occurred!Eine bestehende Verbindung wurde softwaregesteuert
durch den Hostcomputer abgebrochen
This might be due to the system being German. The test also failed before the fix.

@lukasaignerrsg
Copy link
Contributor Author

@lukasaignerrsg please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree company="Raiffeisen Software GmbH"

Copy link

codecov bot commented Mar 25, 2024

Codecov Report

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

Project coverage is 50.13%. Comparing base (c74a51e) to head (6d5258b).
Report is 11 commits behind head on main.

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

Files Patch % Lines
...a/com/microsoft/sqlserver/jdbc/SQLServerError.java 0.00% 6 Missing ⚠️
...in/java/com/microsoft/sqlserver/jdbc/IOBuffer.java 50.00% 3 Missing and 1 partial ⚠️
...oft/sqlserver/jdbc/SQLServerBulkCSVFileRecord.java 80.00% 3 Missing and 1 partial ⚠️
.../microsoft/sqlserver/jdbc/SQLServerConnection.java 86.66% 2 Missing ⚠️
...rc/main/java/com/microsoft/sqlserver/jdbc/dtv.java 33.33% 1 Missing and 1 partial ⚠️
...oft/sqlserver/jdbc/SQLServerPreparedStatement.java 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2364      +/-   ##
============================================
+ Coverage     50.03%   50.13%   +0.09%     
- Complexity     3804     3828      +24     
============================================
  Files           145      145              
  Lines         33304    33360      +56     
  Branches       5641     5655      +14     
============================================
+ Hits          16665    16726      +61     
+ Misses        14248    14242       -6     
- Partials       2391     2392       +1     

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

@lilgreenbird
Copy link
Contributor

hi @lukasaignerrsg

thank you for submitting this I have marked this for review

@lilgreenbird lilgreenbird changed the title Bugfix/2352 Clear prepared statement handle before reconnect Mar 26, 2024
@lilgreenbird lilgreenbird added this to the 12.7.0 milestone Mar 26, 2024
tkyc
tkyc previously approved these changes Apr 1, 2024
private void clearPrepStmtHandle(){
prepStmtHandle = 0;
cachedPreparedStatementHandle = null;
if(getStatementLogger().isLoggable(Level.FINER)){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor formatting issue. Should have a space preceeding the initila if statement brace and after the closing brace

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if(getStatementLogger().isLoggable(Level.FINER)){
if (getStatementLogger().isLoggable(Level.FINER)){

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls run formatter on all files

lilgreenbird
lilgreenbird previously approved these changes Apr 2, 2024
@lilgreenbird
Copy link
Contributor

ignore failing pipelines it's an issue in our test lab

private void clearPrepStmtHandle(){
prepStmtHandle = 0;
cachedPreparedStatementHandle = null;
if(getStatementLogger().isLoggable(Level.FINER)){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if(getStatementLogger().isLoggable(Level.FINER)){
if (getStatementLogger().isLoggable(Level.FINER)){

Jeffery-Wasty
Jeffery-Wasty previously approved these changes Apr 3, 2024
@Jeffery-Wasty Jeffery-Wasty self-requested a review April 3, 2024 00:47
@lilgreenbird lilgreenbird merged commit 51ca5a0 into microsoft:main Apr 3, 2024
17 checks passed
Jeffery-Wasty added a commit that referenced this pull request May 17, 2024
Jeffery-Wasty added a commit that referenced this pull request May 22, 2024
dongjoon-hyun pushed a commit to apache/spark that referenced this pull request Aug 3, 2024
…QLServer docker image tag to `2022-CU14-ubuntu-22.04`

### What changes were proposed in this pull request?

This PR aims to upgrade `mssql-jdbc` to 12.8.0.jre11 and MySQLServer docker image to `mcr.microsoft.com/mssql/server:2022-CU14-ubuntu-22.04`.

### Why are the changes needed?

This is the latest stable version of `mssql-jdbc`,  related release notes:

https://github.com/microsoft/mssql-jdbc/releases/tag/v12.7.0
https://github.com/microsoft/mssql-jdbc/releases/tag/v12.7.1
https://github.com/microsoft/mssql-jdbc/releases/tag/v12.8.0

Some fixed issues:

- Fix to ensure metadata returned follows JDBC data type specs microsoft/mssql-jdbc#2326
- Added token cache map to fix use of unintended auth token for subsequent connections microsoft/mssql-jdbc#2341
- Clear prepared statement handle before reconnect microsoft/mssql-jdbc#2364
- Reset socketTimeout to original value after a successful connection open microsoft/mssql-jdbc#2355
- Clear prepared statement cache when resetting statement pool connection microsoft/mssql-jdbc#2361
- Fixed ClassLoader leak of ActivityCorrelator ThreadLocal microsoft/mssql-jdbc#2366

### Does this PR introduce _any_ user-facing change?

No.
### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #47569 from wayneguow/ms_12_8.

Authored-by: Wei Guo <guow93@gmail.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
fusheng-rd pushed a commit to fusheng-rd/spark that referenced this pull request Aug 6, 2024
…QLServer docker image tag to `2022-CU14-ubuntu-22.04`

### What changes were proposed in this pull request?

This PR aims to upgrade `mssql-jdbc` to 12.8.0.jre11 and MySQLServer docker image to `mcr.microsoft.com/mssql/server:2022-CU14-ubuntu-22.04`.

### Why are the changes needed?

This is the latest stable version of `mssql-jdbc`,  related release notes:

https://github.com/microsoft/mssql-jdbc/releases/tag/v12.7.0
https://github.com/microsoft/mssql-jdbc/releases/tag/v12.7.1
https://github.com/microsoft/mssql-jdbc/releases/tag/v12.8.0

Some fixed issues:

- Fix to ensure metadata returned follows JDBC data type specs microsoft/mssql-jdbc#2326
- Added token cache map to fix use of unintended auth token for subsequent connections microsoft/mssql-jdbc#2341
- Clear prepared statement handle before reconnect microsoft/mssql-jdbc#2364
- Reset socketTimeout to original value after a successful connection open microsoft/mssql-jdbc#2355
- Clear prepared statement cache when resetting statement pool connection microsoft/mssql-jdbc#2361
- Fixed ClassLoader leak of ActivityCorrelator ThreadLocal microsoft/mssql-jdbc#2366

### Does this PR introduce _any_ user-facing change?

No.
### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#47569 from wayneguow/ms_12_8.

Authored-by: Wei Guo <guow93@gmail.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
attilapiros pushed a commit to attilapiros/spark that referenced this pull request Oct 4, 2024
…QLServer docker image tag to `2022-CU14-ubuntu-22.04`

### What changes were proposed in this pull request?

This PR aims to upgrade `mssql-jdbc` to 12.8.0.jre11 and MySQLServer docker image to `mcr.microsoft.com/mssql/server:2022-CU14-ubuntu-22.04`.

### Why are the changes needed?

This is the latest stable version of `mssql-jdbc`,  related release notes:

https://github.com/microsoft/mssql-jdbc/releases/tag/v12.7.0
https://github.com/microsoft/mssql-jdbc/releases/tag/v12.7.1
https://github.com/microsoft/mssql-jdbc/releases/tag/v12.8.0

Some fixed issues:

- Fix to ensure metadata returned follows JDBC data type specs microsoft/mssql-jdbc#2326
- Added token cache map to fix use of unintended auth token for subsequent connections microsoft/mssql-jdbc#2341
- Clear prepared statement handle before reconnect microsoft/mssql-jdbc#2364
- Reset socketTimeout to original value after a successful connection open microsoft/mssql-jdbc#2355
- Clear prepared statement cache when resetting statement pool connection microsoft/mssql-jdbc#2361
- Fixed ClassLoader leak of ActivityCorrelator ThreadLocal microsoft/mssql-jdbc#2366

### Does this PR introduce _any_ user-facing change?

No.
### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#47569 from wayneguow/ms_12_8.

Authored-by: Wei Guo <guow93@gmail.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
himadripal pushed a commit to himadripal/spark that referenced this pull request Oct 19, 2024
…QLServer docker image tag to `2022-CU14-ubuntu-22.04`

### What changes were proposed in this pull request?

This PR aims to upgrade `mssql-jdbc` to 12.8.0.jre11 and MySQLServer docker image to `mcr.microsoft.com/mssql/server:2022-CU14-ubuntu-22.04`.

### Why are the changes needed?

This is the latest stable version of `mssql-jdbc`,  related release notes:

https://github.com/microsoft/mssql-jdbc/releases/tag/v12.7.0
https://github.com/microsoft/mssql-jdbc/releases/tag/v12.7.1
https://github.com/microsoft/mssql-jdbc/releases/tag/v12.8.0

Some fixed issues:

- Fix to ensure metadata returned follows JDBC data type specs microsoft/mssql-jdbc#2326
- Added token cache map to fix use of unintended auth token for subsequent connections microsoft/mssql-jdbc#2341
- Clear prepared statement handle before reconnect microsoft/mssql-jdbc#2364
- Reset socketTimeout to original value after a successful connection open microsoft/mssql-jdbc#2355
- Clear prepared statement cache when resetting statement pool connection microsoft/mssql-jdbc#2361
- Fixed ClassLoader leak of ActivityCorrelator ThreadLocal microsoft/mssql-jdbc#2366

### Does this PR introduce _any_ user-facing change?

No.
### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes apache#47569 from wayneguow/ms_12_8.

Authored-by: Wei Guo <guow93@gmail.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
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
5 participants