You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2019/07/26 23:58:58 UTC

[impala] branch master updated (12575f8 -> 20b5b1f)

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

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


    from 12575f8  Add support to tag docker images when pushing them
     new 2551a87  IMPALA-8489: Partitions created by RECOVER PARTITIONS fail to create insert events with IllegalStateException.
     new 20b5b1f  IMPALA-8794: Skipping testPiggyback* in Hive 3 builds

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../java/org/apache/impala/catalog/HdfsTable.java  |  2 +-
 .../apache/impala/service/CatalogOpExecutor.java   | 50 +++++++++++++++-------
 .../catalog/local/CatalogdMetaProviderTest.java    | 14 +++++-
 3 files changed, 49 insertions(+), 17 deletions(-)


[impala] 02/02: IMPALA-8794: Skipping testPiggyback* in Hive 3 builds

Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 20b5b1f13a5cdafd793014c81ae3c3b4e5568d06
Author: Csaba Ringhofer <cs...@cloudera.com>
AuthorDate: Thu Jul 25 21:02:02 2019 +0200

    IMPALA-8794: Skipping testPiggyback* in Hive 3 builds
    
    The tests turned out to be very flaky with Hive 3. Skipping them
    for now to make the builds green.
    
    Change-Id: Ice8f52caa20021ad9e36cbebe62a2bb247a4273c
    Reviewed-on: http://gerrit.cloudera.org:8080/13920
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../impala/catalog/local/CatalogdMetaProviderTest.java     | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java b/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
index 60db091..c4276d2 100644
--- a/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/local/CatalogdMetaProviderTest.java
@@ -39,6 +39,7 @@ import org.apache.impala.catalog.local.MetaProvider.TableMetaRef;
 import org.apache.impala.common.Pair;
 import org.apache.impala.service.FeSupport;
 import org.apache.impala.service.FrontendProfile;
+import org.apache.impala.testutil.TestUtils;
 import org.apache.impala.thrift.TBackendGflags;
 import org.apache.impala.thrift.TCatalogObject;
 import org.apache.impala.thrift.TCatalogObjectType;
@@ -47,6 +48,7 @@ import org.apache.impala.thrift.TNetworkAddress;
 import org.apache.impala.thrift.TRuntimeProfileNode;
 import org.apache.impala.thrift.TTable;
 import org.apache.impala.util.ListMap;
+import org.junit.Assume;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -251,11 +253,21 @@ public class CatalogdMetaProviderTest {
 
   @Test
   public void testPiggybackSuccess() throws Exception {
+    // TODO: investigate the cause of flakiness (IMPALA-8794)
+    Assume.assumeTrue(
+        "Skipping this test because it is flaky with Hive3",
+        TestUtils.getHiveMajorVersion() == 2);
+
     doTestPiggyback(/*success=*/true);
   }
 
   @Test
   public void testPiggybackFailure() throws Exception {
+    // TODO: investigate the cause of flakiness (IMPALA-8794)
+    Assume.assumeTrue(
+        "Skipping this test because it is flaky with Hive3",
+        TestUtils.getHiveMajorVersion() == 2);
+
     doTestPiggyback(/*success=*/false);
   }
 
@@ -319,4 +331,4 @@ public class CatalogdMetaProviderTest {
     }
   }
 
-}
\ No newline at end of file
+}


[impala] 01/02: IMPALA-8489: Partitions created by RECOVER PARTITIONS fail to create insert events with IllegalStateException.

Posted by ta...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 2551a873a0ab5636b3693711de11e2a91474d86a
Author: Anurag Mantripragada <an...@gmail.com>
AuthorDate: Fri Jul 12 12:26:52 2019 -0700

    IMPALA-8489: Partitions created by RECOVER PARTITIONS fail to create
    insert events with IllegalStateException.
    
    createInsertEvents() uses partition ids to keep track of the files
    added to the partitions by the insert by finding the delta of files
    in a partition before and after load() call. However, if partitions
    are marked dirty (for eg.: partitions created by RECOVER PARTITIONS),
    load() will drop and re-create them which will change the partition
    ids. createInsertEvents() then cannot find these parittions and fails
    with exception.
    
    In this patch, partitions are tracked by partition names instead of
    partition ids so drop + reload will not affect the logic.
    
    Testing:
    1. Ran TestRecoverPartitions.test_post_invalidate() which was
       failing.
    2. Ran MetastoreEventProcessorTest FE tests.
    
    Change-Id: Idef7f6aadff2868047c861ebfcc05d65f080eab9
    Reviewed-on: http://gerrit.cloudera.org:8080/13860
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
    Reviewed-by: Vihang Karajgaonkar <vi...@cloudera.com>
---
 .../java/org/apache/impala/catalog/HdfsTable.java  |  2 +-
 .../apache/impala/service/CatalogOpExecutor.java   | 50 +++++++++++++++-------
 2 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
index 0255e21..99eb943 100644
--- a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
@@ -1110,7 +1110,7 @@ public class HdfsTable extends Table implements FeFsTable {
    * Given a set of partition names, returns the corresponding HdfsPartition
    * objects.
    */
-  private List<HdfsPartition> getPartitionsForNames(Collection<String> partitionNames) {
+  public List<HdfsPartition> getPartitionsForNames(Collection<String> partitionNames) {
     List<HdfsPartition> parts = Lists.newArrayListWithCapacity(partitionNames.size());
     for (String partitionName: partitionNames) {
       String partName = DEFAULT_PARTITION_NAME;
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index 0951349..cf0d0fc 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -32,6 +32,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import java.util.stream.Collectors;
 import org.apache.hadoop.fs.FSDataInputStream;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
@@ -3872,22 +3873,28 @@ public class CatalogOpExecutor {
       List<FeFsPartition> affectedExistingPartitions, boolean isInsertOverwrite) {
     if (!catalog_.isExternalEventProcessingEnabled() ||
         affectedExistingPartitions.size() == 0) return;
-    // Map of partition ids to file names of all existing partitions touched by the
+
+    // Map of partition names to file names of all existing partitions touched by the
     // insert.
-    Map<Long, Set<String>> partitionFilesMap = new HashMap<>();
+    Map<String, Set<String>> partitionFilesMapBeforeInsert = new HashMap<>();
     if (!isInsertOverwrite) {
-      for (FeFsPartition partition : affectedExistingPartitions) {
-        partitionFilesMap.put(partition.getId(),
-            ((HdfsPartition) partition).getFileNames());
-      }
+      partitionFilesMapBeforeInsert =
+          getPartitionNameFilesMap(affectedExistingPartitions);
     }
     // If table is partitioned, we add all existing partitions touched by this insert
-    // to the insert event. If it is not an insert overwrite operation, we find new
-    // files added by this insert.
+    // to the insert event.
     Collection<? extends FeFsPartition> partsPostInsert;
-    partsPostInsert = table.getNumClusteringCols() > 0 ?
-        ((FeFsTable)table).loadPartitions(partitionFilesMap.keySet()) :
-            FeCatalogUtils.loadAllPartitions((HdfsTable) table);
+    partsPostInsert =
+        ((HdfsTable) table).getPartitionsForNames(
+            partitionFilesMapBeforeInsert.keySet());
+
+    // If it is not an insert overwrite operation, we find new files added by this insert.
+    Map<String, Set<String>> partitionFilesMapPostInsert = new HashMap<>();
+    if (!isInsertOverwrite) {
+      partitionFilesMapPostInsert =
+          getPartitionNameFilesMap(partsPostInsert);
+    }
+
     for (FeFsPartition part : partsPostInsert) {
       // Find the delta of the files added by the insert if it is not an overwrite
       // operation. HMS fireListenerEvent() expects an empty list if no new files are
@@ -3895,14 +3902,17 @@ public class CatalogOpExecutor {
       Set<String> deltaFiles = new HashSet<>();
       List<String> partVals = null;
       if (!isInsertOverwrite) {
-        Set<String> filesPostInsert = ((HdfsPartition) part).getFileNames();
+        String partitionName = part.getPartitionName() + "/";
+        Set<String> filesPostInsert =
+            partitionFilesMapPostInsert.get(partitionName);
         if (table.getNumClusteringCols() > 0) {
-          Set<String> filesBeforeInsert = partitionFilesMap.get(part.getId());
+          Set<String> filesBeforeInsert =
+              partitionFilesMapBeforeInsert.get(partitionName);
           deltaFiles = Sets.difference(filesBeforeInsert, filesPostInsert);
           partVals = part.getPartitionValuesAsStrings(true);
         } else {
-          Map.Entry<Long, Set<String>> entry =
-              partitionFilesMap.entrySet().iterator().next();
+          Map.Entry<String, Set<String>> entry =
+              partitionFilesMapBeforeInsert.entrySet().iterator().next();
           deltaFiles = Sets.difference(entry.getValue(), filesPostInsert);
         }
         LOG.info("{} new files detected for table {} partition {}.",
@@ -3926,6 +3936,16 @@ public class CatalogOpExecutor {
   }
 
   /**
+   * Util method to return a map of partition names to list of files for that partition.
+   */
+  private static Map<String, Set<String>> getPartitionNameFilesMap(Collection<?
+      extends FeFsPartition> partitions) {
+        return partitions.stream().collect(
+            Collectors.toMap(p -> p.getPartitionName() + "/",
+                p -> ((HdfsPartition) p).getFileNames()));
+  }
+
+  /**
    * Returns an existing, loaded table from the Catalog. Throws an exception if any
    * of the following are true:
    * - The table does not exist