You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by pv...@apache.org on 2020/05/27 18:41:22 UTC
[hive] branch master updated: HIVE-23492: Remove unnecessary
FileSystem#exists calls from ql module (Karen Coppage via Peter Vary)
This is an automated email from the ASF dual-hosted git repository.
pvary pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push:
new 56a4019 HIVE-23492: Remove unnecessary FileSystem#exists calls from ql module (Karen Coppage via Peter Vary)
56a4019 is described below
commit 56a4019d30e92cbb3f1c384fa8e0f7703a75c097
Author: Karen Coppage <ka...@cloudera.com>
AuthorDate: Wed May 27 20:40:42 2020 +0200
HIVE-23492: Remove unnecessary FileSystem#exists calls from ql module (Karen Coppage via Peter Vary)
---
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java | 8 ++++++--
.../java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java | 9 +++++++--
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java | 8 +++-----
.../java/org/apache/hadoop/hive/ql/io/orc/FixAcidKeyIndex.java | 5 ++++-
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java | 6 ++++--
.../java/org/apache/hadoop/hive/ql/parse/repl/CopyUtils.java | 7 +++++--
.../org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java | 5 ++++-
ql/src/test/org/apache/hadoop/hive/ql/io/TestAcidUtils.java | 4 ++--
.../apache/hadoop/hive/ql/io/orc/TestInputOutputFormat.java | 10 +++++-----
9 files changed, 40 insertions(+), 22 deletions(-)
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
index 811fcc0..ac66925 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
@@ -324,11 +324,15 @@ public final class Utilities {
try {
if (!HiveConf.getBoolVar(conf, ConfVars.HIVE_RPC_QUERY_PLAN)) {
FileSystem fs = mapPath.getFileSystem(conf);
- if (fs.exists(mapPath)) {
+ try {
fs.delete(mapPath, true);
+ } catch (FileNotFoundException e) {
+ // delete if exists, don't panic if it doesn't
}
- if (fs.exists(reducePath)) {
+ try {
fs.delete(reducePath, true);
+ } catch (FileNotFoundException e) {
+ // delete if exists, don't panic if it doesn't
}
}
} catch (Exception e) {
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java b/ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java
index a7fd0ef..516b6f4 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java
@@ -81,6 +81,7 @@ import org.slf4j.LoggerFactory;
import javax.security.auth.login.LoginException;
import java.io.BufferedReader;
import java.io.File;
+import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.Serializable;
@@ -525,8 +526,10 @@ public class ReplDumpTask extends Task<ReplDumpWork> implements Serializable {
Path bootstrapRoot = new Path(dumpRoot, ReplUtils.INC_BOOTSTRAP_ROOT_DIR_NAME);
Path metadataPath = new Path(bootstrapRoot, EximUtil.METADATA_PATH_NAME);
FileSystem fs = FileSystem.get(metadataPath.toUri(), conf);
- if (fs.exists(metadataPath)) {
+ try {
fs.delete(metadataPath, true);
+ } catch (FileNotFoundException e) {
+ // no worries
}
Path dbRootMetadata = new Path(metadataPath, dbName);
Path dbRootData = new Path(bootstrapRoot, EximUtil.DATA_PATH_NAME + File.separator + dbName);
@@ -588,8 +591,10 @@ public class ReplDumpTask extends Task<ReplDumpWork> implements Serializable {
@Override
public Void execute() throws IOException {
FileSystem fs = FileSystem.get(nextEventRoot.toUri(), conf);
- if (fs.exists(nextEventRoot)) {
+ try {
fs.delete(nextEventRoot, true);
+ } catch (FileNotFoundException e) {
+ // no worries
}
return null;
}
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
index bf332bc..5f6f401 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
@@ -2481,9 +2481,6 @@ public class AcidUtils {
if (dirSnapshot != null && !dirSnapshot.contains(formatFile)) {
return false;
}
- if(dirSnapshot == null && !fs.exists(formatFile)) {
- return false;
- }
try (FSDataInputStream strm = fs.open(formatFile)) {
Map<String, String> metaData = new ObjectMapper().readValue(strm, Map.class);
if(!CURRENT_VERSION.equalsIgnoreCase(metaData.get(Field.VERSION))) {
@@ -2498,8 +2495,9 @@ public class AcidUtils {
throw new IllegalArgumentException("Unexpected value for " + Field.DATA_FORMAT
+ ": " + dataFormat);
}
- }
- catch(IOException e) {
+ } catch (FileNotFoundException e) {
+ return false;
+ } catch(IOException e) {
String msg = "Failed to read " + baseOrDeltaDir + "/" + METADATA_FILE
+ ": " + e.getMessage();
LOG.error(msg, e);
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/orc/FixAcidKeyIndex.java b/ql/src/java/org/apache/hadoop/hive/ql/io/orc/FixAcidKeyIndex.java
index 8a7437e..5f6e832 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/io/orc/FixAcidKeyIndex.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/io/orc/FixAcidKeyIndex.java
@@ -17,6 +17,7 @@
*/
package org.apache.hadoop.hive.ql.io.orc;
+import java.io.FileNotFoundException;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.charset.CharacterCodingException;
@@ -186,8 +187,10 @@ public class FixAcidKeyIndex {
Path recoveredPath = getRecoveryFile(inputPath);
// make sure that file does not exist
- if (fs.exists(recoveredPath)) {
+ try {
fs.delete(recoveredPath, false);
+ } catch (FileNotFoundException e) {
+ // no problem, we're just making sure the file doesn't exist
}
// Writer should match the orc configuration from the original file
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java b/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
index 133c97e..a2dbeeb 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
@@ -4384,10 +4384,12 @@ private void constructOneLBLocationMap(FileStatus fSta,
private static void deleteAndRename(FileSystem destFs, Path destFile, FileStatus srcStatus, Path destPath)
throws IOException {
- if (destFs.exists(destFile)) {
+ try {
// rename cannot overwrite non empty destination directory, so deleting the destination before renaming.
destFs.delete(destFile);
- LOG.info("Deleting destination file" + destFile.toUri());
+ LOG.info("Deleted destination file" + destFile.toUri());
+ } catch (FileNotFoundException e) {
+ // no worries
}
if(!destFs.rename(srcStatus.getPath(), destFile)) {
throw new IOException("rename for src path: " + srcStatus.getPath() + " to dest:"
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/parse/repl/CopyUtils.java b/ql/src/java/org/apache/hadoop/hive/ql/parse/repl/CopyUtils.java
index 437072c..3fb271d 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/parse/repl/CopyUtils.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/parse/repl/CopyUtils.java
@@ -254,10 +254,13 @@ public class CopyUtils {
// if the new file exist then delete it before renaming, to avoid rename failure. If the copy is done
// directly to table path (bypassing staging directory) then there might be some stale files from previous
// incomplete/failed load. No need of recycle as this is a case of stale file.
- if (dstFs.exists(newDestFile)) {
- LOG.debug(" file " + newDestFile + " is deleted before renaming");
+ try {
dstFs.delete(newDestFile, true);
+ LOG.debug(" file " + newDestFile + " is deleted before renaming");
+ } catch (FileNotFoundException e) {
+ // no problem
}
+
boolean result = dstFs.rename(destFile, newDestFile);
if (!result) {
throw new IllegalStateException(
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 0425142..4e5d5b0 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
@@ -20,6 +20,7 @@ package org.apache.hadoop.hive.ql.txn.compactor;
import java.io.DataInput;
import java.io.DataOutput;
+import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
@@ -884,8 +885,10 @@ public class CompactorMR {
Path tmpLocation = AcidUtils.createFilename(rootDir, options);
FileSystem fs = tmpLocation.getFileSystem(jobConf);
- if (fs.exists(tmpLocation)) {
+ try {
fs.delete(tmpLocation, true);
+ } catch (FileNotFoundException e) {
+ // no problem
}
}
diff --git a/ql/src/test/org/apache/hadoop/hive/ql/io/TestAcidUtils.java b/ql/src/test/org/apache/hadoop/hive/ql/io/TestAcidUtils.java
index 9e6d47e..f351f04 100644
--- a/ql/src/test/org/apache/hadoop/hive/ql/io/TestAcidUtils.java
+++ b/ql/src/test/org/apache/hadoop/hive/ql/io/TestAcidUtils.java
@@ -280,7 +280,7 @@ public class TestAcidUtils {
assertEquals(0, dir.getObsolete().size());
assertEquals(0, dir.getOriginalFiles().size());
assertEquals(0, dir.getCurrentDirectories().size());
- assertEquals(0, fs.getNumExistsCalls());
+ assertEquals(0, fs.getNumOpenFileCalls());
}
@Test
@@ -297,7 +297,7 @@ public class TestAcidUtils {
assertEquals(0, dir.getObsolete().size());
assertEquals(0, dir.getOriginalFiles().size());
assertEquals(0, dir.getCurrentDirectories().size());
- assertEquals(2, fs.getNumExistsCalls());
+ assertEquals(2, fs.getNumOpenFileCalls());
}
@Test
diff --git a/ql/src/test/org/apache/hadoop/hive/ql/io/orc/TestInputOutputFormat.java b/ql/src/test/org/apache/hadoop/hive/ql/io/orc/TestInputOutputFormat.java
index 12a15a1..e4440e9 100644
--- a/ql/src/test/org/apache/hadoop/hive/ql/io/orc/TestInputOutputFormat.java
+++ b/ql/src/test/org/apache/hadoop/hive/ql/io/orc/TestInputOutputFormat.java
@@ -1217,7 +1217,7 @@ public class TestInputOutputFormat {
private static String blockedUgi = null;
private final static List<MockFile> globalFiles = new ArrayList<MockFile>();
protected Statistics statistics;
- private int numExistsCalls;
+ private int numOpenFileCalls;
public MockFileSystem() {
// empty
@@ -1225,7 +1225,6 @@ public class TestInputOutputFormat {
@Override
public boolean exists(Path f) throws IOException {
- numExistsCalls++;
return super.exists(f);
}
@@ -1282,12 +1281,13 @@ public class TestInputOutputFormat {
@Override
public FSDataInputStream open(Path path, int i) throws IOException {
+ numOpenFileCalls++;
statistics.incrementReadOps(1);
System.out.println("STATS: open - " + path);
checkAccess();
MockFile file = findFile(path);
if (file != null) return new FSDataInputStream(new MockInputStream(file));
- throw new IOException("File not found: " + path);
+ throw new FileNotFoundException("File not found: " + path);
}
private MockFile findFile(Path path) {
@@ -1585,8 +1585,8 @@ public class TestInputOutputFormat {
return buffer.toString();
}
- public int getNumExistsCalls() {
- return numExistsCalls;
+ public int getNumOpenFileCalls() {
+ return numOpenFileCalls;
}
public static void addGlobalFile(MockFile mockFile) {