From b673f008d7463e84556f88545fc3118098e54b46 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Wed, 26 Jan 2022 18:24:19 -0800 Subject: [PATCH 1/3] use reflection to check for mysql transient exception type --- .../storage/mysql/MySQLConnector.java | 21 ++++++-- .../storage/mysql/MySQLConnectorTest.java | 48 +++++++++++++++++++ .../druid/metadata/SQLMetadataConnector.java | 3 ++ 3 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 extensions-core/mysql-metadata-storage/src/test/java/org/apache/druid/metadata/storage/mysql/MySQLConnectorTest.java diff --git a/extensions-core/mysql-metadata-storage/src/main/java/org/apache/druid/metadata/storage/mysql/MySQLConnector.java b/extensions-core/mysql-metadata-storage/src/main/java/org/apache/druid/metadata/storage/mysql/MySQLConnector.java index fd3a638f7989..e0529d00a83c 100644 --- a/extensions-core/mysql-metadata-storage/src/main/java/org/apache/druid/metadata/storage/mysql/MySQLConnector.java +++ b/extensions-core/mysql-metadata-storage/src/main/java/org/apache/druid/metadata/storage/mysql/MySQLConnector.java @@ -19,11 +19,11 @@ package org.apache.druid.metadata.storage.mysql; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import com.google.inject.Inject; -import com.mysql.jdbc.exceptions.MySQLTransientException; import org.apache.commons.dbcp2.BasicDataSource; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.StringUtils; @@ -45,6 +45,7 @@ public class MySQLConnector extends SQLMetadataConnector private static final String SERIAL_TYPE = "BIGINT(20) AUTO_INCREMENT"; private static final String QUOTE_STRING = "`"; private static final String COLLATION = "CHARACTER SET utf8mb4 COLLATE utf8mb4_bin"; + private static final String MYSQL_TRANSIENT_EXCEPTION_CLASS_NAME = "com.mysql.jdbc.exceptions.MySQLTransientException"; private final DBI dbi; @@ -205,8 +206,7 @@ public boolean tableExists(Handle handle, String tableName) @Override protected boolean connectorIsTransientException(Throwable e) { - return e instanceof MySQLTransientException - || (e instanceof SQLException && ((SQLException) e).getErrorCode() == 1317 /* ER_QUERY_INTERRUPTED */); + return isTransientException(getClass().getClassLoader(), getDatasource().getDriverClassName(), e); } @Override @@ -241,4 +241,19 @@ public DBI getDBI() { return dbi; } + + @VisibleForTesting + static boolean isTransientException(ClassLoader classLoader, String driverName, Throwable e) + { + if (driverName.contains("mysql")) { + try { + Class clazz = Class.forName(MYSQL_TRANSIENT_EXCEPTION_CLASS_NAME, false, classLoader); + return e.getClass().equals(clazz) + || e instanceof SQLException && ((SQLException) e).getErrorCode() == 1317 /* ER_QUERY_INTERRUPTED */; + } + catch (ClassNotFoundException ignored) { + } + } + return false; + } } diff --git a/extensions-core/mysql-metadata-storage/src/test/java/org/apache/druid/metadata/storage/mysql/MySQLConnectorTest.java b/extensions-core/mysql-metadata-storage/src/test/java/org/apache/druid/metadata/storage/mysql/MySQLConnectorTest.java new file mode 100644 index 000000000000..29662e99d5f7 --- /dev/null +++ b/extensions-core/mysql-metadata-storage/src/test/java/org/apache/druid/metadata/storage/mysql/MySQLConnectorTest.java @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.metadata.storage.mysql; + +import com.mysql.jdbc.exceptions.MySQLTransientException; +import org.junit.Assert; +import org.junit.Test; + +import java.sql.SQLException; +import java.sql.SQLTransientConnectionException; + +public class MySQLConnectorTest +{ + @Test + public void testIsExceptionTransient() + { + final String driver = "com.mysql.jdbc.Driver"; + final ClassLoader loader = getClass().getClassLoader(); + Assert.assertTrue(MySQLConnector.isTransientException(loader, driver, new MySQLTransientException())); + Assert.assertTrue( + MySQLConnector.isTransientException(loader, driver, new SQLException("some transient failure", "wtf", 1317)) + ); + Assert.assertFalse( + MySQLConnector.isTransientException(loader, driver, new SQLException("totally realistic test data", "wtf", 1337)) + ); + // this method does not specially handle normal transient exceptions either, since it is not vendor specific + Assert.assertFalse( + MySQLConnector.isTransientException(loader, driver, new SQLTransientConnectionException("transient")) + ); + } +} diff --git a/server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java b/server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java index 80a81b44be4e..9fe81dcdffbf 100644 --- a/server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java +++ b/server/src/main/java/org/apache/druid/metadata/SQLMetadataConnector.java @@ -171,6 +171,9 @@ public final boolean isTransientException(Throwable e) || (e instanceof DBIException && isTransientException(e.getCause()))); } + /** + * Vendor specific errors that are not covered by {@link #isTransientException(Throwable)} + */ protected boolean connectorIsTransientException(Throwable e) { return false; From a27cb72ec096716e747bde77122b7461a19252b7 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Wed, 26 Jan 2022 22:03:10 -0800 Subject: [PATCH 2/3] better --- .../storage/mysql/MySQLConnector.java | 50 +++++++++++-------- .../storage/mysql/MySQLConnectorTest.java | 29 ++++++++--- 2 files changed, 52 insertions(+), 27 deletions(-) diff --git a/extensions-core/mysql-metadata-storage/src/main/java/org/apache/druid/metadata/storage/mysql/MySQLConnector.java b/extensions-core/mysql-metadata-storage/src/main/java/org/apache/druid/metadata/storage/mysql/MySQLConnector.java index e0529d00a83c..3629ef411b35 100644 --- a/extensions-core/mysql-metadata-storage/src/main/java/org/apache/druid/metadata/storage/mysql/MySQLConnector.java +++ b/extensions-core/mysql-metadata-storage/src/main/java/org/apache/druid/metadata/storage/mysql/MySQLConnector.java @@ -35,6 +35,7 @@ import org.skife.jdbi.v2.Handle; import org.skife.jdbi.v2.util.StringMapper; +import javax.annotation.Nullable; import java.io.File; import java.sql.SQLException; @@ -47,6 +48,7 @@ public class MySQLConnector extends SQLMetadataConnector private static final String COLLATION = "CHARACTER SET utf8mb4 COLLATE utf8mb4_bin"; private static final String MYSQL_TRANSIENT_EXCEPTION_CLASS_NAME = "com.mysql.jdbc.exceptions.MySQLTransientException"; + private final Class myTransientExceptionClass; private final DBI dbi; @Inject @@ -58,17 +60,13 @@ public MySQLConnector( ) { super(config, dbTables); - try { - log.info("Loading \"MySQL\" metadata connector driver %s", driverConfig.getDriverClassName()); - Class.forName(driverConfig.getDriverClassName(), false, getClass().getClassLoader()); - } - catch (ClassNotFoundException e) { - throw new ISE(e, "Could not find %s on the classpath. The MySQL Connector library is not included in the Druid " - + "distribution but is required to use MySQL. Please download a compatible library (for example " - + "'mysql-connector-java-5.1.48.jar') and place it under 'extensions/mysql-metadata-storage/'. See " - + "https://druid.apache.org/downloads for more details.", - driverConfig.getDriverClassName() - ); + log.info("Loading \"MySQL\" metadata connector driver %s", driverConfig.getDriverClassName()); + tryLoadDriverClass(driverConfig.getDriverClassName()); + + if (driverConfig.getDriverClassName().contains("mysql")) { + myTransientExceptionClass = tryLoadDriverClass(MYSQL_TRANSIENT_EXCEPTION_CLASS_NAME); + } else { + myTransientExceptionClass = null; } final BasicDataSource datasource = getDatasource(); @@ -206,7 +204,7 @@ public boolean tableExists(Handle handle, String tableName) @Override protected boolean connectorIsTransientException(Throwable e) { - return isTransientException(getClass().getClassLoader(), getDatasource().getDriverClassName(), e); + return isTransientException(myTransientExceptionClass, e); } @Override @@ -242,17 +240,27 @@ public DBI getDBI() return dbi; } + private Class tryLoadDriverClass(String className) + { + try { + return Class.forName(className, false, getClass().getClassLoader()); + } + catch (ClassNotFoundException e) { + throw new ISE(e, "Could not find %s on the classpath. The MySQL Connector library is not included in the Druid " + + "distribution but is required to use MySQL. Please download a compatible library (for example " + + "'mysql-connector-java-5.1.48.jar') and place it under 'extensions/mysql-metadata-storage/'. See " + + "https://druid.apache.org/downloads for more details.", + className + ); + } + } + @VisibleForTesting - static boolean isTransientException(ClassLoader classLoader, String driverName, Throwable e) + static boolean isTransientException(@Nullable Class driverClazz, Throwable e) { - if (driverName.contains("mysql")) { - try { - Class clazz = Class.forName(MYSQL_TRANSIENT_EXCEPTION_CLASS_NAME, false, classLoader); - return e.getClass().equals(clazz) - || e instanceof SQLException && ((SQLException) e).getErrorCode() == 1317 /* ER_QUERY_INTERRUPTED */; - } - catch (ClassNotFoundException ignored) { - } + if (driverClazz != null) { + return driverClazz.isAssignableFrom(e.getClass()) + || e instanceof SQLException && ((SQLException) e).getErrorCode() == 1317 /* ER_QUERY_INTERRUPTED */; } return false; } diff --git a/extensions-core/mysql-metadata-storage/src/test/java/org/apache/druid/metadata/storage/mysql/MySQLConnectorTest.java b/extensions-core/mysql-metadata-storage/src/test/java/org/apache/druid/metadata/storage/mysql/MySQLConnectorTest.java index 29662e99d5f7..d33931420cab 100644 --- a/extensions-core/mysql-metadata-storage/src/test/java/org/apache/druid/metadata/storage/mysql/MySQLConnectorTest.java +++ b/extensions-core/mysql-metadata-storage/src/test/java/org/apache/druid/metadata/storage/mysql/MySQLConnectorTest.java @@ -19,6 +19,7 @@ package org.apache.druid.metadata.storage.mysql; +import com.mysql.jdbc.exceptions.MySQLTransactionRollbackException; import com.mysql.jdbc.exceptions.MySQLTransientException; import org.junit.Assert; import org.junit.Test; @@ -31,18 +32,34 @@ public class MySQLConnectorTest @Test public void testIsExceptionTransient() { - final String driver = "com.mysql.jdbc.Driver"; - final ClassLoader loader = getClass().getClassLoader(); - Assert.assertTrue(MySQLConnector.isTransientException(loader, driver, new MySQLTransientException())); + final Class clazz = MySQLTransientException.class; + Assert.assertTrue(MySQLConnector.isTransientException(clazz, new MySQLTransientException())); + Assert.assertTrue(MySQLConnector.isTransientException(clazz, new MySQLTransactionRollbackException())); Assert.assertTrue( - MySQLConnector.isTransientException(loader, driver, new SQLException("some transient failure", "wtf", 1317)) + MySQLConnector.isTransientException(clazz, new SQLException("some transient failure", "wtf", 1317)) ); Assert.assertFalse( - MySQLConnector.isTransientException(loader, driver, new SQLException("totally realistic test data", "wtf", 1337)) + MySQLConnector.isTransientException(clazz, new SQLException("totally realistic test data", "wtf", 1337)) ); // this method does not specially handle normal transient exceptions either, since it is not vendor specific Assert.assertFalse( - MySQLConnector.isTransientException(loader, driver, new SQLTransientConnectionException("transient")) + MySQLConnector.isTransientException(clazz, new SQLTransientConnectionException("transient")) + ); + } + + @Test + public void testIsExceptionTransientNoMySqlClazz() + { + // no vendor specific for MariaDb, so should always be false + Assert.assertFalse(MySQLConnector.isTransientException(null, new MySQLTransientException())); + Assert.assertFalse( + MySQLConnector.isTransientException(null, new SQLException("some transient failure", "wtf", 1317)) + ); + Assert.assertFalse( + MySQLConnector.isTransientException(null, new SQLException("totally realistic test data", "wtf", 1337)) + ); + Assert.assertFalse( + MySQLConnector.isTransientException(null, new SQLTransientConnectionException("transient")) ); } } From b0b9583dea6e95dfd2dd2b01ab22274c7616cee8 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Wed, 26 Jan 2022 22:06:18 -0800 Subject: [PATCH 3/3] oops --- .../org/apache/druid/metadata/storage/mysql/MySQLConnector.java | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions-core/mysql-metadata-storage/src/main/java/org/apache/druid/metadata/storage/mysql/MySQLConnector.java b/extensions-core/mysql-metadata-storage/src/main/java/org/apache/druid/metadata/storage/mysql/MySQLConnector.java index 3629ef411b35..4cd2dd108457 100644 --- a/extensions-core/mysql-metadata-storage/src/main/java/org/apache/druid/metadata/storage/mysql/MySQLConnector.java +++ b/extensions-core/mysql-metadata-storage/src/main/java/org/apache/druid/metadata/storage/mysql/MySQLConnector.java @@ -48,6 +48,7 @@ public class MySQLConnector extends SQLMetadataConnector private static final String COLLATION = "CHARACTER SET utf8mb4 COLLATE utf8mb4_bin"; private static final String MYSQL_TRANSIENT_EXCEPTION_CLASS_NAME = "com.mysql.jdbc.exceptions.MySQLTransientException"; + @Nullable private final Class myTransientExceptionClass; private final DBI dbi;