[impala] branch master updated: IMPALA-11699: Fix NPE in FE tests thrown from static code of FileSystemUtil

[st...@apache.org]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 645df57af IMPALA-11699: Fix NPE in FE tests thrown from static code of FileSystemUtil
645df57af is described below

commit 645df57af8db40bff19407b7006b469bc3298737
Author: stiga-huang <hu...@gmail.com>
AuthorDate: Thu Nov 3 11:19:51 2022 +0800

    IMPALA-11699: Fix NPE in FE tests thrown from static code of FileSystemUtil
    
    FileSystemUtil has static code to get configuration from
    BackendConfig.INSTANCE which could be null in some FE tests. In commit
    c1610a163 of IMPALA-11469, we fixed the issue by modifying the failed FE
    tests to extend FrontendTestBase which can make sure
    BackendConfig.INSTANCE is initialized. However, AcidUtilsTest and
    TestCaseLoaderTest also have the issue. But they are missed since the
    issue depends on the test order. If a test that inits
    BackendConfig.INSTANCE runs first, the following tests won't suffer this
    issue.
    
    To avoid new tests hitting this issue again, this patch inits
    BackendConfig.INSTANCE lazily in the static code of FileSystemUtil.
    Also adds a warning mentioning this should only happen in tests. Note
    that in impalad and catalogd, BackendConfig.INSTANCE is initialized in
    constructors of JniFrontend and JniCatalog.
    
    The changes of c1610a163 is redundant after this patch so they are
    reverted.
    
    Tests:
     - Run FE tests one by one so each test won't depend on any previous
       env. Only found AcidUtilsTest and TestCaseLoaderTest have the issue.
       Verified this patch fixes the issue in these two tests.
    
    Change-Id: I5c056791406cd4535a7e43889dbb73d153b06f0a
    Reviewed-on: http://gerrit.cloudera.org:8080/19195
    Reviewed-by: Daniel Becker <da...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
    Reviewed-by: Michael Smith <mi...@cloudera.com>
    Reviewed-by: Joe McDonnell <jo...@cloudera.com>
---
 fe/src/main/java/org/apache/impala/common/FileSystemUtil.java       | 6 ++++++
 .../test/java/org/apache/impala/catalog/FileMetadataLoaderTest.java | 3 +--
 fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java   | 3 +--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java b/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
index 899cc71c0..ec178b643 100644
--- a/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
+++ b/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
@@ -37,6 +37,7 @@ import org.apache.hadoop.hdfs.client.HdfsAdmin;
 import org.apache.hadoop.hdfs.protocol.EncryptionZone;
 import org.apache.impala.catalog.HdfsCompression;
 import org.apache.impala.service.BackendConfig;
+import org.apache.impala.thrift.TBackendGflags;
 import org.apache.impala.util.DebugUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -865,6 +866,11 @@ public class FileSystemUtil {
    */
   private static final List<String> TMP_DIR_PREFIX_LIST = new ArrayList<>();
   static {
+    if (BackendConfig.INSTANCE == null) {
+      BackendConfig.create(new TBackendGflags());
+      LOG.warn("Initialized BackendConfig.INSTANCE lazily. This should only happen in" +
+          " tests.");
+    }
     String s = BackendConfig.INSTANCE.getIgnoredDirPrefixList();
     for (String prefix : s.split(",")) {
       if (!prefix.isEmpty()) {
diff --git a/fe/src/test/java/org/apache/impala/catalog/FileMetadataLoaderTest.java b/fe/src/test/java/org/apache/impala/catalog/FileMetadataLoaderTest.java
index b0ad5f768..4f278850c 100644
--- a/fe/src/test/java/org/apache/impala/catalog/FileMetadataLoaderTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/FileMetadataLoaderTest.java
@@ -32,7 +32,6 @@ import org.apache.hadoop.hive.common.ValidWriteIdList;
 import org.apache.hadoop.hive.metastore.api.MetaException;
 import org.apache.impala.catalog.HdfsPartition.FileDescriptor;
 import org.apache.impala.compat.MetastoreShim;
-import org.apache.impala.common.FrontendTestBase;
 import org.apache.impala.thrift.TNetworkAddress;
 import org.apache.impala.util.ListMap;
 import org.junit.Test;
@@ -40,7 +39,7 @@ import org.junit.Test;
 import com.google.common.collect.Collections2;
 import com.google.common.collect.ImmutableList;
 
-public class FileMetadataLoaderTest extends FrontendTestBase {
+public class FileMetadataLoaderTest {
 
   @Test
   public void testRecursiveLoading() throws IOException, CatalogException {
diff --git a/fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java b/fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
index c7e339ebc..38ab46ef5 100644
--- a/fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
+++ b/fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
@@ -25,7 +25,6 @@ import static org.junit.Assert.assertEquals;
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
-import org.apache.impala.common.FrontendTestBase;
 import org.apache.impala.service.BackendConfig;
 import org.apache.impala.thrift.TBackendGflags;
 import org.junit.Test;
@@ -39,7 +38,7 @@ import java.util.List;
 /**
  * Tests for the various util methods in FileSystemUtil class
  */
-public class FileSystemUtilTest extends FrontendTestBase {
+public class FileSystemUtilTest {
   private static final String HIVE_TEMP_FILE_PREFIX = "_tmp.";
   private static final String SPARK_TEMP_FILE_PREFIX = "_spark_metadata";
   private static final Path TEST_TABLE_PATH = new Path("/test-warehouse/foo"