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

Jump to bottom

fix: only update topology for specific methods #364

Merged
merged 3 commits into from
Mar 2, 2023
Merged

fix: only update topology for specific methods #364

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1b23d5e
fix: only update topology for specific network bounded methods to not…
karenc-bq Feb 28, 2023
30c7f09
chore: remove methods that cancel an execution workflow
karenc-bq Mar 1, 2023
fcaae22
refactor: rename method
karenc-bq Mar 1, 2023
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
38 changes: 35 additions & 3 deletions src/main/user-impl/java/com/mysql/cj/jdbc/ha/plugins/failover/FailoverConnectionPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import java.io.EOFException;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -91,6 +92,23 @@ public class FailoverConnectionPlugin implements IConnectionPlugin {
static final String METHOD_CLOSE = "close";
static final String METHOD_IS_CLOSED = "isClosed";

private static final List<String> METHODS_REQUIRE_UPDATED_TOPOLOGY = new ArrayList<>(Arrays.asList(
METHOD_COMMIT,
"connect",
"isValid",
"rollback",
"setAutoCommit",
"setReadOnly",
"execute",
"executeBatch",
"executeLargeBatch",
"executeLargeUpdate",
"executeQuery",
"executeUpdate",
"executeWithFlags",
"getParameterMetaData"
));

private static final String METHOD_GET_TRANSACTION_ISOLATION =
"getTransactionIsolation";
private static final String METHOD_GET_SESSION_MAX_ROWS = "getSessionMaxRows";
Expand Down Expand Up @@ -245,7 +263,9 @@ public Object execute(
Object result = null;

try {
updateTopologyAndConnectIfNeeded(false);
if (canUpdateTopology(methodName)) {
updateTopologyIfNeeded(false);
}
result = this.nextPlugin.execute(methodInvokeOn, methodName, executeSqlFunc, args);
} catch (IllegalStateException e) {
dealWithIllegalStateException(e);
Expand Down Expand Up @@ -432,6 +452,18 @@ private int getRandomReaderIndex() {
return (int) (Math.random() * ((max - min) + 1)) + min;
}

/**
* Updating topology requires creating and executing a new statement.
* This may cause interruptions during certain workflows. For instance,
* the driver should not be updating topology while the connection is fetching a large streaming result set.
*
* @param methodName the method to check.
* @return true if the driver should update topology before executing the method; false otherwise.
*/
private boolean canUpdateTopology(String methodName) {
return METHODS_REQUIRE_UPDATED_TOPOLOGY.contains(methodName);
}

protected void initializeTopology() throws SQLException {
if (this.currentConnectionProvider.getCurrentConnection() == null) {
return;
Expand Down Expand Up @@ -540,7 +572,7 @@ protected void failoverReader(int failedHostIdx) throws SQLException {
updateCurrentConnection(
result.getConnection(),
result.getConnectionIndex());
updateTopologyAndConnectIfNeeded(true);
updateTopologyIfNeeded(true);

if (this.currentHostIndex != NO_CONNECTION_INDEX
&& this.currentHostIndex != WRITER_CONNECTION_INDEX
Expand Down Expand Up @@ -710,7 +742,7 @@ protected void syncSessionState(
sourceUseLocalSessionState.setValue(prevUseLocalSessionState);
}

protected void updateTopologyAndConnectIfNeeded(boolean forceUpdate)
protected void updateTopologyIfNeeded(boolean forceUpdate)
throws SQLException {
final JdbcConnection connection = this.currentConnectionProvider.getCurrentConnection();
if (
Expand Down