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 2022/04/18 14:55:34 UTC
[GitHub] [hive] deniskuzZ opened a new pull request, #3220: HIVE-26149: Non blocking DROP DATABASE implementation
deniskuzZ opened a new pull request, #3220:
URL: https://github.com/apache/hive/pull/3220
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3220:
URL: https://github.com/apache/hive/pull/3220#discussion_r857349784
##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java:
##########
@@ -661,16 +662,36 @@ public void dropDatabase(String name, boolean deleteData, boolean ignoreUnknownD
*/
public void dropDatabase(String name, boolean deleteData, boolean ignoreUnknownDb, boolean cascade)
throws HiveException, NoSuchObjectException {
+ dropDatabase(
+ new DropDatabaseDesc(name, ignoreUnknownDb, cascade, deleteData));
+ }
+
+ public void dropDatabase(DropDatabaseDesc desc)
Review Comment:
Nit: I would guess that we do not need the new line here
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3220:
URL: https://github.com/apache/hive/pull/3220#discussion_r857390546
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -1534,43 +1538,50 @@ public void dropDatabase(String catalogName, String dbName, boolean deleteData,
* @param maxBatchSize
* @throws TException
*/
- private void dropDatabaseCascadePerTable(String catName, String dbName, List<String> tableList,
- boolean deleteData, int maxBatchSize) throws TException {
- String dbNameWithCatalog = prependCatalogToDbName(catName, dbName, conf);
- for (Table table : new TableIterable(this, catName, dbName, tableList, maxBatchSize)) {
+ private void dropDatabaseCascadePerTable(DropDatabaseRequest req, List<String> tableList, int maxBatchSize)
+ throws TException {
+ String dbNameWithCatalog = prependCatalogToDbName(req.getCatalogName(), req.getName(), conf);
+ for (Table table : new TableIterable(
+ this, req.getCatalogName(), req.getName(), tableList, maxBatchSize)) {
boolean success = false;
HiveMetaHook hook = getHook(table);
- if (hook == null) {
- continue;
- }
try {
- hook.preDropTable(table);
- client.drop_table_with_environment_context(dbNameWithCatalog, table.getTableName(), deleteData, null);
- hook.commitDropTable(table, deleteData);
+ if (hook != null) {
+ hook.preDropTable(table);
+ }
+ boolean isSoftDelete = req.isSoftDelete() && Boolean.parseBoolean(
+ table.getParameters().getOrDefault(SOFT_DELETE_TABLE, "false"));
+ EnvironmentContext context = null;
+ if (req.isSetTxnId()) {
+ context = new EnvironmentContext();
+ context.putToProperties("txnId", String.valueOf(req.getTxnId()));
+ req.setDeleteManagedDir(false);
+ }
+ client.drop_table_with_environment_context(dbNameWithCatalog, table.getTableName(),
+ req.isDeleteData() && !isSoftDelete, context);
+ if (hook != null) {
+ hook.commitDropTable(table, req.isDeleteData());
+ }
success = true;
} finally {
- if (!success) {
+ if (!success && hook != null) {
hook.rollbackDropTable(table);
}
}
}
- client.drop_database(dbNameWithCatalog, deleteData, true);
+ client.drop_database_req(req);
}
/**
* Handles dropDatabase by invoking drop_database in HMS.
* Useful when table list in DB can fit in memory, it will retrieve all tables at once and
* call drop_database once. Also handles drop_table hooks.
- * @param catName
- * @param dbName
+ * @param req
* @param tableList
- * @param deleteData
* @throws TException
*/
- private void dropDatabaseCascadePerDb(String catName, String dbName, List<String> tableList,
- boolean deleteData) throws TException {
- String dbNameWithCatalog = prependCatalogToDbName(catName, dbName, conf);
- List<Table> tables = getTableObjectsByName(catName, dbName, tableList);
+ private void dropDatabaseCascadePerDb(DropDatabaseRequest req, List<String> tableList) throws TException {
Review Comment:
How does this work together with:
```
// We want no lock here, as the database lock will cover the tables,
// and putting a lock will actually cause us to deadlock on ourselves.
```
Wouldn't it cause issues with the locks?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3220:
URL: https://github.com/apache/hive/pull/3220#discussion_r857351954
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -1457,39 +1458,42 @@ public void dropDatabase(String name)
@Override
public void dropDatabase(String name, boolean deleteData, boolean ignoreUnknownDb)
- throws NoSuchObjectException, InvalidOperationException, MetaException, TException {
+ throws TException {
Review Comment:
Could we make the old methods deprecated?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3220:
URL: https://github.com/apache/hive/pull/3220#discussion_r860846223
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/drop/DropDatabaseAnalyzer.java:
##########
@@ -49,28 +52,36 @@ public void analyzeInternal(ASTNode root) throws SemanticException {
String databaseName = unescapeIdentifier(root.getChild(0).getText());
boolean ifExists = root.getFirstChildWithType(HiveParser.TOK_IFEXISTS) != null;
boolean cascade = root.getFirstChildWithType(HiveParser.TOK_CASCADE) != null;
+ boolean isSoftDelete = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
Database database = getDatabase(databaseName, !ifExists);
if (database == null) {
return;
}
-
// if cascade=true, then we need to authorize the drop table action as well, and add the tables to the outputs
+ boolean allTablesWithSuffix = false;
if (cascade) {
try {
- for (Table table : db.getAllTableObjects(databaseName)) {
- // We want no lock here, as the database lock will cover the tables,
- // and putting a lock will actually cause us to deadlock on ourselves.
- outputs.add(new WriteEntity(table, WriteEntity.WriteType.DDL_NO_LOCK));
+ List<Table> tables = db.getAllTableObjects(databaseName);
+ allTablesWithSuffix = tables.stream().allMatch(
+ table -> AcidUtils.isTableSoftDeleteEnabled(table, conf));
+ for (Table table : tables) {
+ // Optimization used to limit number of requested locks. Check if table lock is needed or we could get away with single DB level lock,
+ boolean isTableLockNeeded = isSoftDelete && !allTablesWithSuffix;
+ outputs.add(new WriteEntity(table, isTableLockNeeded ?
+ AcidUtils.isTableSoftDeleteEnabled(table, conf) ?
+ WriteEntity.WriteType.DDL_EXCL_WRITE : WriteEntity.WriteType.DDL_EXCLUSIVE :
+ WriteEntity.WriteType.DDL_NO_LOCK));
Review Comment:
refactored
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3220:
URL: https://github.com/apache/hive/pull/3220#discussion_r857376124
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -1534,43 +1538,50 @@ public void dropDatabase(String catalogName, String dbName, boolean deleteData,
* @param maxBatchSize
* @throws TException
*/
- private void dropDatabaseCascadePerTable(String catName, String dbName, List<String> tableList,
- boolean deleteData, int maxBatchSize) throws TException {
- String dbNameWithCatalog = prependCatalogToDbName(catName, dbName, conf);
- for (Table table : new TableIterable(this, catName, dbName, tableList, maxBatchSize)) {
+ private void dropDatabaseCascadePerTable(DropDatabaseRequest req, List<String> tableList, int maxBatchSize)
+ throws TException {
+ String dbNameWithCatalog = prependCatalogToDbName(req.getCatalogName(), req.getName(), conf);
+ for (Table table : new TableIterable(
+ this, req.getCatalogName(), req.getName(), tableList, maxBatchSize)) {
boolean success = false;
HiveMetaHook hook = getHook(table);
- if (hook == null) {
- continue;
- }
try {
- hook.preDropTable(table);
- client.drop_table_with_environment_context(dbNameWithCatalog, table.getTableName(), deleteData, null);
- hook.commitDropTable(table, deleteData);
+ if (hook != null) {
+ hook.preDropTable(table);
+ }
+ boolean isSoftDelete = req.isSoftDelete() && Boolean.parseBoolean(
+ table.getParameters().getOrDefault(SOFT_DELETE_TABLE, "false"));
+ EnvironmentContext context = null;
+ if (req.isSetTxnId()) {
+ context = new EnvironmentContext();
+ context.putToProperties("txnId", String.valueOf(req.getTxnId()));
+ req.setDeleteManagedDir(false);
+ }
+ client.drop_table_with_environment_context(dbNameWithCatalog, table.getTableName(),
+ req.isDeleteData() && !isSoftDelete, context);
+ if (hook != null) {
+ hook.commitDropTable(table, req.isDeleteData());
+ }
success = true;
} finally {
- if (!success) {
+ if (!success && hook != null) {
hook.rollbackDropTable(table);
}
}
}
- client.drop_database(dbNameWithCatalog, deleteData, true);
+ client.drop_database_req(req);
}
/**
* Handles dropDatabase by invoking drop_database in HMS.
* Useful when table list in DB can fit in memory, it will retrieve all tables at once and
* call drop_database once. Also handles drop_table hooks.
- * @param catName
- * @param dbName
+ * @param req
* @param tableList
- * @param deleteData
* @throws TException
*/
- private void dropDatabaseCascadePerDb(String catName, String dbName, List<String> tableList,
- boolean deleteData) throws TException {
- String dbNameWithCatalog = prependCatalogToDbName(catName, dbName, conf);
- List<Table> tables = getTableObjectsByName(catName, dbName, tableList);
+ private void dropDatabaseCascadePerDb(DropDatabaseRequest req, List<String> tableList) throws TException {
Review Comment:
if DB has a mix of soft-delete(prefixed) and managed tables, we acquire exclusive locks and remove as usual managed/external tables, however, for soft-delete tables we acquire excl_write lock and delegate cleanup to the cleaner process. Read locks are only removed on soft-delete tables.
Note: if lockless reads are enabled we do not remove the db folder.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3220:
URL: https://github.com/apache/hive/pull/3220#discussion_r857350352
##########
ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands.java:
##########
@@ -1685,7 +1688,89 @@ public void testDropWithBaseMultiplePartitions() throws Exception {
}
}
}
+
+ @Test
+ public void testDropDatabaseCascadePerTableNonBlocking() throws Exception {
+ MetastoreConf.setLongVar(hiveConf, MetastoreConf.ConfVars.BATCH_RETRIEVE_MAX, 1);
+ dropDatabaseCascadeNonBlocking();
+ }
+ @Test
+ public void testDropDatabaseCascadePerDbNonBlocking() throws Exception {
+ dropDatabaseCascadeNonBlocking();
+ }
+ private void dropDatabaseCascadeNonBlocking() throws Exception {
Review Comment:
Nit: newline
##########
ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands.java:
##########
@@ -1685,7 +1688,89 @@ public void testDropWithBaseMultiplePartitions() throws Exception {
}
}
}
+
+ @Test
+ public void testDropDatabaseCascadePerTableNonBlocking() throws Exception {
+ MetastoreConf.setLongVar(hiveConf, MetastoreConf.ConfVars.BATCH_RETRIEVE_MAX, 1);
+ dropDatabaseCascadeNonBlocking();
+ }
+ @Test
Review Comment:
Nit: newline
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3220:
URL: https://github.com/apache/hive/pull/3220#discussion_r857347623
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/drop/DropDatabaseAnalyzer.java:
##########
@@ -49,28 +52,37 @@ public void analyzeInternal(ASTNode root) throws SemanticException {
String databaseName = unescapeIdentifier(root.getChild(0).getText());
boolean ifExists = root.getFirstChildWithType(HiveParser.TOK_IFEXISTS) != null;
boolean cascade = root.getFirstChildWithType(HiveParser.TOK_CASCADE) != null;
+ boolean isSoftDelete = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
Database database = getDatabase(databaseName, !ifExists);
if (database == null) {
return;
}
-
// if cascade=true, then we need to authorize the drop table action as well, and add the tables to the outputs
+ boolean allTablesWithSuffix = false;
if (cascade) {
try {
- for (Table table : db.getAllTableObjects(databaseName)) {
+ List<Table> tables = db.getAllTableObjects(databaseName);
+ allTablesWithSuffix = tables.stream().allMatch(
+ table -> AcidUtils.isTableSoftDeleteEnabled(table, conf));
+ for (Table table : tables) {
// We want no lock here, as the database lock will cover the tables,
// and putting a lock will actually cause us to deadlock on ourselves.
- outputs.add(new WriteEntity(table, WriteEntity.WriteType.DDL_NO_LOCK));
+ outputs.add(
+ new WriteEntity(table, isSoftDelete && !allTablesWithSuffix ?
Review Comment:
Nit: Could we create boolean variables with descriptive names? It is hard to follow what happens here.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3220:
URL: https://github.com/apache/hive/pull/3220#discussion_r860668405
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/drop/DropDatabaseAnalyzer.java:
##########
@@ -49,28 +52,36 @@ public void analyzeInternal(ASTNode root) throws SemanticException {
String databaseName = unescapeIdentifier(root.getChild(0).getText());
boolean ifExists = root.getFirstChildWithType(HiveParser.TOK_IFEXISTS) != null;
boolean cascade = root.getFirstChildWithType(HiveParser.TOK_CASCADE) != null;
+ boolean isSoftDelete = HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_ACID_LOCKLESS_READS_ENABLED);
Database database = getDatabase(databaseName, !ifExists);
if (database == null) {
return;
}
-
// if cascade=true, then we need to authorize the drop table action as well, and add the tables to the outputs
+ boolean allTablesWithSuffix = false;
if (cascade) {
try {
- for (Table table : db.getAllTableObjects(databaseName)) {
- // We want no lock here, as the database lock will cover the tables,
- // and putting a lock will actually cause us to deadlock on ourselves.
- outputs.add(new WriteEntity(table, WriteEntity.WriteType.DDL_NO_LOCK));
+ List<Table> tables = db.getAllTableObjects(databaseName);
+ allTablesWithSuffix = tables.stream().allMatch(
+ table -> AcidUtils.isTableSoftDeleteEnabled(table, conf));
+ for (Table table : tables) {
+ // Optimization used to limit number of requested locks. Check if table lock is needed or we could get away with single DB level lock,
+ boolean isTableLockNeeded = isSoftDelete && !allTablesWithSuffix;
+ outputs.add(new WriteEntity(table, isTableLockNeeded ?
+ AcidUtils.isTableSoftDeleteEnabled(table, conf) ?
+ WriteEntity.WriteType.DDL_EXCL_WRITE : WriteEntity.WriteType.DDL_EXCLUSIVE :
+ WriteEntity.WriteType.DDL_NO_LOCK));
Review Comment:
Would this be better:
```
LockType lockType = WriteEntity.WriteType.DDL_NO_LOCK;
if (isTableLockNeeded) {
lockType = AcidUtils.isTableSoftDeleteEnabled(table, conf) ?
WriteEntity.WriteType.DDL_EXCL_WRITE : WriteEntity.WriteType.DDL_EXCLUSIVE;
}
outputs.add(new WriteEntity(table, lockType));
```
I think having too many `:` and `?` is really hard to read.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3220:
URL: https://github.com/apache/hive/pull/3220#discussion_r857355770
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -1534,43 +1538,50 @@ public void dropDatabase(String catalogName, String dbName, boolean deleteData,
* @param maxBatchSize
* @throws TException
*/
- private void dropDatabaseCascadePerTable(String catName, String dbName, List<String> tableList,
- boolean deleteData, int maxBatchSize) throws TException {
- String dbNameWithCatalog = prependCatalogToDbName(catName, dbName, conf);
- for (Table table : new TableIterable(this, catName, dbName, tableList, maxBatchSize)) {
+ private void dropDatabaseCascadePerTable(DropDatabaseRequest req, List<String> tableList, int maxBatchSize)
+ throws TException {
+ String dbNameWithCatalog = prependCatalogToDbName(req.getCatalogName(), req.getName(), conf);
+ for (Table table : new TableIterable(
+ this, req.getCatalogName(), req.getName(), tableList, maxBatchSize)) {
boolean success = false;
HiveMetaHook hook = getHook(table);
- if (hook == null) {
- continue;
- }
try {
- hook.preDropTable(table);
- client.drop_table_with_environment_context(dbNameWithCatalog, table.getTableName(), deleteData, null);
- hook.commitDropTable(table, deleteData);
+ if (hook != null) {
+ hook.preDropTable(table);
+ }
+ boolean isSoftDelete = req.isSoftDelete() && Boolean.parseBoolean(
+ table.getParameters().getOrDefault(SOFT_DELETE_TABLE, "false"));
+ EnvironmentContext context = null;
+ if (req.isSetTxnId()) {
+ context = new EnvironmentContext();
+ context.putToProperties("txnId", String.valueOf(req.getTxnId()));
+ req.setDeleteManagedDir(false);
+ }
+ client.drop_table_with_environment_context(dbNameWithCatalog, table.getTableName(),
+ req.isDeleteData() && !isSoftDelete, context);
+ if (hook != null) {
+ hook.commitDropTable(table, req.isDeleteData());
+ }
success = true;
} finally {
- if (!success) {
+ if (!success && hook != null) {
hook.rollbackDropTable(table);
}
}
}
- client.drop_database(dbNameWithCatalog, deleteData, true);
+ client.drop_database_req(req);
}
/**
* Handles dropDatabase by invoking drop_database in HMS.
* Useful when table list in DB can fit in memory, it will retrieve all tables at once and
* call drop_database once. Also handles drop_table hooks.
- * @param catName
- * @param dbName
+ * @param req
* @param tableList
- * @param deleteData
* @throws TException
*/
- private void dropDatabaseCascadePerDb(String catName, String dbName, List<String> tableList,
- boolean deleteData) throws TException {
- String dbNameWithCatalog = prependCatalogToDbName(catName, dbName, conf);
- List<Table> tables = getTableObjectsByName(catName, dbName, tableList);
+ private void dropDatabaseCascadePerDb(DropDatabaseRequest req, List<String> tableList) throws TException {
Review Comment:
What happens when the tables inside the db has a different configuration. Some of the tables are soft delete, and some of the tables are hard delete. Also what happens if the db and the table soft delete configuration is different?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3220:
URL: https://github.com/apache/hive/pull/3220#discussion_r857351082
##########
ql/src/test/org/apache/hadoop/hive/ql/lockmgr/TestDbTxnManager2.java:
##########
@@ -3914,4 +3914,72 @@ private void testRenamePartition(boolean blocking) throws Exception {
driver.getFetchTask().fetch(res);
Assert.assertEquals("Expecting 1 rows and found " + res.size(), 1, res.size());
}
+
+ @Test
+ public void testDropDatabaseNonBlocking() throws Exception {
+ dropDatabaseNonBlocking(false, false);
+ }
+ @Test
Review Comment:
nit: newlines
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3220:
URL: https://github.com/apache/hive/pull/3220#discussion_r857525828
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -1534,43 +1538,50 @@ public void dropDatabase(String catalogName, String dbName, boolean deleteData,
* @param maxBatchSize
* @throws TException
*/
- private void dropDatabaseCascadePerTable(String catName, String dbName, List<String> tableList,
- boolean deleteData, int maxBatchSize) throws TException {
- String dbNameWithCatalog = prependCatalogToDbName(catName, dbName, conf);
- for (Table table : new TableIterable(this, catName, dbName, tableList, maxBatchSize)) {
+ private void dropDatabaseCascadePerTable(DropDatabaseRequest req, List<String> tableList, int maxBatchSize)
+ throws TException {
+ String dbNameWithCatalog = prependCatalogToDbName(req.getCatalogName(), req.getName(), conf);
+ for (Table table : new TableIterable(
+ this, req.getCatalogName(), req.getName(), tableList, maxBatchSize)) {
boolean success = false;
HiveMetaHook hook = getHook(table);
- if (hook == null) {
- continue;
- }
try {
- hook.preDropTable(table);
- client.drop_table_with_environment_context(dbNameWithCatalog, table.getTableName(), deleteData, null);
- hook.commitDropTable(table, deleteData);
+ if (hook != null) {
+ hook.preDropTable(table);
+ }
+ boolean isSoftDelete = req.isSoftDelete() && Boolean.parseBoolean(
+ table.getParameters().getOrDefault(SOFT_DELETE_TABLE, "false"));
+ EnvironmentContext context = null;
+ if (req.isSetTxnId()) {
+ context = new EnvironmentContext();
+ context.putToProperties("txnId", String.valueOf(req.getTxnId()));
+ req.setDeleteManagedDir(false);
+ }
+ client.drop_table_with_environment_context(dbNameWithCatalog, table.getTableName(),
+ req.isDeleteData() && !isSoftDelete, context);
+ if (hook != null) {
+ hook.commitDropTable(table, req.isDeleteData());
+ }
success = true;
} finally {
- if (!success) {
+ if (!success && hook != null) {
hook.rollbackDropTable(table);
}
}
}
- client.drop_database(dbNameWithCatalog, deleteData, true);
+ client.drop_database_req(req);
}
/**
* Handles dropDatabase by invoking drop_database in HMS.
* Useful when table list in DB can fit in memory, it will retrieve all tables at once and
* call drop_database once. Also handles drop_table hooks.
- * @param catName
- * @param dbName
+ * @param req
* @param tableList
- * @param deleteData
* @throws TException
*/
- private void dropDatabaseCascadePerDb(String catName, String dbName, List<String> tableList,
- boolean deleteData) throws TException {
- String dbNameWithCatalog = prependCatalogToDbName(catName, dbName, conf);
- List<Table> tables = getTableObjectsByName(catName, dbName, tableList);
+ private void dropDatabaseCascadePerDb(DropDatabaseRequest req, List<String> tableList) throws TException {
Review Comment:
that remains if soft delete is disabled, otherwise, we'll be locking just tables/not DB with an appropriate type of lock.
allTablesWithSuffix is an optimization, if all tables under DB are soft-delete eligible - grad just table-level lock.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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 #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
deniskuzZ merged PR #3220:
URL: https://github.com/apache/hive/pull/3220
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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] pvary commented on a diff in pull request #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
pvary commented on code in PR #3220:
URL: https://github.com/apache/hive/pull/3220#discussion_r860672337
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java:
##########
@@ -1869,8 +1880,7 @@ void dropDatabase(String catName, String dbName, boolean deleteData, boolean ign
* @throws MetaException something went wrong, usually either in the RDBMS or storage.
* @throws TException general thrift error.
*/
- default void dropDatabase(String catName, String dbName, boolean deleteData,
- boolean ignoreUnknownDb)
+ default void dropDatabase(String catName, String dbName, boolean deleteData, boolean ignoreUnknownDb)
Review Comment:
Nit: I think this could be deprecated as well
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3220:
URL: https://github.com/apache/hive/pull/3220#discussion_r860846614
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java:
##########
@@ -1869,8 +1880,7 @@ void dropDatabase(String catName, String dbName, boolean deleteData, boolean ign
* @throws MetaException something went wrong, usually either in the RDBMS or storage.
* @throws TException general thrift error.
*/
- default void dropDatabase(String catName, String dbName, boolean deleteData,
- boolean ignoreUnknownDb)
+ default void dropDatabase(String catName, String dbName, boolean deleteData, boolean ignoreUnknownDb)
Review Comment:
marked as deprecated
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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 diff in pull request #3220: HIVE-26149: Non blocking DROP DATABASE implementation
Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on code in PR #3220:
URL: https://github.com/apache/hive/pull/3220#discussion_r857525828
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -1534,43 +1538,50 @@ public void dropDatabase(String catalogName, String dbName, boolean deleteData,
* @param maxBatchSize
* @throws TException
*/
- private void dropDatabaseCascadePerTable(String catName, String dbName, List<String> tableList,
- boolean deleteData, int maxBatchSize) throws TException {
- String dbNameWithCatalog = prependCatalogToDbName(catName, dbName, conf);
- for (Table table : new TableIterable(this, catName, dbName, tableList, maxBatchSize)) {
+ private void dropDatabaseCascadePerTable(DropDatabaseRequest req, List<String> tableList, int maxBatchSize)
+ throws TException {
+ String dbNameWithCatalog = prependCatalogToDbName(req.getCatalogName(), req.getName(), conf);
+ for (Table table : new TableIterable(
+ this, req.getCatalogName(), req.getName(), tableList, maxBatchSize)) {
boolean success = false;
HiveMetaHook hook = getHook(table);
- if (hook == null) {
- continue;
- }
try {
- hook.preDropTable(table);
- client.drop_table_with_environment_context(dbNameWithCatalog, table.getTableName(), deleteData, null);
- hook.commitDropTable(table, deleteData);
+ if (hook != null) {
+ hook.preDropTable(table);
+ }
+ boolean isSoftDelete = req.isSoftDelete() && Boolean.parseBoolean(
+ table.getParameters().getOrDefault(SOFT_DELETE_TABLE, "false"));
+ EnvironmentContext context = null;
+ if (req.isSetTxnId()) {
+ context = new EnvironmentContext();
+ context.putToProperties("txnId", String.valueOf(req.getTxnId()));
+ req.setDeleteManagedDir(false);
+ }
+ client.drop_table_with_environment_context(dbNameWithCatalog, table.getTableName(),
+ req.isDeleteData() && !isSoftDelete, context);
+ if (hook != null) {
+ hook.commitDropTable(table, req.isDeleteData());
+ }
success = true;
} finally {
- if (!success) {
+ if (!success && hook != null) {
hook.rollbackDropTable(table);
}
}
}
- client.drop_database(dbNameWithCatalog, deleteData, true);
+ client.drop_database_req(req);
}
/**
* Handles dropDatabase by invoking drop_database in HMS.
* Useful when table list in DB can fit in memory, it will retrieve all tables at once and
* call drop_database once. Also handles drop_table hooks.
- * @param catName
- * @param dbName
+ * @param req
* @param tableList
- * @param deleteData
* @throws TException
*/
- private void dropDatabaseCascadePerDb(String catName, String dbName, List<String> tableList,
- boolean deleteData) throws TException {
- String dbNameWithCatalog = prependCatalogToDbName(catName, dbName, conf);
- List<Table> tables = getTableObjectsByName(catName, dbName, tableList);
+ private void dropDatabaseCascadePerDb(DropDatabaseRequest req, List<String> tableList) throws TException {
Review Comment:
that remains if soft delete is disabled, otherwise, we'll be locking just tables/not DB with an appropriate type of lock.
allTablesWithSuffix is an optimization, if all tables under DB are soft-delete eligible - grad just DB-level lock.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
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