This is an automated email from the ASF dual-hosted git repository.
lingmiao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-doris.git
The following commit(s) were added to refs/heads/master by this push:
new f0c5fb9 [Bug] Fix bug of NPE caused by the absence of table in replay
process. (#6136)
f0c5fb9 is described below
commit f0c5fb9f3819e700396a641ad6c5a3157d8c8f1b
Author: Mingyu Chen <morningman....@gmail.com>
AuthorDate: Fri Jul 2 19:24:13 2021 +0800
[Bug] Fix bug of NPE caused by the absence of table in replay process.
(#6136)
In the previous version, we changed the db level lock to the table level
to reduce lock contention. But this change will cause some metadata
playback problems.
Because most operations on a table will only acquire table locks. These
operations
may occur at the same time as the operation of dropping the table.
This may cause the metadata operation sequence to be inconsistent with the
log replay sequence,
which may cause some problems.
---
.../main/java/org/apache/doris/alter/Alter.java | 125 ++-------------------
.../doris/alter/MaterializedViewHandler.java | 10 +-
.../java/org/apache/doris/catalog/Catalog.java | 43 ++++++-
.../apache/doris/catalog/ColocateTableIndex.java | 10 +-
.../doris/transaction/DatabaseTransactionMgr.java | 10 ++
5 files changed, 73 insertions(+), 125 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
index 694126a..bfc6df1 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
@@ -60,13 +60,12 @@ import org.apache.doris.persist.ReplaceTableOperationLog;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.thrift.TTabletType;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-
-import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
import java.util.Arrays;
import java.util.List;
import java.util.Map;
@@ -539,119 +538,15 @@ public class Alter {
Catalog.getCurrentCatalog().getEditLog().logBatchModifyPartition(info);
}
- /**
- * Update partition's properties
- * caller should hold the db lock
- */
- public ModifyPartitionInfo modifyPartitionProperty(Database db,
- OlapTable olapTable,
- String partitionName,
- Map<String, String>
properties)
- throws DdlException {
-
Preconditions.checkArgument(olapTable.isWriteLockHeldByCurrentThread());
- if (olapTable.getState() != OlapTableState.NORMAL) {
- throw new DdlException("Table[" + olapTable.getName() + "]'s state
is not NORMAL");
- }
-
- Partition partition = olapTable.getPartition(partitionName);
- if (partition == null) {
- throw new DdlException(
- "Partition[" + partitionName + "] does not exist in
table[" + olapTable.getName() + "]");
- }
-
- PartitionInfo partitionInfo = olapTable.getPartitionInfo();
-
- // 1. data property
- DataProperty oldDataProperty =
partitionInfo.getDataProperty(partition.getId());
- DataProperty newDataProperty = null;
- try {
- newDataProperty = PropertyAnalyzer.analyzeDataProperty(properties,
oldDataProperty);
- } catch (AnalysisException e) {
- throw new DdlException(e.getMessage());
- }
- Preconditions.checkNotNull(newDataProperty);
-
- if (newDataProperty.equals(oldDataProperty)) {
- newDataProperty = null;
- }
-
- // 2. replication num
- short oldReplicationNum =
partitionInfo.getReplicationNum(partition.getId());
- short newReplicationNum = (short) -1;
- try {
- newReplicationNum =
PropertyAnalyzer.analyzeReplicationNum(properties, oldReplicationNum);
- } catch (AnalysisException e) {
- throw new DdlException(e.getMessage());
- }
-
- if (newReplicationNum == oldReplicationNum) {
- newReplicationNum = (short) -1;
- } else if
(Catalog.getCurrentColocateIndex().isColocateTable(olapTable.getId())) {
-
ErrorReport.reportDdlException(ErrorCode.ERR_COLOCATE_TABLE_MUST_HAS_SAME_REPLICATION_NUM,
oldReplicationNum);
- }
-
- // 3. in memory
- boolean isInMemory = PropertyAnalyzer.analyzeBooleanProp(properties,
- PropertyAnalyzer.PROPERTIES_INMEMORY,
partitionInfo.getIsInMemory(partition.getId()));
-
- // 4. tablet type
- TTabletType tabletType = TTabletType.TABLET_TYPE_DISK;
- try {
- tabletType = PropertyAnalyzer.analyzeTabletType(properties);
- } catch (AnalysisException e) {
- throw new DdlException(e.getMessage());
- }
-
- // check if has other undefined properties
- if (properties != null && !properties.isEmpty()) {
- Joiner.MapJoiner mapJoiner = Joiner.on(",
").withKeyValueSeparator(" = ");
- throw new DdlException("Unknown properties: " +
mapJoiner.join(properties));
- }
-
- // modify meta here
- // date property
- if (newDataProperty != null) {
- partitionInfo.setDataProperty(partition.getId(), newDataProperty);
- LOG.debug("modify partition[{}-{}-{}] data property to {}",
db.getId(), olapTable.getId(), partitionName,
- newDataProperty.toString());
- }
-
- // replication num
- if (newReplicationNum != (short) -1) {
- partitionInfo.setReplicationNum(partition.getId(),
newReplicationNum);
- LOG.debug("modify partition[{}-{}-{}] replication num to {}",
db.getId(), olapTable.getId(), partitionName,
- newReplicationNum);
- }
-
- // in memory
- if (isInMemory != partitionInfo.getIsInMemory(partition.getId())) {
- partitionInfo.setIsInMemory(partition.getId(), isInMemory);
- LOG.debug("modify partition[{}-{}-{}] in memory to {}",
db.getId(), olapTable.getId(), partitionName,
- isInMemory);
- }
-
- // tablet type
- // TODO: serialize to edit log
- if (tabletType != partitionInfo.getTabletType(partition.getId())) {
- partitionInfo.setTabletType(partition.getId(), tabletType);
- LOG.debug("modify partition[{}-{}-{}] tablet type to {}",
db.getId(), olapTable.getId(), partitionName,
- tabletType);
- }
-
- // log
- ModifyPartitionInfo info = new ModifyPartitionInfo(db.getId(),
olapTable.getId(), partition.getId(),
- newDataProperty, newReplicationNum, isInMemory);
- Catalog.getCurrentCatalog().getEditLog().logModifyPartition(info);
-
- LOG.info("finish modify partition[{}-{}-{}]", db.getId(),
olapTable.getId(), partitionName);
- return info;
- }
-
public void replayModifyPartition(ModifyPartitionInfo info) {
Database db = Catalog.getCurrentCatalog().getDb(info.getDbId());
- db.writeLock();
+ OlapTable olapTable = (OlapTable) db.getTable(info.getTableId());
+ if (olapTable == null) {
+ LOG.warn("table {} does not eixst when replaying modify partition.
db: {}", info.getTableId(), info.getDbId());
+ return;
+ }
+ olapTable.writeLock();
try {
- OlapTable olapTable = (OlapTable) db.getTable(info.getTableId());
PartitionInfo partitionInfo = olapTable.getPartitionInfo();
if (info.getDataProperty() != null) {
partitionInfo.setDataProperty(info.getPartitionId(),
info.getDataProperty());
@@ -661,7 +556,7 @@ public class Alter {
}
partitionInfo.setIsInMemory(info.getPartitionId(),
info.isInMemory());
} finally {
- db.writeUnlock();
+ olapTable.writeUnlock();
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java
b/fe/fe-core/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java
index 87c9c70..7fab577 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/alter/MaterializedViewHandler.java
@@ -61,14 +61,14 @@ import org.apache.doris.qe.OriginStatement;
import org.apache.doris.thrift.TStorageFormat;
import org.apache.doris.thrift.TStorageMedium;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@@ -834,6 +834,10 @@ public class MaterializedViewHandler extends AlterHandler {
TabletInvertedIndex invertedIndex = Catalog.getCurrentInvertedIndex();
OlapTable olapTable = (OlapTable) db.getTable(tableId);
+ if (olapTable == null) {
+ LOG.warn("table {} does not exist when replaying drop rollup. db:
{}", tableId, db.getId());
+ return;
+ }
olapTable.writeLock();
try {
for (Partition partition : olapTable.getPartitions()) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Catalog.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/Catalog.java
index 32b9057..c730f8e 100755
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Catalog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Catalog.java
@@ -3388,6 +3388,10 @@ public class Catalog {
public void replayDropPartition(DropPartitionInfo info) {
Database db = this.getDb(info.getDbId());
OlapTable olapTable = (OlapTable) db.getTable(info.getTableId());
+ if (olapTable == null) {
+ LOG.warn("table {} does not exist when replaying drop rollup. db:
{}", info.getTableId(), db.getId());
+ return;
+ }
olapTable.writeLock();
try {
if (info.isTempPartition()) {
@@ -3408,6 +3412,10 @@ public class Catalog {
long dbId = info.getDbId();
Database db = getDb(dbId);
Table table = db.getTable(info.getTableId());
+ if (table == null) {
+ LOG.warn("table {} does not exist when replaying drop rollup. db:
{}", info.getTableId(), db.getId());
+ return;
+ }
table.writeLock();
try {
Catalog.getCurrentRecycleBin().replayRecoverPartition((OlapTable)
table, info.getPartitionId());
@@ -5200,7 +5208,7 @@ public class Catalog {
}
}
- // the invoker should keep db write lock
+ // the invoker should keep table's write lock
public void modifyTableColocate(Database db, OlapTable table, String
colocateGroup, boolean isReplay,
GroupId assignedGroupId)
throws DdlException {
@@ -5285,6 +5293,11 @@ public class Catalog {
Database db = getDb(info.getGroupId().dbId);
OlapTable table = (OlapTable) db.getTable(tableId);
+ if (table == null) {
+ LOG.warn("table {} does not exist when replaying modify table
colocate. db: {}",
+ tableId, info.getGroupId().dbId);
+ return;
+ }
table.writeLock();
try {
modifyTableColocate(db, table,
properties.get(PropertyAnalyzer.PROPERTIES_COLOCATE_WITH), true,
@@ -5345,6 +5358,10 @@ public class Catalog {
Database db = getDb(dbId);
OlapTable table = (OlapTable) db.getTable(tableId);
+ if (table == null) {
+ LOG.warn("table {} does not exist when replaying rename rollup.
db: {}", tableId, dbId);
+ return;
+ }
table.writeLock();
try {
String rollupName = table.getIndexNameById(indexId);
@@ -5398,7 +5415,7 @@ public class Catalog {
}
}
- public void replayRenamePartition(TableInfo tableInfo) throws DdlException
{
+ public void replayRenamePartition(TableInfo tableInfo) {
long dbId = tableInfo.getDbId();
long tableId = tableInfo.getTableId();
long partitionId = tableInfo.getPartitionId();
@@ -5406,6 +5423,10 @@ public class Catalog {
Database db = getDb(dbId);
OlapTable table = (OlapTable) db.getTable(tableId);
+ if (table == null) {
+ LOG.warn("table {} does not exist when replaying rename partition.
db: {}", tableId, dbId);
+ return;
+ }
table.writeLock();
try {
Partition partition = table.getPartition(partitionId);
@@ -5451,7 +5472,7 @@ public class Catalog {
* @param properties
* @throws DdlException
*/
- // The caller need to hold the db write lock
+ // The caller need to hold the table's write lock
public void modifyTableReplicationNum(Database db, OlapTable table,
Map<String, String> properties) throws DdlException {
Preconditions.checkArgument(table.isWriteLockHeldByCurrentThread());
String defaultReplicationNumName = "default."+
PropertyAnalyzer.PROPERTIES_REPLICATION_NUM;
@@ -5486,7 +5507,7 @@ public class Catalog {
* @param table
* @param properties
*/
- // The caller need to hold the db write lock
+ // The caller need to hold the table's write lock
public void modifyTableDefaultReplicationNum(Database db, OlapTable table,
Map<String, String> properties) {
Preconditions.checkArgument(table.isWriteLockHeldByCurrentThread());
TableProperty tableProperty = table.getTableProperty();
@@ -5530,6 +5551,10 @@ public class Catalog {
Database db = getDb(dbId);
OlapTable olapTable = (OlapTable) db.getTable(tableId);
+ if (olapTable == null) {
+ LOG.warn("table {} does not exist when replaying modify table
property log. db: {}", tableId, dbId);
+ return;
+ }
olapTable.writeLock();
try {
TableProperty tableProperty = olapTable.getTableProperty();
@@ -6608,6 +6633,11 @@ public class Catalog {
public void replayTruncateTable(TruncateTableInfo info) {
Database db = getDb(info.getDbId());
OlapTable olapTable = (OlapTable) db.getTable(info.getTblId());
+ if (olapTable == null) {
+ LOG.warn("table {} does not exist when replaying truncate table
log. db id: {}",
+ info.getTblId(), info.getDbId());
+ return;
+ }
olapTable.writeLock();
try {
truncateTableInternal(olapTable, info.getPartitions(),
info.isEntireTable());
@@ -6759,6 +6789,11 @@ public class Catalog {
public void replayConvertDistributionType(TableInfo tableInfo) {
Database db = getDb(tableInfo.getDbId());
OlapTable tbl = (OlapTable) db.getTable(tableInfo.getTableId());
+ if (tbl == null) {
+ LOG.warn("table {} does not exist when replaying convert
distribution type. db: {}",
+ tableInfo.getTableId(), tableInfo.getDbId());
+ return;
+ }
tbl.writeLock();
try {
tbl.convertRandomDistributionToHashDistribution();
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/catalog/ColocateTableIndex.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/ColocateTableIndex.java
index dc6bf25..756d268 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/ColocateTableIndex.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/ColocateTableIndex.java
@@ -444,9 +444,13 @@ public class ColocateTableIndex implements Writable {
public void replayAddTableToGroup(ColocatePersistInfo info) {
Database db =
Catalog.getCurrentCatalog().getDb(info.getGroupId().dbId);
Preconditions.checkNotNull(db);
- OlapTable tbl = (OlapTable)db.getTable(info.getTableId());
- Preconditions.checkNotNull(tbl);
-
+ OlapTable tbl = (OlapTable) db.getTable(info.getTableId());
+ if (tbl == null) {
+ LOG.warn("table {} does not exist when replaying rename rollup.
db: {}",
+ info.getTableId(), info.getGroupId().dbId);
+ return;
+ }
+
writeLock();
try {
if (!group2BackendsPerBucketSeq.containsKey(info.getGroupId())) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/transaction/DatabaseTransactionMgr.java
b/fe/fe-core/src/main/java/org/apache/doris/transaction/DatabaseTransactionMgr.java
index a33b3ae..121ff60 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/transaction/DatabaseTransactionMgr.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/transaction/DatabaseTransactionMgr.java
@@ -1289,6 +1289,11 @@ public class DatabaseTransactionMgr {
for (TableCommitInfo tableCommitInfo :
transactionState.getIdToTableCommitInfos().values()) {
long tableId = tableCommitInfo.getTableId();
OlapTable table = (OlapTable) db.getTable(tableId);
+ if (table == null) {
+ LOG.warn("table {} does not exist when update catalog after
committed. transaction: {}, db: {}",
+ tableId, transactionState.getTransactionId(),
db.getId());
+ continue;
+ }
for (PartitionCommitInfo partitionCommitInfo :
tableCommitInfo.getIdToPartitionCommitInfo().values()) {
long partitionId = partitionCommitInfo.getPartitionId();
Partition partition = table.getPartition(partitionId);
@@ -1324,6 +1329,11 @@ public class DatabaseTransactionMgr {
for (TableCommitInfo tableCommitInfo :
transactionState.getIdToTableCommitInfos().values()) {
long tableId = tableCommitInfo.getTableId();
OlapTable table = (OlapTable) db.getTable(tableId);
+ if (table == null) {
+ LOG.warn("table {} does not exist when update catalog after
visible. transaction: {}, db: {}",
+ tableId, transactionState.getTransactionId(),
db.getId());
+ continue;
+ }
for (PartitionCommitInfo partitionCommitInfo :
tableCommitInfo.getIdToPartitionCommitInfo().values()) {
long partitionId = partitionCommitInfo.getPartitionId();
long newCommitVersion = partitionCommitInfo.getVersion();
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
