caiconghui commented on a change in pull request #3775:
URL: https://github.com/apache/incubator-doris/pull/3775#discussion_r531487397
##########
File path: fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
##########
@@ -232,54 +223,61 @@ private void processAlterExternalTable(AlterTableStmt
stmt, Table externalTable,
List<AlterClause> alterClauses = stmt.getOps();
AlterOperations currentAlterOps = new AlterOperations();
currentAlterOps.checkConflict(alterClauses);
-
if (currentAlterOps.hasRenameOp()) {
processRename(db, externalTable, alterClauses);
} else if (currentAlterOps.hasSchemaChangeOp()) {
- schemaChangeHandler.processExternalTable(alterClauses, db,
externalTable);
+ externalTable.writeLock();
+ try {
+ schemaChangeHandler.processExternalTable(alterClauses, db,
externalTable);
+ } finally {
+ externalTable.writeUnlock();
+ }
}
}
public void processAlterTable(AlterTableStmt stmt) throws UserException {
TableName dbTableName = stmt.getTbl();
String dbName = dbTableName.getDb();
+ String tableName = dbTableName.getTbl();
final String clusterName = stmt.getClusterName();
Database db = Catalog.getCurrentCatalog().getDb(dbName);
if (db == null) {
ErrorReport.reportDdlException(ErrorCode.ERR_BAD_DB_ERROR, dbName);
}
+ Table table = db.getTable(tableName);
+ if (table == null) {
+ ErrorReport.reportDdlException(ErrorCode.ERR_BAD_TABLE_ERROR,
tableName);
+ }
List<AlterClause> alterClauses = Lists.newArrayList();
+ // some operations will take long time to process, need to be done
outside the table lock
+ boolean needProcessOutsideTableLock = false;
- // some operations will take long time to process, need to be done
outside the database lock
- boolean needProcessOutsideDatabaseLock = false;
- String tableName = dbTableName.getTbl();
- db.writeLock();
- try {
- Table table = db.getTable(tableName);
- if (table == null) {
- ErrorReport.reportDdlException(ErrorCode.ERR_BAD_TABLE_ERROR,
tableName);
- }
+ // check conflict alter ops first
+ AlterOperations currentAlterOps = new AlterOperations();
+ currentAlterOps.checkConflict(alterClauses);
+ // check cluster capacity and db quota outside table lock to escape
dead lock, only need to check once.
+ if (currentAlterOps.needCheckCapacity()) {
+ Catalog.getCurrentSystemInfo().checkClusterCapacity(clusterName);
+ db.checkQuota();
+ }
- switch (table.getType()) {
- case OLAP:
- OlapTable olapTable = (OlapTable) table;
- needProcessOutsideDatabaseLock =
processAlterOlapTable(stmt, olapTable, alterClauses, clusterName, db);
- break;
- case ODBC:
- case MYSQL:
- case ELASTICSEARCH:
- processAlterExternalTable(stmt, table, db);
- return;
- default:
- throw new DdlException("Do not support alter " +
table.getType().toString() + " table[" + tableName + "]");
- }
- } finally {
- db.writeUnlock();
+ switch (table.getType()) {
Review comment:
some function need db lock and table lock but some function only need
table lock, what't more, If the nesting function is too deep， it is easy to
repeat locking， if we lock small scope for modificaion， which will be more
clear although we may write many lock code
----------------------------------------------------------------
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
