You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by na...@apache.org on 2013/01/07 07:40:41 UTC
svn commit: r1429686 - in /hive/trunk: build-common.xml
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
ql/src/test/queries/clientpositive/load_fs2.q
ql/src/test/results/clientpositive/load_fs2.q.out
Author: namit
Date: Mon Jan 7 06:40:41 2013
New Revision: 1429686
URL: http://svn.apache.org/viewvc?rev=1429686&view=rev
Log:
HIVE-3300 LOAD DATA INPATH fails if a hdfs file with same name is added to table
(Navis via namit)
Added:
hive/trunk/ql/src/test/queries/clientpositive/load_fs2.q
hive/trunk/ql/src/test/results/clientpositive/load_fs2.q.out
Modified:
hive/trunk/build-common.xml
hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
Modified: hive/trunk/build-common.xml
URL: http://svn.apache.org/viewvc/hive/trunk/build-common.xml?rev=1429686&r1=1429685&r2=1429686&view=diff
==============================================================================
--- hive/trunk/build-common.xml (original)
+++ hive/trunk/build-common.xml Mon Jan 7 06:40:41 2013
@@ -57,7 +57,7 @@
<property name="test.output" value="true"/>
<property name="test.junit.output.format" value="xml"/>
<property name="test.junit.output.usefile" value="true"/>
- <property name="minimr.query.files" value="list_bucket_dml_10.q,input16_cc.q,scriptfile1.q,bucket4.q,bucketmapjoin6.q,disable_merge_for_bucketing.q,reduce_deduplicate.q,smb_mapjoin_8.q,join1.q,groupby2.q,bucketizedhiveinputformat.q,bucketmapjoin7.q,optrstat_groupby.q,bucket_num_reducers.q,bucket5.q"/>
+ <property name="minimr.query.files" value="list_bucket_dml_10.q,input16_cc.q,scriptfile1.q,bucket4.q,bucketmapjoin6.q,disable_merge_for_bucketing.q,reduce_deduplicate.q,smb_mapjoin_8.q,join1.q,groupby2.q,bucketizedhiveinputformat.q,bucketmapjoin7.q,optrstat_groupby.q,bucket_num_reducers.q,bucket5.q,load_fs2.q"/>
<property name="minimr.query.negative.files" value="cluster_tasklog_retrieval.q,minimr_broken_pipe.q,mapreduce_stack_trace.q,mapreduce_stack_trace_turnoff.q,mapreduce_stack_trace_hadoop20.q,mapreduce_stack_trace_turnoff_hadoop20.q" />
<property name="test.silent" value="true"/>
<property name="hadoopVersion" value="${hadoop.version.ant-internal}"/>
Modified: hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
URL: http://svn.apache.org/viewvc/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java?rev=1429686&r1=1429685&r2=1429686&view=diff
==============================================================================
--- hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java (original)
+++ hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java Mon Jan 7 06:40:41 2013
@@ -31,6 +31,7 @@ import static org.apache.hadoop.hive.ser
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
@@ -103,16 +104,16 @@ public class Hive {
private IMetaStoreClient metaStoreClient;
private String currentDatabase;
- private static ThreadLocal<Hive> hiveDB = new ThreadLocal() {
+ private static ThreadLocal<Hive> hiveDB = new ThreadLocal<Hive>() {
@Override
- protected synchronized Object initialValue() {
+ protected synchronized Hive initialValue() {
return null;
}
@Override
public synchronized void remove() {
if (this.get() != null) {
- ((Hive) this.get()).close();
+ this.get().close();
}
super.remove();
}
@@ -1998,28 +1999,59 @@ private void constructOneLBLocationMap(F
}
}
- static private void checkPaths(HiveConf conf,
- FileSystem fs, FileStatus[] srcs, Path destf,
- boolean replace) throws HiveException {
- try {
+ // for each file or directory in 'srcs', make mapping for every file in src to safe name in dest
+ private static List<List<Path[]>> checkPaths(HiveConf conf,
+ FileSystem fs, FileStatus[] srcs, Path destf,
+ boolean replace) throws HiveException {
+
+ List<List<Path[]>> result = new ArrayList<List<Path[]>>();
+ try {
+ FileStatus destStatus = !replace && fs.exists(destf) ? fs.getFileStatus(destf) : null;
+ if (destStatus != null && !destStatus.isDir()) {
+ throw new HiveException("checkPaths: destination " + destf
+ + " should be a directory");
+ }
for (FileStatus src : srcs) {
- FileStatus[] items = fs.listStatus(src.getPath());
+ FileStatus[] items;
+ if (src.isDir()) {
+ items = fs.listStatus(src.getPath());
+ Arrays.sort(items);
+ } else {
+ items = new FileStatus[] {src};
+ }
+
+ List<Path[]> srcToDest = new ArrayList<Path[]>();
for (FileStatus item : items) {
- Path itemStaging = item.getPath();
+
+ Path itemSource = item.getPath();
if (Utilities.isTempPath(item)) {
// This check is redundant because temp files are removed by
// execution layer before
// calling loadTable/Partition. But leaving it in just in case.
- fs.delete(itemStaging, true);
+ fs.delete(itemSource, true);
continue;
}
if (!conf.getBoolVar(HiveConf.ConfVars.HIVE_HADOOP_SUPPORTS_SUBDIRECTORIES) &&
item.isDir()) {
throw new HiveException("checkPaths: " + src.getPath()
- + " has nested directory" + itemStaging);
+ + " has nested directory" + itemSource);
}
+ // Strip off the file type, if any so we don't make:
+ // 000000_0.gz -> 000000_0.gz_copy_1
+ String name = itemSource.getName();
+ String filetype;
+ int index = name.lastIndexOf('.');
+ if (index >= 0) {
+ filetype = name.substring(index);
+ name = name.substring(0, index);
+ } else {
+ filetype = "";
+ }
+
+ Path itemDest = new Path(destf, itemSource.getName());
+
if (!replace) {
// It's possible that the file we're copying may have the same
// relative name as an existing file in the "destf" directory.
@@ -2029,51 +2061,30 @@ private void constructOneLBLocationMap(F
// on "_copy_N" where N starts at 1 and works its way up until
// we find a free space.
- // Note: there are race conditions here, but I don't believe
- // they're worse than what was already present.
- int counter = 1;
-
- // Strip off the file type, if any so we don't make:
- // 000000_0.gz -> 000000_0.gz_copy_1
- String name = itemStaging.getName();
- String filetype;
- int index = name.lastIndexOf('.');
- if (index >= 0) {
- filetype = name.substring(index);
- name = name.substring(0, index);
- } else {
- filetype = "";
- }
-
- Path itemDest = new Path(destf, itemStaging.getName());
- Path itemStagingBase = new Path(itemStaging.getParent(), name);
-
- while (fs.exists(itemDest)) {
- Path proposedStaging = itemStagingBase
- .suffix("_copy_" + counter++).suffix(filetype);
- Path proposedDest = new Path(destf, proposedStaging.getName());
-
- if (fs.exists(proposedDest)) {
- // There's already a file in our destination directory with our
- // _copy_N suffix. We've been here before...
- LOG.trace(proposedDest + " already exists");
- continue;
- }
-
- if (!fs.rename(itemStaging, proposedStaging)) {
- LOG.debug("Unsuccessfully in attempt to rename " + itemStaging + " to " + proposedStaging + "...");
- continue;
- }
-
- LOG.debug("Successfully renamed " + itemStaging + " to " + proposedStaging);
- itemDest = proposedDest;
+ // removed source file staging.. it's more confusing when faild.
+ for (int counter = 1; fs.exists(itemDest) || destExists(result, itemDest); counter++) {
+ itemDest = new Path(destf, name + ("_copy_" + counter) + filetype);
}
}
+ srcToDest.add(new Path[]{itemSource, itemDest});
}
+ result.add(srcToDest);
}
} catch (IOException e) {
throw new HiveException("checkPaths: filesystem error in check phase", e);
}
+ return result;
+ }
+
+ private static boolean destExists(List<List<Path[]>> result, Path proposed) {
+ for (List<Path[]> sdpairs : result) {
+ for (Path[] sdpair : sdpairs) {
+ if (sdpair[1].equals(proposed)) {
+ return true;
+ }
+ }
+ }
+ return false;
}
static protected void copyFiles(HiveConf conf, Path srcf, Path destf, FileSystem fs)
@@ -2102,17 +2113,14 @@ private void constructOneLBLocationMap(F
// srcs = new FileStatus[0]; Why is this needed?
}
// check that source and target paths exist
- checkPaths(conf, fs, srcs, destf, false);
+ List<List<Path[]>> result = checkPaths(conf, fs, srcs, destf, false);
// move it, move it
try {
- for (FileStatus src : srcs) {
- FileStatus[] items = fs.listStatus(src.getPath());
- for (FileStatus item : items) {
- Path source = item.getPath();
- Path target = new Path(destf, item.getPath().getName());
- if (!fs.rename(source, target)) {
- throw new IOException("Cannot move " + source + " to " + target);
+ for (List<Path[]> sdpairs : result) {
+ for (Path[] sdpair : sdpairs) {
+ if (!fs.rename(sdpair[0], sdpair[1])) {
+ throw new IOException("Cannot move " + sdpair[0] + " to " + sdpair[1]);
}
}
}
@@ -2152,7 +2160,7 @@ private void constructOneLBLocationMap(F
LOG.info("No sources specified to move: " + srcf);
return;
}
- checkPaths(conf, fs, srcs, destf, true);
+ List<List<Path[]>> result = checkPaths(conf, fs, srcs, destf, true);
// point of no return -- delete oldPath
if (oldPath != null) {
@@ -2191,11 +2199,11 @@ private void constructOneLBLocationMap(F
fs.mkdirs(destf);
}
// srcs must be a list of files -- ensured by LoadSemanticAnalyzer
- for (FileStatus src : srcs) {
- Path destPath = new Path(destf, src.getPath().getName());
- if (!fs.rename(src.getPath(), destPath)) {
- throw new HiveException("Error moving: " + src.getPath()
- + " into: " + destf);
+ for (List<Path[]> sdpairs : result) {
+ for (Path[] sdpair : sdpairs) {
+ if (!fs.rename(sdpair[0], sdpair[1])) {
+ throw new IOException("Error moving: " + sdpair[0] + " into: " + sdpair[1]);
+ }
}
}
}
Added: hive/trunk/ql/src/test/queries/clientpositive/load_fs2.q
URL: http://svn.apache.org/viewvc/hive/trunk/ql/src/test/queries/clientpositive/load_fs2.q?rev=1429686&view=auto
==============================================================================
--- hive/trunk/ql/src/test/queries/clientpositive/load_fs2.q (added)
+++ hive/trunk/ql/src/test/queries/clientpositive/load_fs2.q Mon Jan 7 06:40:41 2013
@@ -0,0 +1,20 @@
+-- HIVE-3300 [jira] LOAD DATA INPATH fails if a hdfs file with same name is added to table
+-- 'loader' table is used only for uploading kv1.txt to HDFS (!hdfs -put is not working on minMRDriver)
+
+create table result (key string, value string);
+create table loader (key string, value string);
+
+load data local inpath '../data/files/kv1.txt' into table loader;
+
+load data inpath '/build/ql/test/data/warehouse/loader/kv1.txt' into table result;
+show table extended like result;
+
+load data local inpath '../data/files/kv1.txt' into table loader;
+
+load data inpath '/build/ql/test/data/warehouse/loader/kv1.txt' into table result;
+show table extended like result;
+
+load data local inpath '../data/files/kv1.txt' into table loader;
+
+load data inpath '/build/ql/test/data/warehouse/loader/kv1.txt' into table result;
+show table extended like result;
Added: hive/trunk/ql/src/test/results/clientpositive/load_fs2.q.out
URL: http://svn.apache.org/viewvc/hive/trunk/ql/src/test/results/clientpositive/load_fs2.q.out?rev=1429686&view=auto
==============================================================================
--- hive/trunk/ql/src/test/results/clientpositive/load_fs2.q.out (added)
+++ hive/trunk/ql/src/test/results/clientpositive/load_fs2.q.out Mon Jan 7 06:40:41 2013
@@ -0,0 +1,103 @@
+PREHOOK: query: -- HIVE-3300 [jira] LOAD DATA INPATH fails if a hdfs file with same name is added to table
+-- 'loader' table is used only for uploading kv1.txt to HDFS (!hdfs -put is not working on minMRDriver)
+
+create table result (key string, value string)
+PREHOOK: type: CREATETABLE
+POSTHOOK: query: -- HIVE-3300 [jira] LOAD DATA INPATH fails if a hdfs file with same name is added to table
+-- 'loader' table is used only for uploading kv1.txt to HDFS (!hdfs -put is not working on minMRDriver)
+
+create table result (key string, value string)
+POSTHOOK: type: CREATETABLE
+POSTHOOK: Output: default@result
+PREHOOK: query: create table loader (key string, value string)
+PREHOOK: type: CREATETABLE
+POSTHOOK: query: create table loader (key string, value string)
+POSTHOOK: type: CREATETABLE
+POSTHOOK: Output: default@loader
+PREHOOK: query: load data local inpath '../data/files/kv1.txt' into table loader
+PREHOOK: type: LOAD
+PREHOOK: Output: default@loader
+POSTHOOK: query: load data local inpath '../data/files/kv1.txt' into table loader
+POSTHOOK: type: LOAD
+POSTHOOK: Output: default@loader
+PREHOOK: query: load data inpath '/build/ql/test/data/warehouse/loader/kv1.txt' into table result
+PREHOOK: type: LOAD
+PREHOOK: Output: default@result
+POSTHOOK: query: load data inpath '/build/ql/test/data/warehouse/loader/kv1.txt' into table result
+POSTHOOK: type: LOAD
+POSTHOOK: Output: default@result
+PREHOOK: query: show table extended like result
+PREHOOK: type: SHOW_TABLESTATUS
+POSTHOOK: query: show table extended like result
+POSTHOOK: type: SHOW_TABLESTATUS
+tableName:result
+#### A masked pattern was here ####
+inputformat:org.apache.hadoop.mapred.TextInputFormat
+outputformat:org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat
+columns:struct columns { string key, string value}
+partitioned:false
+partitionColumns:
+totalNumberFiles:1
+totalFileSize:5812
+maxFileSize:5812
+minFileSize:5812
+#### A masked pattern was here ####
+
+PREHOOK: query: load data local inpath '../data/files/kv1.txt' into table loader
+PREHOOK: type: LOAD
+PREHOOK: Output: default@loader
+POSTHOOK: query: load data local inpath '../data/files/kv1.txt' into table loader
+POSTHOOK: type: LOAD
+POSTHOOK: Output: default@loader
+PREHOOK: query: load data inpath '/build/ql/test/data/warehouse/loader/kv1.txt' into table result
+PREHOOK: type: LOAD
+PREHOOK: Output: default@result
+POSTHOOK: query: load data inpath '/build/ql/test/data/warehouse/loader/kv1.txt' into table result
+POSTHOOK: type: LOAD
+POSTHOOK: Output: default@result
+PREHOOK: query: show table extended like result
+PREHOOK: type: SHOW_TABLESTATUS
+POSTHOOK: query: show table extended like result
+POSTHOOK: type: SHOW_TABLESTATUS
+tableName:result
+#### A masked pattern was here ####
+inputformat:org.apache.hadoop.mapred.TextInputFormat
+outputformat:org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat
+columns:struct columns { string key, string value}
+partitioned:false
+partitionColumns:
+totalNumberFiles:2
+totalFileSize:11624
+maxFileSize:5812
+minFileSize:5812
+#### A masked pattern was here ####
+
+PREHOOK: query: load data local inpath '../data/files/kv1.txt' into table loader
+PREHOOK: type: LOAD
+PREHOOK: Output: default@loader
+POSTHOOK: query: load data local inpath '../data/files/kv1.txt' into table loader
+POSTHOOK: type: LOAD
+POSTHOOK: Output: default@loader
+PREHOOK: query: load data inpath '/build/ql/test/data/warehouse/loader/kv1.txt' into table result
+PREHOOK: type: LOAD
+PREHOOK: Output: default@result
+POSTHOOK: query: load data inpath '/build/ql/test/data/warehouse/loader/kv1.txt' into table result
+POSTHOOK: type: LOAD
+POSTHOOK: Output: default@result
+PREHOOK: query: show table extended like result
+PREHOOK: type: SHOW_TABLESTATUS
+POSTHOOK: query: show table extended like result
+POSTHOOK: type: SHOW_TABLESTATUS
+tableName:result
+#### A masked pattern was here ####
+inputformat:org.apache.hadoop.mapred.TextInputFormat
+outputformat:org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat
+columns:struct columns { string key, string value}
+partitioned:false
+partitionColumns:
+totalNumberFiles:3
+totalFileSize:17436
+maxFileSize:5812
+minFileSize:5812
+#### A masked pattern was here ####
+