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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
