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

performance optimization #965

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ public class ConnectionPluginManagerBenchmarks {
@Mock TelemetryContext mockTelemetryContext;
@Mock TelemetryCounter mockTelemetryCounter;
@Mock TelemetryGauge mockTelemetryGauge;
@Mock TargetDriverDialect mockTargetDriverDialect;
ConfigurationProfile configurationProfile;
private AutoCloseable closeable;

Expand Down Expand Up @@ -139,7 +140,7 @@ public void setUpIteration() throws Exception {
.thenReturn("myInstance1.domain.com", "myInstance2.domain.com", "myInstance3.domain.com");
when(mockPluginService.getCurrentConnection()).thenReturn(mockConnection);
when(mockPluginService.getTelemetryFactory()).thenReturn(mockTelemetryFactory);

when(mockPluginService.getTargetDriverDialect()).thenReturn(mockTargetDriverDialect);
// Create a plugin chain with 10 custom test plugins.
final List<Class<? extends ConnectionPluginFactory>> pluginFactories = new ArrayList<>(
Collections.nCopies(10, BenchmarkPluginFactory.class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.Properties;

public class HikariExample {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ public class ConnectionPluginManager implements CanReleaseResources, Wrapper {
protected final ConnectionWrapper connectionWrapper;
protected PluginService pluginService;
protected TelemetryFactory telemetryFactory;
protected final boolean isTelemetryEnabled;
protected Set<String> networkBoundedMethods;

@SuppressWarnings("rawtypes")
protected final Map<String, PluginChainJdbcCallable> pluginChainFuncMap = new HashMap<>();
Expand All @@ -119,6 +121,7 @@ public ConnectionPluginManager(
this.effectiveConnProvider = effectiveConnProvider;
this.connectionWrapper = connectionWrapper;
this.telemetryFactory = telemetryFactory;
this.isTelemetryEnabled = this.telemetryFactory.isEnabled();
}

/**
Expand Down Expand Up @@ -152,6 +155,7 @@ public ConnectionPluginManager(
this.plugins = plugins;
this.connectionWrapper = connectionWrapper;
this.telemetryFactory = telemetryFactory;
this.isTelemetryEnabled = this.telemetryFactory.isEnabled();
}

public void lock() {
Expand Down Expand Up @@ -199,6 +203,8 @@ public void init(
pluginManagerService,
props,
configurationProfile);

this.networkBoundedMethods = this.pluginService.getTargetDriverDialect().getNetworkBoundMethodNames();
}

protected <T, E extends Exception> T executeWithSubscribedPlugins(
Expand Down Expand Up @@ -259,14 +265,25 @@ protected <T, E extends Exception> PluginChainJdbcCallable<T, E> makePluginChain

if (isSubscribed) {
if (pluginChainFunc == null) {
pluginChainFunc = (pipelineFunc, jdbcFunc) ->
executeWithTelemetry(() -> pipelineFunc.call(plugin, jdbcFunc), pluginName);
if (this.isTelemetryEnabled) {
pluginChainFunc = (pipelineFunc, jdbcFunc) ->
executeWithTelemetry(() -> pipelineFunc.call(plugin, jdbcFunc), pluginName);
} else {
pluginChainFunc = (pipelineFunc, jdbcFunc) ->
pipelineFunc.call(plugin, jdbcFunc);
}
} else {
final PluginChainJdbcCallable<T, E> finalPluginChainFunc = pluginChainFunc;
pluginChainFunc = (pipelineFunc, jdbcFunc) ->
executeWithTelemetry(() -> pipelineFunc.call(
plugin, () -> finalPluginChainFunc.call(pipelineFunc, jdbcFunc)),
pluginName);
if (this.isTelemetryEnabled) {
pluginChainFunc = (pipelineFunc, jdbcFunc) ->
executeWithTelemetry(() -> pipelineFunc.call(
plugin, () -> finalPluginChainFunc.call(pipelineFunc, jdbcFunc)),
pluginName);
} else {
pluginChainFunc = (pipelineFunc, jdbcFunc) ->
pipelineFunc.call(
plugin, () -> finalPluginChainFunc.call(pipelineFunc, jdbcFunc));
}
}
}
}
Expand Down Expand Up @@ -315,16 +332,18 @@ public <T, E extends Exception> T execute(
final Object[] jdbcMethodArgs)
throws E {

// The target driver may block on Statement.getConnection().
if (!AsynchronousMethodsHelper.ASYNCHRONOUS_METHODS.contains(methodName)) {
final Connection conn = WrapperUtils.getConnectionFromSqlObject(methodInvokeOn);
if (conn != null
&& conn != this.pluginService.getCurrentConnection()
&& !sqlMethodAnalyzer.isMethodClosingSqlObject(methodName)) {
throw WrapperUtils.wrapExceptionIfNeeded(
exceptionClass,
new SQLException(
Messages.get("ConnectionPluginManager.invokedAgainstOldConnection", new Object[] {methodInvokeOn})));
if (this.networkBoundedMethods.contains(methodName)) {
// The target driver may block on Statement.getConnection().
if (!AsynchronousMethodsHelper.ASYNCHRONOUS_METHODS.contains(methodName)) {
final Connection conn = WrapperUtils.getConnectionFromSqlObject(methodInvokeOn);
if (conn != null
&& conn != this.pluginService.getCurrentConnection()
&& !sqlMethodAnalyzer.isMethodClosingSqlObject(methodName)) {
throw WrapperUtils.wrapExceptionIfNeeded(
exceptionClass,
new SQLException(
Messages.get("ConnectionPluginManager.invokedAgainstOldConnection", new Object[]{methodInvokeOn})));
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions wrapper/src/main/java/software/amazon/jdbc/PluginService.java
Original file line number Diff line number Diff line change
Expand Up @@ -184,4 +184,8 @@ HostSpec getHostSpecByStrategy(HostRole role, String strategy)
String getTargetName();

@NonNull SessionStateService getSessionStateService();

void setRequiredMaintainTransactionContext(final boolean required);

boolean isRequiredMaintainTransactionContext();
}
11 changes: 11 additions & 0 deletions wrapper/src/main/java/software/amazon/jdbc/PluginServiceImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public class PluginServiceImpl implements PluginService, CanReleaseResources,
protected final SessionStateService sessionStateService;

protected final ReentrantLock connectionSwitchLock = new ReentrantLock();
protected boolean isRequiredMaintainTransactionContext = false;

public PluginServiceImpl(
@NonNull final ConnectionPluginManager pluginManager,
Expand Down Expand Up @@ -658,4 +659,14 @@ public String getTargetName() {
public @NonNull SessionStateService getSessionStateService() {
return this.sessionStateService;
}

@Override
public void setRequiredMaintainTransactionContext(final boolean required) {
this.isRequiredMaintainTransactionContext = required;
}

@Override
public boolean isRequiredMaintainTransactionContext() {
return this.isRequiredMaintainTransactionContext;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,7 @@ public class AuroraConnectionTrackerPlugin extends AbstractConnectionPlugin impl

static final String METHOD_ABORT = "Connection.abort";
static final String METHOD_CLOSE = "Connection.close";
private static final Set<String> subscribedMethods =
Collections.unmodifiableSet(new HashSet<String>() {
{
addAll(SubscribedMethodHelper.NETWORK_BOUND_METHODS);
add("connect");
add("forceConnect");
add("notifyNodeListChanged");
}
});

private final Set<String> subscribedMethods;
private final PluginService pluginService;
private final RdsUtils rdsHelper;
private final OpenedConnectionTracker tracker;
Expand All @@ -71,6 +62,13 @@ public class AuroraConnectionTrackerPlugin extends AbstractConnectionPlugin impl
this.pluginService = pluginService;
this.rdsHelper = rdsUtils;
this.tracker = tracker;

final HashSet<String> methods = new HashSet<>();
methods.add("connect");
methods.add("forceConnect");
methods.add("notifyNodeListChanged");
methods.addAll(this.pluginService.getTargetDriverDialect().getNetworkBoundMethodNames());
this.subscribedMethods = Collections.unmodifiableSet(methods);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@
private final PluginService pluginService;
private final PluginManagerService pluginManagerService;

private final String targetName;
private final boolean isTelemetryEnabled;
private boolean isRequiredMaintainTransactionContext = false;

Check warning on line 73 in wrapper/src/main/java/software/amazon/jdbc/plugin/DefaultConnectionPlugin.java

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unused assignment

Variable `isRequiredMaintainTransactionContext` initializer `false` is redundant

public DefaultConnectionPlugin(
final PluginService pluginService,
final ConnectionProvider defaultConnProvider,
Expand Down Expand Up @@ -101,6 +105,9 @@
this.defaultConnProvider = defaultConnProvider;
this.effectiveConnProvider = effectiveConnProvider;
this.connProviderManager = connProviderManager;
this.targetName = this.pluginService.getTargetName();
this.isTelemetryEnabled = this.pluginService.getTelemetryFactory().isEnabled();
this.isRequiredMaintainTransactionContext = this.pluginService.isRequiredMaintainTransactionContext();
}

@Override
Expand All @@ -121,40 +128,48 @@
LOGGER.finest(
() -> Messages.get("DefaultConnectionPlugin.executingMethod", new Object[] {methodName}));

TelemetryFactory telemetryFactory = this.pluginService.getTelemetryFactory();
TelemetryContext telemetryContext = telemetryFactory.openTelemetryContext(
this.pluginService.getTargetName(), TelemetryTraceLevel.NESTED);
TelemetryContext telemetryContext = null;
if (this.isTelemetryEnabled) {
TelemetryFactory telemetryFactory = this.pluginService.getTelemetryFactory();
telemetryContext = telemetryFactory.openTelemetryContext(
this.targetName, TelemetryTraceLevel.NESTED);
}

T result;
try {
result = jdbcMethodFunc.call();
} finally {
telemetryContext.closeContext();
if (this.isTelemetryEnabled) {
telemetryContext.closeContext();
}
}

final Connection currentConn = this.pluginService.getCurrentConnection();
final Connection boundConnection = WrapperUtils.getConnectionFromSqlObject(methodInvokeOn);
if (boundConnection != null && boundConnection != currentConn) {
// The method being invoked is using an old connection, so transaction/autocommit analysis should be skipped.
// ConnectionPluginManager#execute blocks all methods invoked using old connections except for close/abort.
return result;
}
if (this.isRequiredMaintainTransactionContext) {
final Connection currentConn = this.pluginService.getCurrentConnection();
final Connection boundConnection = WrapperUtils.getConnectionFromSqlObject(methodInvokeOn);
if (boundConnection != null && boundConnection != currentConn) {
// The method being invoked is using an old connection, so transaction/autocommit analysis should be skipped.
// ConnectionPluginManager#execute blocks all methods invoked using old connections except for close/abort.
return result;
}

if (sqlMethodAnalyzer.doesOpenTransaction(currentConn, methodName, jdbcMethodArgs)) {
this.pluginManagerService.setInTransaction(true);
} else if (
sqlMethodAnalyzer.doesCloseTransaction(currentConn, methodName, jdbcMethodArgs)
// According to the JDBC spec, transactions are committed if autocommit is switched from false to true.
|| sqlMethodAnalyzer.doesSwitchAutoCommitFalseTrue(currentConn, methodName,
jdbcMethodArgs)) {
this.pluginManagerService.setInTransaction(false);
if (sqlMethodAnalyzer.doesOpenTransaction(currentConn, methodName, jdbcMethodArgs)) {
this.pluginManagerService.setInTransaction(true);
} else if (
sqlMethodAnalyzer.doesCloseTransaction(currentConn, methodName, jdbcMethodArgs)
// According to the JDBC spec, transactions are committed if autocommit is switched from false to true.
|| sqlMethodAnalyzer.doesSwitchAutoCommitFalseTrue(currentConn, methodName,
jdbcMethodArgs)) {
this.pluginManagerService.setInTransaction(false);
}
}

// TODO: verify and reconsider this logic
if (sqlMethodAnalyzer.isStatementSettingAutoCommit(methodName, jdbcMethodArgs)) {
final Boolean autocommit = sqlMethodAnalyzer.getAutoCommitValueFromSqlStatement(jdbcMethodArgs);
if (autocommit != null) {
try {
currentConn.setAutoCommit(autocommit);
this.pluginService.getCurrentConnection().setAutoCommit(autocommit);
} catch (final SQLException e) {
// do nothing
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,16 @@ public class HostMonitoringConnectionPlugin extends AbstractConnectionPlugin
"3",
"Number of failed connection checks before considering database node unhealthy.");

private static final Set<String> subscribedMethods =
Collections.unmodifiableSet(new HashSet<>(Collections.singletonList("*")));

protected @NonNull Properties properties;
private final @NonNull Supplier<MonitorService> monitorServiceSupplier;
private final @NonNull PluginService pluginService;
private MonitorService monitorService;
private final RdsUtils rdsHelper;
private HostSpec monitoringHostSpec;
protected final boolean isEnabled;
private final Set<String> subscribedMethods;



static {
PropertyDefinition.registerPluginProperties(HostMonitoringConnectionPlugin.class);
Expand Down Expand Up @@ -121,6 +122,15 @@ public HostMonitoringConnectionPlugin(
this.properties = properties;
this.monitorServiceSupplier = monitorServiceSupplier;
this.rdsHelper = rdsHelper;
this.isEnabled = FAILURE_DETECTION_ENABLED.getBoolean(this.properties);

final HashSet<String> methods = new HashSet<>();
if (this.isEnabled) {
methods.add("connect");
methods.add("forceConnect");
methods.addAll(this.pluginService.getTargetDriverDialect().getNetworkBoundMethodNames());
}
this.subscribedMethods = Collections.unmodifiableSet(methods);
}

@Override
Expand All @@ -142,10 +152,8 @@ public <T, E extends Exception> T execute(
final Object[] jdbcMethodArgs)
throws E {

// update config settings since they may change
final boolean isEnabled = FAILURE_DETECTION_ENABLED.getBoolean(this.properties);

if (!isEnabled || !SubscribedMethodHelper.NETWORK_BOUND_METHODS.contains(methodName)) {
if (!isEnabled || !this.subscribedMethods.contains(methodName)) {
// In this case the plugin isn't subscribed to any methods, and we shouldn't reach this code.
return jdbcMethodFunc.call();
}

Expand Down
Loading
Loading