caiconghui commented on a change in pull request #3775: URL: https://github.com/apache/incubator-doris/pull/3775#discussion_r531421620
########## File path: fe/fe-core/src/main/java/org/apache/doris/catalog/Catalog.java ########## @@ -6673,86 +6680,75 @@ public void convertDistributionType(Database db, OlapTable tbl) throws DdlExcept editLog.logModifyDistributionType(tableInfo); LOG.info("finished to modify distribution type of table: " + tbl.getName()); } finally { - db.writeUnlock(); + tbl.writeUnlock(); } } public void replayConvertDistributionType(TableInfo tableInfo) { Database db = getDb(tableInfo.getDbId()); - db.writeLock(); + OlapTable tbl = (OlapTable) db.getTable(tableInfo.getTableId()); + if (tbl == null) { + return; + } + tbl.writeLock(); try { - OlapTable tbl = (OlapTable) db.getTable(tableInfo.getTableId()); tbl.convertRandomDistributionToHashDistribution(); LOG.info("replay modify distribution type of table: " + tbl.getName()); } finally { - db.writeUnlock(); + tbl.writeUnlock(); } } /* * The entry of replacing partitions with temp partitions. */ - public void replaceTempPartition(Database db, String tableName, ReplacePartitionClause clause) throws DdlException { + public void replaceTempPartition(Database db, OlapTable olapTable, ReplacePartitionClause clause) throws DdlException { + Preconditions.checkState(olapTable.isWriteLockHeldByCurrentThread()); List<String> partitionNames = clause.getPartitionNames(); List<String> tempPartitionNames = clause.getTempPartitionNames(); boolean isStrictRange = clause.isStrictRange(); boolean useTempPartitionName = clause.useTempPartitionName(); - db.writeLock(); - try { - Table table = db.getTable(tableName); - if (table == null) { - ErrorReport.reportDdlException(ErrorCode.ERR_BAD_TABLE_ERROR, tableName); - } - - if (table.getType() != TableType.OLAP) { - throw new DdlException("Table[" + tableName + "] is not OLAP table"); + // check partition exist + for (String partName : partitionNames) { + if (!olapTable.checkPartitionNameExist(partName, false)) { Review comment: already get table lock outside replaceTempPartition ########## File path: fe/fe-core/src/main/java/org/apache/doris/catalog/Catalog.java ########## @@ -6673,86 +6680,75 @@ public void convertDistributionType(Database db, OlapTable tbl) throws DdlExcept editLog.logModifyDistributionType(tableInfo); LOG.info("finished to modify distribution type of table: " + tbl.getName()); } finally { - db.writeUnlock(); + tbl.writeUnlock(); } } public void replayConvertDistributionType(TableInfo tableInfo) { Database db = getDb(tableInfo.getDbId()); - db.writeLock(); + OlapTable tbl = (OlapTable) db.getTable(tableInfo.getTableId()); + if (tbl == null) { + return; + } + tbl.writeLock(); try { - OlapTable tbl = (OlapTable) db.getTable(tableInfo.getTableId()); tbl.convertRandomDistributionToHashDistribution(); LOG.info("replay modify distribution type of table: " + tbl.getName()); } finally { - db.writeUnlock(); + tbl.writeUnlock(); } } /* * The entry of replacing partitions with temp partitions. */ - public void replaceTempPartition(Database db, String tableName, ReplacePartitionClause clause) throws DdlException { + public void replaceTempPartition(Database db, OlapTable olapTable, ReplacePartitionClause clause) throws DdlException { + Preconditions.checkState(olapTable.isWriteLockHeldByCurrentThread()); List<String> partitionNames = clause.getPartitionNames(); List<String> tempPartitionNames = clause.getTempPartitionNames(); boolean isStrictRange = clause.isStrictRange(); boolean useTempPartitionName = clause.useTempPartitionName(); - db.writeLock(); - try { - Table table = db.getTable(tableName); - if (table == null) { - ErrorReport.reportDdlException(ErrorCode.ERR_BAD_TABLE_ERROR, tableName); - } - - if (table.getType() != TableType.OLAP) { - throw new DdlException("Table[" + tableName + "] is not OLAP table"); + // check partition exist + for (String partName : partitionNames) { + if (!olapTable.checkPartitionNameExist(partName, false)) { + throw new DdlException("Partition[" + partName + "] does not exist"); } - - OlapTable olapTable = (OlapTable) table; - // check partition exist - for (String partName : partitionNames) { - if (!olapTable.checkPartitionNameExist(partName, false)) { - throw new DdlException("Partition[" + partName + "] does not exist"); - } - } - for (String partName : tempPartitionNames) { - if (!olapTable.checkPartitionNameExist(partName, true)) { - throw new DdlException("Temp partition[" + partName + "] does not exist"); - } + } + for (String partName : tempPartitionNames) { + if (!olapTable.checkPartitionNameExist(partName, true)) { + throw new DdlException("Temp partition[" + partName + "] does not exist"); } - - olapTable.replaceTempPartitions(partitionNames, tempPartitionNames, isStrictRange, useTempPartitionName); - - // write log - ReplacePartitionOperationLog info = new ReplacePartitionOperationLog(db.getId(), olapTable.getId(), - partitionNames, tempPartitionNames, isStrictRange, useTempPartitionName); - editLog.logReplaceTempPartition(info); - LOG.info("finished to replace partitions {} with temp partitions {} from table: {}", - clause.getPartitionNames(), clause.getTempPartitionNames(), tableName); - } finally { - db.writeUnlock(); } + olapTable.replaceTempPartitions(partitionNames, tempPartitionNames, isStrictRange, useTempPartitionName); Review comment: already get table lock outside replaceTempPartition ---------------------------------------------------------------- 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