You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by kg...@apache.org on 2017/11/10 10:52:54 UTC
hive git commit: HIVE-17947: Concurrent inserts might fail for ACID
table since HIVE-17526 on branch-1 (Daniel Voros reviewed by Eugene Koifman)
Repository: hive
Updated Branches:
refs/heads/branch-1 cd08cd6d3 -> f99314747
HIVE-17947: Concurrent inserts might fail for ACID table since HIVE-17526 on branch-1 (Daniel Voros reviewed by Eugene Koifman)
Signed-off-by: Zoltan Haindrich <ki...@rxd.hu>
Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/f9931474
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/f9931474
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/f9931474
Branch: refs/heads/branch-1
Commit: f993147470e576b17b17c89bd988ef8feeb276c5
Parents: cd08cd6
Author: Daniel Voros <da...@gmail.com>
Authored: Fri Nov 10 11:47:31 2017 +0100
Committer: Zoltan Haindrich <ki...@rxd.hu>
Committed: Fri Nov 10 11:47:31 2017 +0100
----------------------------------------------------------------------
.../TransactionalValidationListener.java | 75 +++++++++++++-------
1 file changed, 50 insertions(+), 25 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hive/blob/f9931474/metastore/src/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
----------------------------------------------------------------------
diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java b/metastore/src/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
index 78c0eb9..0368ed6 100644
--- a/metastore/src/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
+++ b/metastore/src/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
@@ -18,10 +18,10 @@
package org.apache.hadoop.hive.metastore;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
-import org.apache.hadoop.fs.LocatedFileStatus;
import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.fs.RemoteIterator;
+import org.apache.hadoop.hive.common.FileUtils;
import org.apache.hadoop.hive.metastore.api.*;
import org.apache.hadoop.hive.metastore.events.PreAlterTableEvent;
import org.apache.hadoop.hive.metastore.events.PreCreateTableEvent;
@@ -30,9 +30,11 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
+import java.util.Arrays;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
+import java.util.Stack;
import java.util.regex.Pattern;
final class TransactionalValidationListener extends MetaStorePreEventListener {
@@ -90,7 +92,14 @@ final class TransactionalValidationListener extends MetaStorePreEventListener {
//normalize prop name
parameters.put(hive_metastoreConstants.TABLE_IS_TRANSACTIONAL, transactionalValue);
}
- if ("true".equalsIgnoreCase(transactionalValue)) {
+ Table oldTable = context.getOldTable();
+ String oldTransactionalValue = null;
+ for (String key : oldTable.getParameters().keySet()) {
+ if (hive_metastoreConstants.TABLE_IS_TRANSACTIONAL.equalsIgnoreCase(key)) {
+ oldTransactionalValue = oldTable.getParameters().get(key);
+ }
+ }
+ if ("true".equalsIgnoreCase(transactionalValue) && !"true".equalsIgnoreCase(oldTransactionalValue)) {
if (!conformToAcid(newTable)) {
throw new MetaException("The table must be bucketed and stored using an ACID compliant" +
" format (such as ORC)");
@@ -101,20 +110,13 @@ final class TransactionalValidationListener extends MetaStorePreEventListener {
" cannot be declared transactional because it's an external table");
}
- if (containsCopyNFiles(context.getHandler().getMS(), newTable)) {
+ if (containsCopyNFiles(context.getHandler(), newTable)) {
throw new MetaException(newTable.getDbName() + "." + newTable.getTableName() +
" cannot be declared transactional because it has _COPY_N files.");
}
return;
}
- Table oldTable = context.getOldTable();
- String oldTransactionalValue = null;
- for (String key : oldTable.getParameters().keySet()) {
- if (hive_metastoreConstants.TABLE_IS_TRANSACTIONAL.equalsIgnoreCase(key)) {
- oldTransactionalValue = oldTable.getParameters().get(key);
- }
- }
if (oldTransactionalValue == null ? transactionalValue == null
: oldTransactionalValue.equalsIgnoreCase(transactionalValue)) {
//this covers backward compat cases where this prop may have been set already
@@ -209,9 +211,9 @@ final class TransactionalValidationListener extends MetaStorePreEventListener {
* @return True if table contains files named *_copy_N. False otherwise.
* @throws MetaException
*/
- boolean containsCopyNFiles(RawStore ms, Table table) throws MetaException {
- Warehouse wh = new Warehouse(getConf());
-
+ boolean containsCopyNFiles(HiveMetaStore.HMSHandler handler, Table table) throws MetaException {
+ Warehouse wh = handler.getWh();
+ RawStore ms = handler.getMS();
try {
Path tablePath;
if (table.getSd().getLocation() == null
@@ -222,21 +224,44 @@ final class TransactionalValidationListener extends MetaStorePreEventListener {
tablePath = wh.getDnsPath(new Path(table.getSd().getLocation()));
}
FileSystem fs = wh.getFs(tablePath);
- RemoteIterator<LocatedFileStatus> iterator = fs.listFiles(tablePath, true);
- while (iterator.hasNext()) {
- LocatedFileStatus fileStatus = iterator.next();
- if (COPY_N_PATTERN.matcher(fileStatus.getPath().getName()).matches()) {
- return true;
- }
- }
+ return containsCopyNFiles(fs, tablePath);
} catch (IOException e) {
- throw new MetaException("Unable to list files for " + table.getDbName() + "."+
- table.getTableName());
+ String errorMessage = "Unable to list files for " + table.getDbName() + "."+
+ table.getTableName();
+ LOG.error("IOException during listing copyNFiles: ", e);
+ throw new MetaException(errorMessage);
} catch (NoSuchObjectException e) {
- throw new MetaException("Unable to get location for " + table.getDbName() + "."+
- table.getTableName());
+ String errorMessage = "Unable to get location for " + table.getDbName() + "."+
+ table.getTableName();
+ LOG.error(errorMessage, e);
+ throw new MetaException(errorMessage);
+ }
+ }
+
+ /**
+ * Check if there are files named *_copy_N under a path on a given FileSystem.
+ * @param fs
+ * @param rootPath
+ * @return True if there is at least one file named *_copy_N. False otherwise.
+ * @throws IOException On exception during listing the files.
+ */
+ boolean containsCopyNFiles(FileSystem fs, Path rootPath) throws IOException {
+ if (!fs.isDirectory(rootPath)) {
+ return COPY_N_PATTERN.matcher(rootPath.getName()).matches();
}
+ Stack<FileStatus> stack = new Stack<>();
+ stack.push(fs.getFileStatus(rootPath));
+ while (!stack.isEmpty()) {
+ FileStatus dir = stack.pop();
+ for (FileStatus child : fs.listStatus(dir.getPath(), FileUtils.HIDDEN_FILES_PATH_FILTER)) {
+ if (child.isDirectory()) {
+ stack.push(child);
+ } else if (COPY_N_PATTERN.matcher(child.getPath().getName()).matches()) {
+ return true;
+ }
+ }
+ }
return false;
}
}