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.

Sign up for GitHub

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

Re-introduce Retry logic for prepared statement caching #618

Merged
merged 13 commits into from
Feb 15, 2018
Merged

Re-introduce Retry logic for prepared statement caching #618

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a6eee22
change back to retryLogic and change the default of prepared statemen…
AfsanehR-zz Jan 29, 2018
69edc7b
check if statementPooling is enabled for all retry logics
AfsanehR-zz Jan 30, 2018
b8e9ff9
add the setter and getter methods for disableStatementPooling property
AfsanehR-zz Jan 31, 2018
af76bfc
move same code in a function.
AfsanehR-zz Jan 31, 2018
fee85b2
Merge branch 'dev' of https://github.com/Microsoft/mssql-jdbc into re…
AfsanehR-zz Jan 31, 2018
a237ebe
prepare cache in setDisableStatementPooling()
AfsanehR-zz Jan 31, 2018
63ab78f
a small fix
AfsanehR-zz Jan 31, 2018
dbefa1c
fixed the double insertion issue with retry
AfsanehR-zz Jan 31, 2018
870e853
fixed some review comments and updated test to use raiseError
AfsanehR-zz Feb 1, 2018
f523ca9
added a fix with connection property
AfsanehR-zz Feb 1, 2018
6ed48e2
added unit Tests
AfsanehR-zz Feb 1, 2018
e221b32
Revert back needsPrepare check in retryBasedOnFailedReuseOfCachedHandle
cheenamalhotra Feb 14, 2018
9ae0843
Remove unwanted method parameter
cheenamalhotra Feb 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@ final void doExecutePreparedStatement(PrepStmtExecCmd command) throws SQLServerE
}

String dbName = connection.getSCatalog();
boolean needsPrepare = false;
// Retry execution if existing handle could not be re-used.
for (int attempt = 1; attempt <= 2; ++attempt) {
try {
Expand All @@ -540,14 +541,14 @@ final void doExecutePreparedStatement(PrepStmtExecCmd command) throws SQLServerE
// continue using it after we return.
TDSWriter tdsWriter = command.startRequest(TDS.PKT_RPC);

doPrepExec(tdsWriter, inOutParam, hasNewTypeDefinitions, hasExistingTypeDefinitions);
needsPrepare = doPrepExec(tdsWriter, inOutParam, hasNewTypeDefinitions, hasExistingTypeDefinitions);

ensureExecuteResultsReader(command.startResponse(getIsResponseBufferingAdaptive()));
startResults();
getNextResult();
}
catch (SQLException e) {
if (retryBasedOnFailedReuseOfCachedHandle(e, attempt) && connection.isStatementPoolingEnabled())
if (retryBasedOnFailedReuseOfCachedHandle(e, attempt, needsPrepare) && connection.isStatementPoolingEnabled())
continue;
else
throw e;
Expand All @@ -565,13 +566,18 @@ else if (EXECUTE_UPDATE == executeMethod && null != resultSet) {

/** Should the execution be retried because the re-used cached handle could not be re-used due to server side state changes? */
private boolean retryBasedOnFailedReuseOfCachedHandle(SQLException e,
Copy link
Contributor

@TobiasSQL TobiasSQL Feb 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Posting here as well as the method changed. Suggest changing retryBasedOnFailedReuseOfCachedHandle to take care of the connection.IsStatementPoolingEnabled() check which is part of whether or not to reuse.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. Thanks!

int attempt) {
int attempt, boolean needsPrepare) {
// Only retry based on these error codes:
// 586: The prepared statement handle %d is not valid in this context. Please verify that current database, user default schema, and
// ANSI_NULLS and QUOTED_IDENTIFIER set options are not changed since the handle is prepared.
// 8179: Could not find prepared statement with handle %d.
// 99586: Error used for testing.
return 1 == attempt && (586 == e.getErrorCode() || 8179 == e.getErrorCode() || 99586 == e.getErrorCode());
if (needsPrepare) {
return false;
}
else {
return 1 == attempt && (586 == e.getErrorCode() || 8179 == e.getErrorCode() || 99586 == e.getErrorCode());
}
}

/**
Expand Down Expand Up @@ -2628,6 +2634,7 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th
}

String dbName = connection.getSCatalog();
boolean needsPrepare = false;
// Retry execution if existing handle could not be re-used.
for (int attempt = 1; attempt <= 2; ++attempt) {
try {
Expand Down Expand Up @@ -2655,8 +2662,8 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th
// the size of a batch's string parameter values changes such
// that repreparation is necessary.
++numBatchesPrepared;

if (doPrepExec(tdsWriter, batchParam, hasNewTypeDefinitions, hasExistingTypeDefinitions) || numBatchesPrepared == numBatches) {
needsPrepare = doPrepExec(tdsWriter, batchParam, hasNewTypeDefinitions, hasExistingTypeDefinitions);
if ( needsPrepare || numBatchesPrepared == numBatches) {
ensureExecuteResultsReader(batchCommand.startResponse(getIsResponseBufferingAdaptive()));

boolean retry = false;
Expand Down Expand Up @@ -2690,7 +2697,7 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th
throw e;

// Retry if invalid handle exception.
if (retryBasedOnFailedReuseOfCachedHandle(e, attempt) && connection.isStatementPoolingEnabled()) {
if (retryBasedOnFailedReuseOfCachedHandle(e, attempt, needsPrepare) && connection.isStatementPoolingEnabled()) {
// reset number of batches prepare
numBatchesPrepared = numBatchesExecuted;
retry = true;
Expand Down Expand Up @@ -2721,7 +2728,7 @@ final void doExecutePreparedStatementBatch(PrepStmtBatchExecCmd batchCommand) th
}
}
catch (SQLException e) {
if (retryBasedOnFailedReuseOfCachedHandle(e, attempt) && connection.isStatementPoolingEnabled()) {
if (retryBasedOnFailedReuseOfCachedHandle(e, attempt, needsPrepare) && connection.isStatementPoolingEnabled()) {
// Reset number of batches prepared.
numBatchesPrepared = numBatchesExecuted;
continue;
Expand Down