(impala) 02/03: IMPALA-12470 (PART-3): delete temporary jar file in GenericJdbcDatabaseAccessor close() function

[mi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

michaelsmith pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 8fe471d4698180e1b8b726c278b8a066fe06e416
Author: gaurav1086 <ga...@gmail.com>
AuthorDate: Tue Nov 14 10:06:21 2023 -0800

    IMPALA-12470 (PART-3): delete temporary jar file
    in GenericJdbcDatabaseAccessor close() function
    
    The earlier change had a bug where we are deleting
    the temporary jdbc jar file too early from the
    /tmp directory before it can be loaded. The
    GenericJdbcDatabaseAccessor class loader works by
    OnDemand loading. Hence move the delete file logic
    to the GenericJdbcDatabaseAccessor close()
    function instead.
    
    Testing:
    1. Make sure the Impala cluster has been started.
    
    2. Copy the jar files of JDBC drivers and the data source library into
    HDFS.
    ${IMPALA_HOME}/testdata/bin/copy-ext-data-sources.sh
    
    Verify that the mysql-jdbc.jar is present in the hdfs path:
    hadoop fs -ls /test-warehouse/data-sources/jdbc-drivers
    
    3. Create an `alltypes` table in the mysql database.
    ${IMPALA_HOME}/testdata/bin/load-ext-data-sources.sh
    
    4. Create mysql data source tables (alltypes_jdbc_datasource and
    alltypes_jdbc_datasource_2).
    ${IMPALA_HOME}/bin/impala-shell.sh -f\
      ${IMPALA_HOME}/testdata/bin/create-ext-data-source-table.sql
    
    5. Make sure that the mysql jar file is not present in the classpath
    grep 'mysql' /home/gsingh/Impala/fe/target/build-classpath.txt \
    /home/gsingh/Impala/fe/target/test-classpath.txt \
    /home/gsingh/Impala/java/executor-deps/target/build-executor-\
    deps-classpath.txt | wc -l
    
    returns 0
    
    6. Run the impala-shell query:
    use functional;
    select count(*) from alltypes_jdbc_mysql_datasource;
    
    executes successfully and returns the row count.
    
    Change-Id: I1becc01a9d93a99be8f47dfe99258dea3a8abeb3
    Reviewed-on: http://gerrit.cloudera.org:8080/20706
    Reviewed-by: Wenzhe Zhou <wz...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java    | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java b/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
index 7e106f6b3..1da58fbbe 100644
--- a/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
+++ b/java/ext-data-source/jdbc/src/main/java/org/apache/impala/extdatasource/jdbc/dao/GenericJdbcDatabaseAccessor.java
@@ -64,6 +64,7 @@ public class GenericJdbcDatabaseAccessor implements DatabaseAccessor {
   protected static final int DEFAULT_FETCH_SIZE = 1000;
   protected static final int CACHE_EXPIRE_TIMEOUT_S = 1800;
   protected static final int CACHE_SIZE = 100;
+  protected String jdbcDriverLocalPath = null;
 
   protected DataSource dbcpDataSource = null;
   // Cache datasource for sharing
@@ -159,6 +160,11 @@ public class GenericJdbcDatabaseAccessor implements DatabaseAccessor {
       dataSourceCache.invalidateAll();
       dataSourceCache = null;
     }
+    if (jdbcDriverLocalPath != null) {
+      // Delete the jar file of jdbc driver.
+      Path localJarPath = new Path("file://" + jdbcDriverLocalPath);
+      FileSystemUtil.deleteIfExists(localJarPath);
+    }
   }
 
   /**
@@ -245,14 +251,12 @@ public class GenericJdbcDatabaseAccessor implements DatabaseAccessor {
                 // Create class loader for jdbc driver and set it for the
                 // BasicDataSource object so that the driver class could be loaded
                 // from jar file without searching classpath.
+                jdbcDriverLocalPath = driverLocalPath;
                 URL driverJarUrl = new File(driverLocalPath).toURI().toURL();
                 URLClassLoader driverLoader =
                     URLClassLoader.newInstance( new URL[] { driverJarUrl },
                         getClass().getClassLoader());
                 basicDataSource.setDriverClassLoader(driverLoader);
-                // Delete the jar file once its loaded
-                Path localJarPath = new Path("file://" + driverLocalPath);
-                FileSystemUtil.deleteIfExists(localJarPath);
                 return basicDataSource;
               });
         }