You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/07/06 11:30:33 UTC
[GitHub] [incubator-doris] caiconghui opened a new pull request #4029: Support check commited txns before catalog drop meta, like db, table, partition etc
caiconghui opened a new pull request #4029:
URL: https://github.com/apache/incubator-doris/pull/4029
This PR is to ensure that dropped db , table or partition can be with normal state after recovered by user. Commited txns can not be aborted, because the partitions's commited versions have been changed, and some tablets may already have new visiable versions. If user just don't want the meta(db, table or partition) anymore, just use dropp instead of drop to skip commited txn check.
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on pull request #4029: Support check commited txns before catalog drop meta, like db, table, partition etc
Posted by GitBox <gi...@apache.org>.
caiconghui commented on pull request #4029:
URL: https://github.com/apache/incubator-doris/pull/4029#issuecomment-654177442
for #3988
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request #4029: Support check commited txns before catalog drop meta, like db, table, partition etc
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #4029:
URL: https://github.com/apache/incubator-doris/pull/4029#discussion_r450338931
##########
File path: fe/src/main/java/org/apache/doris/catalog/Catalog.java
##########
@@ -3308,6 +3315,20 @@ public void dropPartition(Database db, OlapTable olapTable, DropPartitionClause
if (isTempPartition) {
olapTable.dropTempPartition(partitionName, true);
} else {
+ if (clause.isNeedCheckCommitedTxns()) {
Review comment:
my bad, I will fix it.
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] chaoyli commented on pull request #4029: Support check committed txns before catalog drop meta, like db, table, partition etc
Posted by GitBox <gi...@apache.org>.
chaoyli commented on pull request #4029:
URL: https://github.com/apache/incubator-doris/pull/4029#issuecomment-660640082
I found if perform DROP database force, it's committed version will be recovered by RECOVER command?
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] chaoyli merged pull request #4029: Support check committed txns before catalog drop meta, like db, table, partition etc
Posted by GitBox <gi...@apache.org>.
chaoyli merged pull request #4029:
URL: https://github.com/apache/incubator-doris/pull/4029
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request #4029: Support check commited txns before catalog drop meta, like db, table, partition etc
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #4029:
URL: https://github.com/apache/incubator-doris/pull/4029#discussion_r450272944
##########
File path: fe/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java
##########
@@ -716,7 +716,6 @@ public void processBatchDropRollup(List<AlterClause> dropRollupClauses, Database
public void processDropMaterializedView(DropMaterializedViewStmt dropMaterializedViewStmt, Database db,
OlapTable olapTable) throws DdlException, MetaNotFoundException {
- db.writeLock();
Review comment:
Add a `Preconditions.checkState( db.isWriteLockHeldByCurrentThread())` to NOTICE.
##########
File path: fe/src/main/java/org/apache/doris/analysis/DropTableStmt.java
##########
@@ -32,17 +32,20 @@
private boolean ifExists;
private final TableName tableName;
private final boolean isView;
+ private boolean needCheckCommitedTxns;
Review comment:
```suggestion
private boolean needCheckCommittedTxns;
```
##########
File path: fe/src/main/java/org/apache/doris/catalog/Catalog.java
##########
@@ -2666,6 +2666,13 @@ public void dropDb(DropDbStmt stmt) throws DdlException {
Database db = this.fullNameToDb.get(dbName);
db.writeLock();
try {
+ if (stmt.isNeedCheckCommitedTxns()) {
+ if (Catalog.getCurrentCatalog().getGlobalTransactionMgr().existCommittedTxns(db.getId(), null, null)) {
+ throw new DdlException("There are still some commited txns cannot be aborted in db ["
Review comment:
There are still some transactions in the COMMITTED state waiting to be completed. The database cannot be dropped. If you want to force drop(cannot be recovered), please use "DROPP database";
##########
File path: fe/src/main/java/org/apache/doris/analysis/DropTableStmt.java
##########
@@ -61,6 +64,10 @@ public boolean isView() {
return isView;
}
+ public boolean isNeedCheckCommitedTxns() {
Review comment:
```suggestion
public boolean isNeedCheckCommittedTxns() {
```
##########
File path: fe/src/main/java/org/apache/doris/analysis/DropTableStmt.java
##########
@@ -32,17 +32,20 @@
private boolean ifExists;
private final TableName tableName;
private final boolean isView;
+ private boolean needCheckCommitedTxns;
- public DropTableStmt(boolean ifExists, TableName tableName) {
+ public DropTableStmt(boolean ifExists, TableName tableName, boolean needCheckCommitedTxns) {
Review comment:
```suggestion
public DropTableStmt(boolean ifExists, TableName tableName, boolean needCheckCommittedTxns) {
```
##########
File path: fe/src/main/java/org/apache/doris/analysis/DropTableStmt.java
##########
@@ -32,17 +32,20 @@
private boolean ifExists;
private final TableName tableName;
private final boolean isView;
+ private boolean needCheckCommitedTxns;
- public DropTableStmt(boolean ifExists, TableName tableName) {
+ public DropTableStmt(boolean ifExists, TableName tableName, boolean needCheckCommitedTxns) {
this.ifExists = ifExists;
this.tableName = tableName;
this.isView = false;
+ this.needCheckCommitedTxns = needCheckCommitedTxns;
Review comment:
```suggestion
this.needCheckCommittedTxns = needCheckCommittedTxns;
```
##########
File path: fe/src/main/java/org/apache/doris/catalog/Catalog.java
##########
@@ -3308,6 +3315,20 @@ public void dropPartition(Database db, OlapTable olapTable, DropPartitionClause
if (isTempPartition) {
olapTable.dropTempPartition(partitionName, true);
} else {
+ if (clause.isNeedCheckCommitedTxns()) {
Review comment:
Duplicated `if clause`?
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on pull request #4029: Support check commited txns before catalog drop meta, like db, table, partition etc
Posted by GitBox <gi...@apache.org>.
caiconghui commented on pull request #4029:
URL: https://github.com/apache/incubator-doris/pull/4029#issuecomment-654309078
> If I use `DROPP Table` to forcibly drop a table with COMMITTED txns.
> Can those COMMITTED txns be cleaned automatically?
@morningman in fe, the txn will be publish timeout, and publish version daemon will try to finish transaction, it find that table have been dropped and will remove table committed info from txn, the txn will be visible. In be, txns seem that it wiil be cleaned by reporter handler?
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on pull request #4029: Support check committed txns before catalog drop meta, like db, table, partition etc
Posted by GitBox <gi...@apache.org>.
caiconghui commented on pull request #4029:
URL: https://github.com/apache/incubator-doris/pull/4029#issuecomment-660669466
> I found if perform DROP database force, it's committed version will be recovered by RECOVER command?
@chaoyli I will fix it, make force drop operation do not recycle meta.
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] chaoyli commented on a change in pull request #4029: Support check committed txns before catalog drop meta, like db, table, partition etc
Posted by GitBox <gi...@apache.org>.
chaoyli commented on a change in pull request #4029:
URL: https://github.com/apache/incubator-doris/pull/4029#discussion_r450621125
##########
File path: fe/src/main/cup/sql_parser.cup
##########
@@ -879,7 +879,11 @@ alter_table_clause ::=
:}
| KW_DROP opt_tmp:isTempPartition KW_PARTITION opt_if_exists:ifExists ident:partitionName
{:
- RESULT = new DropPartitionClause(ifExists, partitionName, isTempPartition);
+ RESULT = new DropPartitionClause(ifExists, partitionName, isTempPartition, !isTempPartition);
+ :}
+ | KW_DROPP opt_tmp:isTempPartition KW_PARTITION opt_if_exists:ifExists ident:partitionName
Review comment:
DROPP is a not a good syntax. It's better to use
```
DROP TABLE $table_name FORCE
```
##########
File path: fe/src/main/java/org/apache/doris/catalog/Catalog.java
##########
@@ -2666,6 +2666,13 @@ public void dropDb(DropDbStmt stmt) throws DdlException {
Database db = this.fullNameToDb.get(dbName);
db.writeLock();
try {
+ if (stmt.isNeedCheckCommittedTxns()) {
+ if (Catalog.getCurrentCatalog().getGlobalTransactionMgr().existCommittedTxns(db.getId(), null, null)) {
+ throw new DdlException("There are still some transactions in the COMMITTED state waiting to be completed. " +
+ "The database [" + dbName +"] cannot be dropped. If you want to forcibly drop(cannot be recovered)," +
+ " please use \"DROP database force\".");
+ }
Review comment:
DROP database FORCE. Use upper case will be better.
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request #4029: Support check committed txns before catalog drop meta, like db, table, partition etc
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #4029:
URL: https://github.com/apache/incubator-doris/pull/4029#discussion_r460692744
##########
File path: fe/src/main/java/org/apache/doris/catalog/Catalog.java
##########
@@ -2666,6 +2666,13 @@ public void dropDb(DropDbStmt stmt) throws DdlException {
Database db = this.fullNameToDb.get(dbName);
db.writeLock();
try {
+ if (stmt.isNeedCheckCommittedTxns()) {
+ if (Catalog.getCurrentCatalog().getGlobalTransactionMgr().existCommittedTxns(db.getId(), null, null)) {
+ throw new DdlException("There are still some transactions in the COMMITTED state waiting to be completed. " +
+ "The database [" + dbName +"] cannot be dropped. If you want to forcibly drop(cannot be recovered)," +
+ " please use \"DROP database force\".");
+ }
Review comment:
ok
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on pull request #4029: Support check commited txns before catalog drop meta, like db, table, partition etc
Posted by GitBox <gi...@apache.org>.
morningman commented on pull request #4029:
URL: https://github.com/apache/incubator-doris/pull/4029#issuecomment-654287503
If I use `DROPP Table` to forcibly drop a table with COMMITTED txns.
Can those COMMITTED txns be cleaned automatically?
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui commented on a change in pull request #4029: Support check committed txns before catalog drop meta, like db, table, partition etc
Posted by GitBox <gi...@apache.org>.
caiconghui commented on a change in pull request #4029:
URL: https://github.com/apache/incubator-doris/pull/4029#discussion_r460699221
##########
File path: fe/src/main/cup/sql_parser.cup
##########
@@ -879,7 +879,11 @@ alter_table_clause ::=
:}
| KW_DROP opt_tmp:isTempPartition KW_PARTITION opt_if_exists:ifExists ident:partitionName
{:
- RESULT = new DropPartitionClause(ifExists, partitionName, isTempPartition);
+ RESULT = new DropPartitionClause(ifExists, partitionName, isTempPartition, !isTempPartition);
+ :}
+ | KW_DROPP opt_tmp:isTempPartition KW_PARTITION opt_if_exists:ifExists ident:partitionName
Review comment:
@chaoyli I has fixed it before. old commit version?
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] caiconghui edited a comment on pull request #4029: Support check commited txns before catalog drop meta, like db, table, partition etc
Posted by GitBox <gi...@apache.org>.
caiconghui edited a comment on pull request #4029:
URL: https://github.com/apache/incubator-doris/pull/4029#issuecomment-654309078
> If I use `DROPP Table` to forcibly drop a table with COMMITTED txns.
> Can those COMMITTED txns be cleaned automatically?
@morningman in fe, the txn will be publish timeout, and publish version daemon will try to finish transaction, it find that table have been dropped and will remove table committed info from txn, the txn will be visible. In be, it seems that txns wiil be cleaned by reporter handler?
----------------------------------------------------------------
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org