[impala] 01/03: IMPALA-9907: Fix NullPointerException in ParallelFileMetadataLoader's load() method

[as...@apache.org]

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

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

commit 36ced942d84e7237de28a0ebfae785139dbd3836
Author: Zoltan Borok-Nagy <bo...@cloudera.com>
AuthorDate: Mon Jun 29 17:31:34 2020 +0200

    IMPALA-9907: Fix NullPointerException in ParallelFileMetadataLoader's load() method
    
    ParallelFileMetadataLoader.loaders_ became a Map, but at one place we
    still treat it as a List:
    
      loaders_.get(i).getPartDir()
    
    'i' is an integer, while the loaders_'s key type is Path.
    
    Unfortunately the code compiled which means we got a
    NullPointerException for this statement. I changed the logic to avoid
    the NPE.
    
    Testing:
    
      * Tested manually on an ACID table with deleted rows
      * I didn't add an automated test because I'm about to extend the ACID
        functionalities, hence getting rid of these exceptions
      * The other case when we can hit this NPE is when we get an
        IOException in the file metadata loader's load() method
    
    Change-Id: I4322a18a1f07e4106789591b80d25105c565a2b6
    Reviewed-on: http://gerrit.cloudera.org:8080/16121
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../org/apache/impala/catalog/ParallelFileMetadataLoader.java | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java b/fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java
index 5381f2e..fa51cbf 100644
--- a/fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java
+++ b/fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java
@@ -35,6 +35,7 @@ import org.apache.impala.catalog.FeFsTable.Utils;
 import org.apache.impala.catalog.HdfsPartition.FileDescriptor;
 import org.apache.impala.catalog.MetaStoreClientPool.MetaStoreClient;
 import org.apache.impala.common.FileSystemUtil;
+import org.apache.impala.common.Pair;
 import org.apache.impala.compat.MetastoreShim;
 import org.apache.impala.service.BackendConfig;
 import org.apache.impala.thrift.TValidWriteIdList;
@@ -158,19 +159,21 @@ public class ParallelFileMetadataLoader {
     int failedLoadTasks = 0;
     ExecutorService pool = createPool();
     try (ThreadNameAnnotator tna = new ThreadNameAnnotator(logPrefix_)) {
-      List<Future<Void>> futures = new ArrayList<>(loaders_.size());
+      List<Pair<FileMetadataLoader, Future<Void>>> futures =
+          new ArrayList<>(loaders_.size());
       for (FileMetadataLoader loader : loaders_.values()) {
-        futures.add(pool.submit(() -> { loader.load(); return null; }));
+        futures.add(new Pair<FileMetadataLoader, Future<Void>>(
+            loader, pool.submit(() -> { loader.load(); return null; })));
       }
 
       // Wait for the loaders to finish.
       for (int i = 0; i < futures.size(); i++) {
         try {
-          futures.get(i).get();
+          futures.get(i).second.get();
         } catch (ExecutionException | InterruptedException e) {
           if (++failedLoadTasks <= MAX_PATH_METADATA_LOADING_ERRORS_TO_LOG) {
             LOG.error(logPrefix_ + " encountered an error loading data for path " +
-                loaders_.get(i).getPartDir(), e);
+                futures.get(i).first.getPartDir(), e);
           }
         }
       }