This is an automated email from the ASF dual-hosted git repository. joemcdonnell pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit afa329fd8961f766c48bb81859b47b078d5e2b55 Author: Zoltan Borok-Nagy <[email protected]> AuthorDate: Thu Apr 17 17:20:35 2025 +0200 IMPALA-13931: TestIcebergRestCatalog.test_rest_catalog_basic failed at setup There were several issues with test_rest_catalog_basic which made it fail in environments that used Ozone or S3. Missing dependency of Ozone and S3 classes: * This is resolved in iceberg-rest-catalog-test/pom.xml by adding a dependency to impala-executor-deps Hadoop configuration was initialized properly: * run-iceberg-rest-server.sh used Maven to run Iceberg REST Catalog in which case Maven is in charge of setting the CLASSPATH but the core-site/ozone-site/etc. config files were not on it, so the REST Catalog used a default Hadoop configuration that wasn't good for our environment. * To overcome the CLASSPATH problem now we create a runnable JAR in iceberg-rest-catalog-test/pom.xml and also generate the proper CLASSPATH during compilation. * run-iceberg-rest-server.sh now uses java -cp to run the REST Catalog S3 builds threw NoSuchMethodException for the "create" method of ApacheHttpClientConfigurations: * The Iceberg library dynamically load its http client builders to workaround an error, see details in https://github.com/apache/iceberg/issues/6715 * So the Iceberg lib dynamically wants to load the "create" method of its own ApacheHttpClientConfigurations class but it fails with NoSuchMethodException. * The critical code is invoked from Impala's IcebergMetadataScanner's ScanMetadataTable() method which happens to be invoked through JNI from the C++ backend. * The context class loader of such threads are NULL, which means Java will use the bootstrap class loader to load classes and methods, but that doesn't have the proper resources on its classpath. * To overcome this issue we set the context class loader for the thread to the class loader that originally loaded the IcebergMetadataScanner class. Change-Id: I9dc0e30aeaff0b8de41426ba38506383b4af472c Reviewed-on: http://gerrit.cloudera.org:8080/22818 Reviewed-by: Jason Fehr <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> Reviewed-by: Riza Suminto <[email protected]> --- .../impala/catalog/iceberg/IcebergCatalog.java | 8 ++--- .../java/org/apache/impala/common/JniUtil.java | 13 +++++++ .../apache/impala/util/IcebergMetadataScanner.java | 5 +++ java/iceberg-rest-catalog-test/pom.xml | 42 ++++++++++++++++++++-- .../iceberg/rest/IcebergRestCatalogTest.java | 7 ++-- testdata/bin/run-iceberg-rest-server.sh | 19 ++++++---- 6 files changed, 79 insertions(+), 15 deletions(-) diff --git a/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java b/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java index d75e7359c..6b6b04891 100644 --- a/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java +++ b/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalog.java @@ -26,6 +26,7 @@ import org.apache.impala.catalog.FeIcebergTable; import org.apache.impala.catalog.IcebergTableLoadingException; import org.apache.impala.catalog.TableLoadingException; import org.apache.impala.common.ImpalaRuntimeException; +import org.apache.impala.common.JniUtil; /** * Interface for Iceberg catalogs. Only contains a minimal set of methods to make @@ -86,14 +87,13 @@ public interface IcebergCatalog { /** * Some of the implemetation methods might be running on native threads as they might * be invoked via JNI. In that case the context class loader for those threads are - * null. 'Catalogs' uses JNDI to load the catalog implementations, e.g. HadoopCatalog - * or HiveCatalog. JNDI uses the context class loader, but as it is null it falls back + * null. 'Catalogs' dynamically loads catalog implementations, e.g. HadoopCatalog or + * HiveCatalog. It uses the context class loader, but as it is null it falls back * to the bootstrap class loader that doesn't have the Iceberg classes on its classpath. * To avoid ClassNotFoundException we set the context class loader to the class loader * that loaded this class. */ default void setContextClassLoader() { - if (Thread.currentThread().getContextClassLoader() != null) return; - Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader()); + JniUtil.setContextClassLoaderForThisThread(this.getClass().getClassLoader()); } } diff --git a/fe/src/main/java/org/apache/impala/common/JniUtil.java b/fe/src/main/java/org/apache/impala/common/JniUtil.java index decefcc8e..b55d7ee5b 100644 --- a/fe/src/main/java/org/apache/impala/common/JniUtil.java +++ b/fe/src/main/java/org/apache/impala/common/JniUtil.java @@ -410,4 +410,17 @@ public class JniUtil { } return groups; } + + /** + * Some methods might be running on native threads as they might be invoked via JNI. + * In that case the context class loader for those threads are null. Dynamic class + * loading uses the context class loader, but as it is null it falls back to the + * bootstrap class loader that doesn't have the Iceberg classes on its classpath. To + * avoid ClassNotFoundException we can set the context class loader with this method. + * It will only set the context class loader if it is NULL. + */ + public static void setContextClassLoaderForThisThread(ClassLoader cl) { + if (Thread.currentThread().getContextClassLoader() != null) return; + Thread.currentThread().setContextClassLoader(cl); + } } diff --git a/fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java b/fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java index 2d3253281..8596c5703 100644 --- a/fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java +++ b/fe/src/main/java/org/apache/impala/util/IcebergMetadataScanner.java @@ -31,6 +31,7 @@ import org.apache.iceberg.MetadataTableType; import org.apache.iceberg.MetadataTableUtils; import org.apache.iceberg.StructLike; import org.apache.impala.catalog.FeIcebergTable; +import org.apache.impala.common.JniUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.iceberg.Table; @@ -78,6 +79,10 @@ public class IcebergMetadataScanner { * the Iceberg Api. */ public void ScanMetadataTable() { + // If we are on a stack frame that was created through JNI we need to set the context + // class loader as Iceberg might use reflection to dynamically load classes and + // methods. + JniUtil.setContextClassLoaderForThisThread(this.getClass().getClassLoader()); // Create and scan the metadata table LOG.trace("Metadata table schema: " + metadataTable_.schema().toString()); TableScan scan = metadataTable_.newScan(); diff --git a/java/iceberg-rest-catalog-test/pom.xml b/java/iceberg-rest-catalog-test/pom.xml index ea3278a1a..4400bc8a1 100644 --- a/java/iceberg-rest-catalog-test/pom.xml +++ b/java/iceberg-rest-catalog-test/pom.xml @@ -33,12 +33,19 @@ under the License. <name>Iceberg REST Catalog Test</name> <dependencies> + + <dependency> + <groupId>org.apache.impala</groupId> + <artifactId>impala-executor-deps</artifactId> + <version>${project.version}</version> + <type>pom</type> + </dependency> + <dependency> <groupId>org.apache.hadoop</groupId> <artifactId>hadoop-common</artifactId> <version>${hadoop.version}</version> <exclusions> - <!-- IMPALA-9468: Avoid pulling in netty for security reasons --> <exclusion> <groupId>io.netty</groupId> <artifactId>*</artifactId> @@ -66,7 +73,6 @@ under the License. <version>${iceberg.version}</version> </dependency> - <dependency> <groupId>org.apache.iceberg</groupId> <artifactId>iceberg-core</artifactId> @@ -83,6 +89,24 @@ under the License. <build> <plugins> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-dependency-plugin</artifactId> + <version>3.5.0</version> + <executions> + <execution> + <id>write-classpath</id> + <goals> + <goal>build-classpath</goal> + </goals> + <configuration> + <outputFile>${project.build.directory}/build-classpath.txt</outputFile> + <includeScope>runtime</includeScope> + <excludeTypes>pom</excludeTypes> + </configuration> + </execution> + </executions> + </plugin> <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-compiler-plugin</artifactId> @@ -92,7 +116,6 @@ under the License. <target>1.8</target> </configuration> </plugin> - <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-surefire-plugin</artifactId> @@ -101,6 +124,19 @@ under the License. <redirectTestOutputToFile>true</redirectTestOutputToFile> </configuration> </plugin> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-jar-plugin</artifactId> + <version>3.3.0</version> + <configuration> + <archive> + <manifest> + <addClasspath>true</addClasspath> + <mainClass>org.apache.iceberg.rest.IcebergRestCatalogTest</mainClass> + </manifest> + </archive> + </configuration> + </plugin> </plugins> </build> diff --git a/java/iceberg-rest-catalog-test/src/main/java/org/apache/iceberg/rest/IcebergRestCatalogTest.java b/java/iceberg-rest-catalog-test/src/main/java/org/apache/iceberg/rest/IcebergRestCatalogTest.java index bce4dd553..5ff996f1a 100644 --- a/java/iceberg-rest-catalog-test/src/main/java/org/apache/iceberg/rest/IcebergRestCatalogTest.java +++ b/java/iceberg-rest-catalog-test/src/main/java/org/apache/iceberg/rest/IcebergRestCatalogTest.java @@ -28,7 +28,7 @@ import java.util.Map; import java.util.function.Consumer; import com.fasterxml.jackson.databind.ObjectMapper; -import org.apache.hadoop.hdfs.HdfsConfiguration; +import org.apache.hadoop.conf.Configuration; import org.apache.iceberg.hadoop.HadoopCatalog; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.rest.responses.ErrorResponse; @@ -60,7 +60,10 @@ public class IcebergRestCatalogTest { } private Catalog initializeBackendCatalog() throws IOException { - HdfsConfiguration conf = new HdfsConfiguration(); + Configuration conf = new Configuration(); + conf.set("io-impl", "org.apache.iceberg.hadoop.HadoopFileIO"); + LOG.info("Default filesystem configured for this Iceberg REST Catalog is " + + conf.get("fs.defaultFS")); return new HadoopCatalog(conf, getWarehouseLocation()); } diff --git a/testdata/bin/run-iceberg-rest-server.sh b/testdata/bin/run-iceberg-rest-server.sh index 3dae9d5ba..49347df96 100755 --- a/testdata/bin/run-iceberg-rest-server.sh +++ b/testdata/bin/run-iceberg-rest-server.sh @@ -17,12 +17,19 @@ # specific language governing permissions and limitations # under the License. -# We can expect that mvn can be found in /usr/local/bin -# set bin/bootstrap_build.sh and bin/bootstrap_system.sh -PATH=${PATH}:/usr/local/bin - cd $IMPALA_HOME . bin/impala-config.sh -bin/mvn-quiet.sh -f "java/iceberg-rest-catalog-test/pom.xml" exec:java \ - -Dexec.mainClass="org.apache.iceberg.rest.IcebergRestCatalogTest" +CP_FILE="$IMPALA_HOME/java/iceberg-rest-catalog-test/target/build-classpath.txt" + +if [ ! -s "$CP_FILE" ]; then + >&2 echo Iceberg REST Catalog classpath file $CP_FILE missing. + >&2 echo Build java/iceberg-rest-catalog-test first. + return 1 +fi + +CLASSPATH=$(cat $CP_FILE):"$CLASSPATH" + +java -cp java/iceberg-rest-catalog-test/target/impala-iceberg-rest-catalog-test-${IMPALA_VERSION}.jar:$CLASSPATH \ + org.apache.iceberg.rest.IcebergRestCatalogTest +
