You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2020/09/28 13:49:12 UTC
[GitHub] [hive] pvargacl opened a new pull request #1532: HIVE-20137: truncate for transactional tables
pvargacl opened a new pull request #1532:
URL: https://github.com/apache/hive/pull/1532
### What changes were proposed in this pull request?
Truncate table for transactional tables no longer deletes all the data, rather just creates an empty base dir.
### Why are the changes needed?
Faster truncate, it can allow concurrent reads
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Tests were added
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvargacl commented on pull request #1532: HIVE-20137: truncate for transactional tables
Posted by GitBox <gi...@apache.org>.
pvargacl commented on pull request #1532:
URL: https://github.com/apache/hive/pull/1532#issuecomment-700962064
@pvary @kuczoram could you review this patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1532: HIVE-20137: truncate for transactional tables
Posted by GitBox <gi...@apache.org>.
pvargacl commented on a change in pull request #1532:
URL: https://github.com/apache/hive/pull/1532#discussion_r498204987
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
##########
@@ -114,16 +114,16 @@
public class AcidUtils {
// This key will be put in the conf file when planning an acid operation
public static final String CONF_ACID_KEY = "hive.doing.acid";
- public static final String BASE_PREFIX = "base_";
+ public static final String BASE_PREFIX = AcidConstants.BASE_PREFIX;
Review comment:
I am not sure I want to remove these public constants, the AcidUtils is unfortunately is being used by spark-acid and it might break compatibility...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a change in pull request #1532: HIVE-20137: truncate for transactional tables
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on a change in pull request #1532:
URL: https://github.com/apache/hive/pull/1532#discussion_r499536263
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
##########
@@ -2999,6 +2980,10 @@ Seems much cleaner if each stmt is identified as a particular HiveOperation (whi
compBuilder.setExclusive();
compBuilder.setOperationType(DataOperationType.NO_TXN);
break;
+ case DDL_EXCL_WRITE:
+ compBuilder.setExclWrite();
Review comment:
ExclWrite is gonna block concurrent reads. Is it expected?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a change in pull request #1532: HIVE-20137: truncate for transactional tables
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on a change in pull request #1532:
URL: https://github.com/apache/hive/pull/1532#discussion_r498072413
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/truncate/TruncateTableAnalyzer.java
##########
@@ -112,23 +113,27 @@ private void checkTruncateEligibility(ASTNode ast, ASTNode root, String tableNam
private void addTruncateTableOutputs(ASTNode root, Table table, Map<String, String> partitionSpec)
throws SemanticException {
+ boolean shared = AcidUtils.isTransactionalTable(table) &&
Review comment:
could we rename local var to truncateFiles or something (same as below)
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
##########
@@ -3357,6 +3355,52 @@ private void truncateTableInternal(String dbName, String tableName, List<String>
}
}
+ /**
+ * Add an empty baseDir with a truncate metadatafile
+ * @param location
+ * @param writeId
+ * @param fs
+ * @throws Exception
+ */
+ private void addTruncateBaseFile(Path location, long writeId, FileSystem fs) throws Exception {
+ Path basePath = new Path(location, AcidConstants.baseDir(writeId));
+ fs.mkdirs(basePath);
+ // We can not leave the folder empty, otherwise it will be skipped at some file listing in AcidUtils
+ // No need for a data file, a simple metadata is enough
Review comment:
Do you think it would make sense to move metaFile creation under MetaDataFile class, not sure however it should be under AcidConstants?
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java
##########
@@ -114,16 +114,16 @@
public class AcidUtils {
// This key will be put in the conf file when planning an acid operation
public static final String CONF_ACID_KEY = "hive.doing.acid";
- public static final String BASE_PREFIX = "base_";
+ public static final String BASE_PREFIX = AcidConstants.BASE_PREFIX;
Review comment:
could we remove those and reference constants from AcidConstants directly
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1532: HIVE-20137: truncate for transactional tables
Posted by GitBox <gi...@apache.org>.
pvargacl commented on a change in pull request #1532:
URL: https://github.com/apache/hive/pull/1532#discussion_r498224776
##########
File path: standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
##########
@@ -3357,6 +3355,52 @@ private void truncateTableInternal(String dbName, String tableName, List<String>
}
}
+ /**
+ * Add an empty baseDir with a truncate metadatafile
+ * @param location
+ * @param writeId
+ * @param fs
+ * @throws Exception
+ */
+ private void addTruncateBaseFile(Path location, long writeId, FileSystem fs) throws Exception {
+ Path basePath = new Path(location, AcidConstants.baseDir(writeId));
+ fs.mkdirs(basePath);
+ // We can not leave the folder empty, otherwise it will be skipped at some file listing in AcidUtils
+ // No need for a data file, a simple metadata is enough
Review comment:
added the write to the MetaDataFile and moved it out from the AcidConstants class
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ merged pull request #1532: HIVE-20137: truncate for transactional tables
Posted by GitBox <gi...@apache.org>.
deniskuzZ merged pull request #1532:
URL: https://github.com/apache/hive/pull/1532
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org