[hive] branch branch-3.1 updated: HIVE-24653: Race condition between compactor marker generation and get splits. (#1882) (Antal Sinkovits, reviewed by Laszlo Pinter)

[lp...@apache.org]

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

lpinter pushed a commit to branch branch-3.1
in repository https://gitbox.apache.org/repos/asf/hive.git


The following commit(s) were added to refs/heads/branch-3.1 by this push:
     new 97583d4  HIVE-24653: Race condition between compactor marker generation and get splits. (#1882) (Antal Sinkovits, reviewed by Laszlo Pinter)
97583d4 is described below

commit 97583d4119604550717f1ff7845bfdb074d64b3e
Author: Antal Sinkovits <as...@cloudera.com>
AuthorDate: Mon Feb 1 11:11:55 2021 +0100

    HIVE-24653: Race condition between compactor marker generation and get splits. (#1882) (Antal Sinkovits, reviewed by Laszlo Pinter)
---
 .../hadoop/hive/ql/txn/compactor/CompactorMR.java   | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java b/ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java
index 6044719..95870ad 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java
@@ -1181,15 +1181,14 @@ public class CompactorMR {
       //name is that we want to rename; leave it for another day
       // TODO: if we expect one dir why don't we enforce it?
       for (FileStatus fileStatus : contents) {
+        Path tmpPath = fileStatus.getPath();
         //newPath is the base/delta dir
-        Path newPath = new Path(finalLocation, fileStatus.getPath().getName());
-        /*rename(A, B) has "interesting" behavior if A and B are directories. If  B doesn't exist,
-        * it does the expected operation and everything that was in A is now in B.  If B exists,
-        * it will make A a child of B...  thus make sure the rename() is done before creating the
-        * meta files which will create base_x/ (i.e. B)...*/
-        fs.rename(fileStatus.getPath(), newPath);
-        AcidUtils.OrcAcidVersion.writeVersionFile(newPath, fs);
-        createCompactorMarker(conf, newPath, fs);
+        Path newPath = new Path(finalLocation, tmpPath.getName());
+        /* Create the markers in the tmp location and rename everything in the end to prevent race condition between
+         * marker creation and split read. */
+        AcidUtils.OrcAcidVersion.writeVersionFile(tmpPath, fs);
+        createCompactorMarker(conf, tmpPath, fs);
+        fs.rename(tmpPath, newPath);
       }
       fs.delete(tmpLocation, true);
     }
@@ -1235,9 +1234,9 @@ public class CompactorMR {
     if (children.length != 1) {
       throw new IOException("Unexpected files in the source: " + Arrays.toString(children));
     }
-    FileStatus dirPath = children[0];
-    fs.rename(dirPath.getPath(), newBaseDir);
-    AcidUtils.MetaDataFile.createCompactorMarker(newBaseDir, fs);
+    Path tmpPath = children[0].getPath();
+    AcidUtils.MetaDataFile.createCompactorMarker(tmpPath, fs);
+    fs.rename(tmpPath, newBaseDir);
     fs.delete(fromPath, true);
   }
 }