[impala] 01/03: IMPALA-10637: Fixes bug in ValidWriteIdList comparison

[jo...@apache.org]

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

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

commit 5d307cbb7b2ec3432bebd7759b0bcebf54a6cc22
Author: Sourabh Goyal <so...@cloudera.com>
AuthorDate: Tue Apr 6 12:31:14 2021 -0700

    IMPALA-10637: Fixes bug in ValidWriteIdList comparison
    
    For a transactional table, catalogd compares previous and current ValidWriteList
    to determine more recent version out of the two and reloads table cache accordingly.
    Because of a bug in ValidWriteIdList comparison, catalogD was not refreshing table
    metadata in the cache with more recent changes. As a result of which we were seeing
    inconsistencies in read after write into the table.
    
    Tested by
      1. Adding a unit test to compare WriteIDLists.
    
    Change-Id: Idaa4bcdbda1757a6451122efc505d1d483c879cc
    Reviewed-on: http://gerrit.cloudera.org:8080/17276
    Reviewed-by: Sourabh Goyal <so...@cloudera.com>
    Reviewed-by: Vihang Karajgaonkar <vi...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 fe/src/main/java/org/apache/impala/util/AcidUtils.java    |  2 +-
 .../test/java/org/apache/impala/util/AcidUtilsTest.java   | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/fe/src/main/java/org/apache/impala/util/AcidUtils.java b/fe/src/main/java/org/apache/impala/util/AcidUtils.java
index b9fef23..cff46ad 100644
--- a/fe/src/main/java/org/apache/impala/util/AcidUtils.java
+++ b/fe/src/main/java/org/apache/impala/util/AcidUtils.java
@@ -749,7 +749,7 @@ public class AcidUtils {
       }
     } else {
       if (b.getHighWatermark() != a.getInvalidWriteIds()[minLen] - 1) {
-        return Long.signum(b.getHighWatermark() - (a.getInvalidWriteIds()[minLen] - 1));
+        return Long.signum((a.getInvalidWriteIds()[minLen] - 1) - b.getHighWatermark());
       }
       if (allInvalidFrom(a.getInvalidWriteIds(), minLen, a.getHighWatermark())) {
         return 0;
diff --git a/fe/src/test/java/org/apache/impala/util/AcidUtilsTest.java b/fe/src/test/java/org/apache/impala/util/AcidUtilsTest.java
index 5330276..6a17b82 100644
--- a/fe/src/test/java/org/apache/impala/util/AcidUtilsTest.java
+++ b/fe/src/test/java/org/apache/impala/util/AcidUtilsTest.java
@@ -31,6 +31,7 @@ import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hive.common.ValidReadTxnList;
 import org.apache.hadoop.hive.common.ValidWriteIdList;
+import org.apache.hadoop.hive.common.ValidReaderWriteIdList;
 import org.apache.hadoop.hive.metastore.api.MetaException;
 import org.apache.impala.catalog.CatalogException;
 import org.apache.impala.compat.MetastoreShim;
@@ -595,4 +596,18 @@ public class AcidUtilsTest {
             "delta_0000012_0000012_0000/0000_0",
             "delta_0000012_0000012_0000/0000_1"});
   }
+
+  @Test
+  public void testWriteIdListCompare() {
+    ValidWriteIdList a =
+            new ValidReaderWriteIdList("default.test:1:1:1:");
+    ValidWriteIdList b =
+            new ValidReaderWriteIdList("default.test:1:9223372036854775807::");
+
+    // should return -1 since b is more recent
+    assert(AcidUtils.compare(a, b) == -1);
+
+    //Should return 1 since b is more recent
+    assert(AcidUtils.compare(b,a) == 1);
+  }
 }