[impala] 02/02: IMPALA-11720: Deflake FileMetadataLoaderTest due to FileSystem closed

[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

commit 86fdbfcc995a22a5836c6625fd5c86080fa7764a
Author: stiga-huang <hu...@gmail.com>
AuthorDate: Fri Nov 11 19:41:55 2022 +0800

    IMPALA-11720: Deflake FileMetadataLoaderTest due to FileSystem closed
    
    IMPALA-11699 recently reverted the changes in FileMetadataLoaderTest
    that it no longer extends to FrontendTestBase. This causes tests in
    FileMetadataLoaderTest to become flaky if run individually.
    
    This is a subtle bug in FileMetadataLoader#load(), where we first call
    FileSystem#getFileSystem() to get a FileSystem object, and then call
    methods of FileSystemUtil which will trigger class loading of it and
    running its static code.
    
    FileSystem#getFileSystem() will create a FileSystem object in the first
    get and cache it for follow-up usage. Without extending
    FrontendTestBase, BackendConfig.INSTANCE is not initialized when the
    test is run. So the static code in FileSystemUtil lazily initializes
    BackendConfig.INSTANCE, which initializes FeSupport and finally calls
    JniUtil::InitLibhdfs(). In this method, a FileSystem object is get and
    closed. This is exactly the FileSystem object created in
    FileMetadataLoader#load(). So the following usage on it causes an
    IOException of "Filesystem closed".
    
    The purpose of JniUtil::InitLibhdfs() is to make a simple call on
    libhdfs to make it initialize the JVM. This is crucial when launching
    from C++ codes for impalad and catalogd to init the embedded JVM. So we
    should keep it unchanged.
    
    The fix for this bug is to maintain the state of BackendConfig.INSTANCE
    in the static code of FileSystemUtil (i.e., keep un-initialized or
    initialized), which avoids the subtle side effects.
    
    Tests:
     - Verified tests in FileMetadataLoaderTest individually
    
    Change-Id: Ib6f96950210c9a0124fe03696ef334cb00b057ab
    Reviewed-on: http://gerrit.cloudera.org:8080/19233
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
    Reviewed-by: Michael Smith <mi...@cloudera.com>
    Reviewed-by: Qifan Chen <qf...@hotmail.com>
---
 .../org/apache/impala/common/FileSystemUtil.java   | 29 ++++++++++++++--------
 .../apache/impala/common/FileSystemUtilTest.java   |  6 ++---
 2 files changed, 21 insertions(+), 14 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 ec178b643..82cf9cfcf 100644
--- a/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
+++ b/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java
@@ -37,7 +37,6 @@ 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;
@@ -860,21 +859,31 @@ public class FileSystemUtil {
     return false;
   }
 
+  public static final String DOT = ".";
+  public static final String HIVE_TEMP_FILE_PREFIX = "_tmp.";
+  public static final String SPARK_TEMP_FILE_PREFIX = "_spark_metadata";
+
   /**
    * Prefix string used by tools like hive/spark/flink to write certain temporary or
    * "non-data" files in the table location
    */
   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()) {
-        TMP_DIR_PREFIX_LIST.add(prefix);
+    // Use hard-coded prefix-list if BackendConfig is uninitialized. Note that
+    // getIgnoredDirPrefixList() could return null if BackendConfig is created with
+    // initialize=false in external FE (IMPALA-10515).
+    if (BackendConfig.INSTANCE == null
+        || BackendConfig.INSTANCE.getIgnoredDirPrefixList() == null) {
+      TMP_DIR_PREFIX_LIST.add(DOT);
+      TMP_DIR_PREFIX_LIST.add(HIVE_TEMP_FILE_PREFIX);
+      TMP_DIR_PREFIX_LIST.add(SPARK_TEMP_FILE_PREFIX);
+      LOG.warn("BackendConfig.INSTANCE uninitialized. Use hard-coded prefix-list.");
+    } else {
+      String s = BackendConfig.INSTANCE.getIgnoredDirPrefixList();
+      for (String prefix : s.split(",")) {
+        if (!prefix.isEmpty()) {
+          TMP_DIR_PREFIX_LIST.add(prefix);
+        }
       }
     }
     LOG.info("Prefix list of ignored dirs: " + TMP_DIR_PREFIX_LIST);
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 38ab46ef5..e3b167ba8 100644
--- a/fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
+++ b/fe/src/test/java/org/apache/impala/common/FileSystemUtilTest.java
@@ -17,6 +17,8 @@
 
 package org.apache.impala.common;
 
+import static org.apache.impala.common.FileSystemUtil.HIVE_TEMP_FILE_PREFIX;
+import static org.apache.impala.common.FileSystemUtil.SPARK_TEMP_FILE_PREFIX;
 import static org.apache.impala.common.FileSystemUtil.isIgnoredDir;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -25,8 +27,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.service.BackendConfig;
-import org.apache.impala.thrift.TBackendGflags;
 import org.junit.Test;
 import org.mockito.Mockito;
 
@@ -39,8 +39,6 @@ import java.util.List;
  * Tests for the various util methods in FileSystemUtil class
  */
 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"
       + ".db/filesystem-util-test");