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 <[email protected]>
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 <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>
Reviewed-by: Michael Smith <[email protected]>
Reviewed-by: Joe McDonnell <[email protected]>
---
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"
