morningman commented on a change in pull request #6650:
URL: https://github.com/apache/incubator-doris/pull/6650#discussion_r800152957
##########
File path: fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
##########
@@ -490,6 +496,7 @@ private void replaceTableInternal(Database db, OlapTable
origTable, OlapTable ne
} else {
// not swap, the origin table is not used anymore, need to drop
all its tablets.
Catalog.getCurrentCatalog().onEraseOlapTable(origTable, isReplay);
+ origTable.markDropped();
Review comment:
How about moving `markDropped()` inside the `db.dropTable()` and
`catalog.onEraseOlapTable()` method?
So that we don't need to call `markDropped()` every time along with these
methods manually.
And also, add `markUndropped()` method to `db.createTable()` method to
address the operation like `modifyView` or `renameTable`, etc.
##########
File path: fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
##########
@@ -415,27 +417,31 @@ private void processReplaceTable(Database db, OlapTable
origTable, List<AlterCla
ReplaceTableClause clause = (ReplaceTableClause) alterClauses.get(0);
String newTblName = clause.getTblName();
boolean swapTable = clause.isSwapTable();
- Table newTbl = db.getTableOrMetaException(newTblName, TableType.OLAP);
- OlapTable olapNewTbl = (OlapTable) newTbl;
- db.writeLock();
- origTable.writeLock();
+ db.writeLockOrDdlException();
try {
- String oldTblName = origTable.getName();
- // First, we need to check whether the table to be operated on can
be renamed
- olapNewTbl.checkAndSetName(oldTblName, true);
- if (swapTable) {
- origTable.checkAndSetName(newTblName, true);
+ Table newTbl = db.getTableOrMetaException(newTblName,
TableType.OLAP);
+ OlapTable olapNewTbl = (OlapTable) newTbl;
+ List<Table> tableList = Lists.newArrayList(origTable, newTbl);
+ tableList.sort((Comparator.comparing(Table::getId)));
+ MetaLockUtils.writeLockTables(tableList);
Review comment:
no need to use `writeLockTablesOrDdlException`?
##########
File path: fe/fe-core/src/main/java/org/apache/doris/catalog/Table.java
##########
@@ -54,6 +57,8 @@
// assume that the time a lock is held by thread is less then 100ms
public static final long TRY_LOCK_TIMEOUT_MS = 100L;
+ public volatile boolean isDropped = false;
Review comment:
I think `isDropped` need to be persisted?
##########
File path:
fe/fe-core/src/main/java/org/apache/doris/http/rest/TableRowCountAction.java
##########
@@ -84,13 +84,13 @@ protected void executeWithoutPassword(BaseRequest request,
BaseResponse response
throw new DorisHttpException(HttpResponseStatus.BAD_REQUEST,
e.getMessage());
}
- table.writeLock();
+ table.readLock();
try {
OlapTable olapTable = (OlapTable) table;
resultMap.put("status", 200);
resultMap.put("size", olapTable.proximateRowCount());
} finally {
- table.writeUnlock();
+ table.readUnlock();
Review comment:
There is also a bug in
`org.apache.doris.httpv2.rest.TableRowCountAction` here.
```
olapTable.readLock();
try {
resultMap.put("status", 200);
resultMap.put("size", olapTable.proximateRowCount());
} finally {
olapTable.readLock();
}
```
Please fix it too~
##########
File path: fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java
##########
@@ -1576,7 +1592,9 @@ public void cancelInternal(boolean isReplay) {
}
LOG.info("remove restored partition in table {} when
cancelled: {}",
Review comment:
move this log after `restoreTbl.writeLockIfExist()` succeed.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
