This is an automated email from the ASF dual-hosted git repository.
jakevin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new c76a1401282 [fix](Nereids): log constraint editlog in lock scope
(#30630)
c76a1401282 is described below
commit c76a14012824c7d81c467c2f41267306ce1ba7fa
Author: 谢健 <[email protected]>
AuthorDate: Tue Feb 20 14:39:39 2024 +0800
[fix](Nereids): log constraint editlog in lock scope (#30630)
---
.../java/org/apache/doris/catalog/TableIf.java | 113 +++++++++++++--------
.../trees/plans/commands/AddConstraintCommand.java | 13 +--
.../plans/commands/DropConstraintCommand.java | 5 +-
.../java/org/apache/doris/persist/EditLog.java | 2 +-
.../catalog/constraint/ConstraintPersistTest.java | 55 +++++++++-
5 files changed, 127 insertions(+), 61 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java
index 7c1943c69a6..29c7d6b83e2 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java
@@ -26,6 +26,7 @@ import org.apache.doris.cluster.ClusterNamespace;
import org.apache.doris.common.DdlException;
import org.apache.doris.common.MetaNotFoundException;
import org.apache.doris.nereids.exceptions.AnalysisException;
+import org.apache.doris.persist.AlterConstraintLog;
import org.apache.doris.statistics.AnalysisInfo;
import org.apache.doris.statistics.BaseAnalysisTask;
import org.apache.doris.statistics.ColumnStatistic;
@@ -227,7 +228,7 @@ public interface TableIf {
}
// Note this function is not thread safe
- default void checkConstraintNotExistence(String name, Constraint
primaryKeyConstraint,
+ default void checkConstraintNotExistenceUnsafe(String name, Constraint
primaryKeyConstraint,
Map<String, Constraint> constraintMap) {
if (constraintMap.containsKey(name)) {
throw new RuntimeException(String.format("Constraint name %s has
existed", name));
@@ -240,87 +241,108 @@ public interface TableIf {
}
}
- default Constraint addUniqueConstraint(String name, ImmutableList<String>
columns) {
+ default void addUniqueConstraint(String name, ImmutableList<String>
columns, boolean replay) {
writeLock();
try {
Map<String, Constraint> constraintMap = getConstraintsMapUnsafe();
UniqueConstraint uniqueConstraint = new UniqueConstraint(name,
ImmutableSet.copyOf(columns));
- checkConstraintNotExistence(name, uniqueConstraint, constraintMap);
+ checkConstraintNotExistenceUnsafe(name, uniqueConstraint,
constraintMap);
constraintMap.put(name, uniqueConstraint);
- return uniqueConstraint;
+ if (!replay) {
+ Env.getCurrentEnv().getEditLog().logAddConstraint(
+ new AlterConstraintLog(uniqueConstraint, this));
+ }
} finally {
writeUnlock();
}
}
- default Constraint addPrimaryKeyConstraint(String name,
ImmutableList<String> columns) {
+ default void addPrimaryKeyConstraint(String name, ImmutableList<String>
columns, boolean replay) {
writeLock();
try {
Map<String, Constraint> constraintMap = getConstraintsMapUnsafe();
PrimaryKeyConstraint primaryKeyConstraint = new
PrimaryKeyConstraint(name, ImmutableSet.copyOf(columns));
- checkConstraintNotExistence(name, primaryKeyConstraint,
constraintMap);
+ checkConstraintNotExistenceUnsafe(name, primaryKeyConstraint,
constraintMap);
constraintMap.put(name, primaryKeyConstraint);
- return primaryKeyConstraint;
+ if (!replay) {
+ Env.getCurrentEnv().getEditLog().logAddConstraint(
+ new AlterConstraintLog(primaryKeyConstraint, this));
+ }
} finally {
writeUnlock();
}
}
- default void updatePrimaryKeyForForeignKey(PrimaryKeyConstraint
requirePrimaryKey, TableIf referencedTable) {
- referencedTable.writeLock();
- try {
- Optional<Constraint> primaryKeyConstraint =
referencedTable.getConstraintsMapUnsafe().values().stream()
- .filter(requirePrimaryKey::equals)
- .findFirst();
- if (!primaryKeyConstraint.isPresent()) {
- throw new AnalysisException(String.format(
- "Foreign key constraint requires a primary key
constraint %s in %s",
- requirePrimaryKey.getPrimaryKeyNames(),
referencedTable.getName()));
- }
- ((PrimaryKeyConstraint)
(primaryKeyConstraint.get())).addForeignTable(this);
- } finally {
- referencedTable.writeUnlock();
+ default PrimaryKeyConstraint tryGetPrimaryKeyForForeignKeyUnsafe(
+ PrimaryKeyConstraint requirePrimaryKey, TableIf referencedTable) {
+ Optional<Constraint> primaryKeyConstraint =
referencedTable.getConstraintsMapUnsafe().values().stream()
+ .filter(requirePrimaryKey::equals)
+ .findFirst();
+ if (!primaryKeyConstraint.isPresent()) {
+ throw new AnalysisException(String.format(
+ "Foreign key constraint requires a primary key constraint
%s in %s",
+ requirePrimaryKey.getPrimaryKeyNames(),
referencedTable.getName()));
}
+ return ((PrimaryKeyConstraint) (primaryKeyConstraint.get()));
}
- default Constraint addForeignConstraint(String name, ImmutableList<String>
columns,
- TableIf referencedTable, ImmutableList<String> referencedColumns) {
+ default void addForeignConstraint(String name, ImmutableList<String>
columns,
+ TableIf referencedTable, ImmutableList<String> referencedColumns,
boolean replay) {
writeLock();
+ referencedTable.writeLock();
try {
Map<String, Constraint> constraintMap = getConstraintsMapUnsafe();
ForeignKeyConstraint foreignKeyConstraint =
new ForeignKeyConstraint(name, columns, referencedTable,
referencedColumns);
- checkConstraintNotExistence(name, foreignKeyConstraint,
constraintMap);
- PrimaryKeyConstraint requirePrimaryKey = new
PrimaryKeyConstraint(name,
+ checkConstraintNotExistenceUnsafe(name, foreignKeyConstraint,
constraintMap);
+ PrimaryKeyConstraint requirePrimaryKeyName = new
PrimaryKeyConstraint(name,
foreignKeyConstraint.getReferencedColumnNames());
- updatePrimaryKeyForForeignKey(requirePrimaryKey, referencedTable);
+ PrimaryKeyConstraint primaryKeyConstraint =
+ tryGetPrimaryKeyForForeignKeyUnsafe(requirePrimaryKeyName,
referencedTable);
+ primaryKeyConstraint.addForeignTable(this);
constraintMap.put(name, foreignKeyConstraint);
- return foreignKeyConstraint;
+ if (!replay) {
+ Env.getCurrentEnv().getEditLog().logAddConstraint(
+ new AlterConstraintLog(foreignKeyConstraint, this));
+ }
} finally {
+ referencedTable.writeUnlock();
writeUnlock();
}
}
default void replayAddConstraint(Constraint constraint) {
- if (constraint instanceof UniqueConstraint) {
- UniqueConstraint uniqueConstraint = (UniqueConstraint) constraint;
- this.addUniqueConstraint(constraint.getName(),
-
ImmutableList.copyOf(uniqueConstraint.getUniqueColumnNames()));
- } else if (constraint instanceof PrimaryKeyConstraint) {
- PrimaryKeyConstraint primaryKeyConstraint = (PrimaryKeyConstraint)
constraint;
- this.addPrimaryKeyConstraint(primaryKeyConstraint.getName(),
-
ImmutableList.copyOf(primaryKeyConstraint.getPrimaryKeyNames()));
- } else if (constraint instanceof ForeignKeyConstraint) {
- ForeignKeyConstraint foreignKey = (ForeignKeyConstraint)
constraint;
- this.addForeignConstraint(foreignKey.getName(),
- ImmutableList.copyOf(foreignKey.getForeignKeyNames()),
- foreignKey.getReferencedTable(),
-
ImmutableList.copyOf(foreignKey.getReferencedColumnNames()));
+ // Since constraints are not indispensable, we only log when replay
fails
+ try {
+ if (constraint instanceof UniqueConstraint) {
+ UniqueConstraint uniqueConstraint = (UniqueConstraint)
constraint;
+ this.addUniqueConstraint(constraint.getName(),
+
ImmutableList.copyOf(uniqueConstraint.getUniqueColumnNames()), true);
+ } else if (constraint instanceof PrimaryKeyConstraint) {
+ PrimaryKeyConstraint primaryKeyConstraint =
(PrimaryKeyConstraint) constraint;
+ this.addPrimaryKeyConstraint(primaryKeyConstraint.getName(),
+
ImmutableList.copyOf(primaryKeyConstraint.getPrimaryKeyNames()), true);
+ } else if (constraint instanceof ForeignKeyConstraint) {
+ ForeignKeyConstraint foreignKey = (ForeignKeyConstraint)
constraint;
+ this.addForeignConstraint(foreignKey.getName(),
+ ImmutableList.copyOf(foreignKey.getForeignKeyNames()),
+ foreignKey.getReferencedTable(),
+
ImmutableList.copyOf(foreignKey.getReferencedColumnNames()), true);
+ }
+ } catch (Exception e) {
+ LOG.error(e.getMessage());
}
}
- default Constraint dropConstraint(String name) {
- Constraint dropConstraint;
+ default void replayDropConstraint(String name) {
+ try {
+ dropConstraint(name, true);
+ } catch (Exception e) {
+ LOG.error(e.getMessage());
+ }
+ }
+
+ default void dropConstraint(String name, boolean replay) {
writeLock();
try {
Map<String, Constraint> constraintMap = getConstraintsMapUnsafe();
@@ -334,11 +356,12 @@ public interface TableIf {
((PrimaryKeyConstraint) constraint).getForeignTables()
.forEach(t -> t.dropFKReferringPK(this,
(PrimaryKeyConstraint) constraint));
}
- dropConstraint = constraint;
+ if (!replay) {
+ Env.getCurrentEnv().getEditLog().logDropConstraint(new
AlterConstraintLog(constraint, this));
+ }
} finally {
writeUnlock();
}
- return dropConstraint;
}
default void dropFKReferringPK(TableIf table, PrimaryKeyConstraint
constraint) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AddConstraintCommand.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AddConstraintCommand.java
index 8c90bc0f914..38bf71133c0 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AddConstraintCommand.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/AddConstraintCommand.java
@@ -17,7 +17,6 @@
package org.apache.doris.nereids.trees.plans.commands;
-import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.TableIf;
import org.apache.doris.common.Pair;
import org.apache.doris.nereids.NereidsPlanner;
@@ -30,7 +29,6 @@ import
org.apache.doris.nereids.trees.plans.commands.ExplainCommand.ExplainLevel
import org.apache.doris.nereids.trees.plans.logical.LogicalCatalogRelation;
import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
-import org.apache.doris.persist.AlterConstraintLog;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.qe.StmtExecutor;
@@ -63,19 +61,16 @@ public class AddConstraintCommand extends Command
implements ForwardWithSync {
@Override
public void run(ConnectContext ctx, StmtExecutor executor) throws
Exception {
Pair<ImmutableList<String>, TableIf> columnsAndTable =
extractColumnsAndTable(ctx, constraint.toProject());
- org.apache.doris.catalog.constraint.Constraint catalogConstraint =
null;
if (constraint.isForeignKey()) {
Pair<ImmutableList<String>, TableIf> referencedColumnsAndTable
= extractColumnsAndTable(ctx,
constraint.toReferenceProject());
- catalogConstraint =
columnsAndTable.second.addForeignConstraint(name, columnsAndTable.first,
- referencedColumnsAndTable.second,
referencedColumnsAndTable.first);
+ columnsAndTable.second.addForeignConstraint(name,
columnsAndTable.first,
+ referencedColumnsAndTable.second,
referencedColumnsAndTable.first, false);
} else if (constraint.isPrimaryKey()) {
- catalogConstraint =
columnsAndTable.second.addPrimaryKeyConstraint(name, columnsAndTable.first);
+ columnsAndTable.second.addPrimaryKeyConstraint(name,
columnsAndTable.first, false);
} else if (constraint.isUnique()) {
- catalogConstraint =
columnsAndTable.second.addUniqueConstraint(name, columnsAndTable.first);
+ columnsAndTable.second.addUniqueConstraint(name,
columnsAndTable.first, false);
}
- Env.getCurrentEnv().getEditLog().logAddConstraint(
- new AlterConstraintLog(catalogConstraint,
columnsAndTable.second));
}
private Pair<ImmutableList<String>, TableIf>
extractColumnsAndTable(ConnectContext ctx, LogicalPlan plan) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java
index 84143f234c3..9878b0e9263 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/DropConstraintCommand.java
@@ -17,7 +17,6 @@
package org.apache.doris.nereids.trees.plans.commands;
-import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.TableIf;
import org.apache.doris.nereids.NereidsPlanner;
import org.apache.doris.nereids.exceptions.AnalysisException;
@@ -28,7 +27,6 @@ import
org.apache.doris.nereids.trees.plans.commands.ExplainCommand.ExplainLevel
import org.apache.doris.nereids.trees.plans.logical.LogicalCatalogRelation;
import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
-import org.apache.doris.persist.AlterConstraintLog;
import org.apache.doris.qe.ConnectContext;
import org.apache.doris.qe.StmtExecutor;
@@ -58,8 +56,7 @@ public class DropConstraintCommand extends Command implements
ForwardWithSync {
@Override
public void run(ConnectContext ctx, StmtExecutor executor) throws
Exception {
TableIf table = extractTable(ctx, plan);
- org.apache.doris.catalog.constraint.Constraint catalogConstraint =
table.dropConstraint(name);
- Env.getCurrentEnv().getEditLog().logDropConstraint(new
AlterConstraintLog(catalogConstraint, table));
+ table.dropConstraint(name, false);
}
private TableIf extractTable(ConnectContext ctx, LogicalPlan plan) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java
b/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java
index 7f343f94692..88675d5cca3 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/persist/EditLog.java
@@ -992,7 +992,7 @@ public class EditLog {
}
case OperationType.OP_DROP_CONSTRAINT: {
final AlterConstraintLog log = (AlterConstraintLog)
journal.getData();
-
log.getTableIf().dropConstraint(log.getConstraint().getName());
+
log.getTableIf().replayDropConstraint(log.getConstraint().getName());
break;
}
case OperationType.OP_ALTER_USER: {
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintPersistTest.java
b/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintPersistTest.java
index 782073d4ac3..07c7abf7ce0 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintPersistTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/catalog/constraint/ConstraintPersistTest.java
@@ -24,6 +24,7 @@ import org.apache.doris.catalog.PrimitiveType;
import org.apache.doris.catalog.Table;
import org.apache.doris.catalog.TableIf;
import org.apache.doris.common.Config;
+import org.apache.doris.common.Pair;
import org.apache.doris.datasource.ExternalTable;
import org.apache.doris.datasource.es.EsExternalCatalog;
import org.apache.doris.datasource.es.EsExternalDatabase;
@@ -53,6 +54,7 @@ import java.io.InputStream;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.CopyOnWriteArrayList;
class ConstraintPersistTest extends TestWithFeService implements
PlanPatternMatchSupported {
@@ -144,6 +146,52 @@ class ConstraintPersistTest extends TestWithFeService
implements PlanPatternMatc
Assertions.assertTrue(tableIf.getConstraintsMap().isEmpty());
}
+ @Test
+ void replayDropConstraintLogTest() throws Exception {
+ Config.edit_log_type = "local";
+ ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+ DataOutput output = new DataOutputStream(outputStream);
+ List<Pair<Short, AlterConstraintLog>> logs = new
CopyOnWriteArrayList<>();
+ EditLog editLog = new EditLog("");
+ new MockUp<EditLog>() {
+ @Mock
+ public void logAddConstraint(AlterConstraintLog log) {
+ logs.add(Pair.of(OperationType.OP_ADD_CONSTRAINT, log));
+ }
+
+ @Mock
+ public void logDropConstraint(AlterConstraintLog log) {
+ logs.add(Pair.of(OperationType.OP_DROP_CONSTRAINT, log));
+ }
+ };
+ new MockUp<Env>() {
+ @Mock
+ public EditLog getEditLog() {
+ return editLog;
+ }
+ };
+ addConstraint("alter table t1 add constraint pk primary key (k1)");
+ addConstraint("alter table t2 add constraint pk primary key (k1)");
+ addConstraint("alter table t1 add constraint uk unique (k1)");
+ addConstraint("alter table t1 add constraint fk foreign key (k1)
references t2(k1)");
+ TableIf tableIf = RelationUtil.getTable(
+ RelationUtil.getQualifierName(connectContext,
Lists.newArrayList("test", "t1")),
+ connectContext.getEnv());
+ Assertions.assertEquals(3, tableIf.getConstraintsMap().size());
+ dropConstraint("alter table t1 drop constraint uk");
+ dropConstraint("alter table t1 drop constraint pk");
+ dropConstraint("alter table t2 drop constraint pk");
+ Assertions.assertEquals(0, tableIf.getConstraintsMap().size());
+ for (Pair<Short, AlterConstraintLog> log : logs) {
+ JournalEntity journalEntity = new JournalEntity();
+ journalEntity.setData(log.second);
+ journalEntity.setOpCode(log.first);
+ journalEntity.write(output);
+ }
+ Assertions.assertEquals(0, tableIf.getConstraintsMap().size());
+ Assertions.assertEquals(0, tableIf.getConstraintsMap().size());
+ }
+
@Test
void constraintWithTablePersistTest() throws Exception {
addConstraint("alter table t1 add constraint pk primary key (k1)");
@@ -169,8 +217,11 @@ class ConstraintPersistTest extends TestWithFeService
implements PlanPatternMatc
@Test
void externalTableTest() throws Exception {
ExternalTable externalTable = new ExternalTable();
- externalTable.addPrimaryKeyConstraint("pk", ImmutableList.of("col"));
-
+ try {
+ externalTable.addPrimaryKeyConstraint("pk",
ImmutableList.of("col"), false);
+ } catch (Exception ignore) {
+ // ignore
+ }
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
DataOutput output = new DataOutputStream(outputStream);
externalTable.write(output);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
