Skip to content

Commit

Permalink
[BugFix] fix reload FK of table failure (StarRocks#55126)
Browse files Browse the repository at this point in the history 
Signed-off-by: Murphy <[email protected]>
  • Loading branch information
@murphyatwork
murphyatwork authored Jan 17, 2025
1 parent 5cc4e6a commit 50c805e
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 15 deletions.
5 changes: 2 additions & 3 deletions fe/fe-core/src/main/java/com/starrocks/catalog/OlapTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3105,8 +3105,7 @@ public List<ForeignKeyConstraint> getForeignKeyConstraints() {
*/
@Override
public boolean hasForeignKeyConstraints() {
return tableProperty != null && tableProperty.getForeignKeyConstraints() != null &&
!tableProperty.getForeignKeyConstraints().isEmpty();
return tableProperty != null && CollectionUtils.isNotEmpty(getForeignKeyConstraints());
}

@Override
Expand Down Expand Up @@ -3451,7 +3450,7 @@ public Map<String, String> getCommonProperties() {

// foreign key constraint
String foreignKeyConstraint = tableProperties.get(PropertyAnalyzer.PROPERTIES_FOREIGN_KEY_CONSTRAINT);
if (!Strings.isNullOrEmpty(foreignKeyConstraint)) {
if (!Strings.isNullOrEmpty(foreignKeyConstraint) && hasForeignKeyConstraints()) {
properties.put(PropertyAnalyzer.PROPERTIES_FOREIGN_KEY_CONSTRAINT,
ForeignKeyConstraint.getShowCreateTableConstraintDesc(this, getForeignKeyConstraints()));
}
Expand Down
28 changes: 28 additions & 0 deletions fe/fe-core/src/main/java/com/starrocks/server/GlobalStateMgr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
import com.starrocks.common.util.LogUtil;
import com.starrocks.common.util.PropertyAnalyzer;
import com.starrocks.common.util.SmallFileMgr;
import com.starrocks.common.util.UUIDUtil;
import com.starrocks.common.util.Util;
import com.starrocks.common.util.concurrent.QueryableReentrantLock;
import com.starrocks.common.util.concurrent.lock.LockManager;
Expand Down Expand Up @@ -1641,9 +1642,36 @@ public void loadImage(String imageDir) throws IOException {
}

private void postLoadImage() {
onReloadTables();
processMvRelatedMeta();
}

/**
* Call Table::onReload after load all tables, some properties like FK may depend on other databases/catalogs
*/
private void onReloadTables() {
TemporaryTableMgr temporaryTableMgr = GlobalStateMgr.getCurrentState().getTemporaryTableMgr();
List<String> dbNames = metadataMgr.listDbNames(InternalCatalog.DEFAULT_INTERNAL_CATALOG_NAME);
for (String dbName : dbNames) {
Database db = metadataMgr.getDb(InternalCatalog.DEFAULT_INTERNAL_CATALOG_NAME, dbName);
if (db == null) {
continue;
}
for (Table table : db.getTables()) {
try {
table.onReload();

if (table.isTemporaryTable()) {
temporaryTableMgr.addTemporaryTable(UUIDUtil.genUUID(), db.getId(), table.getName(),
table.getId());
}
} catch (Throwable e) {
LOG.error("reload table failed: {}", table, e);
}
}
}
}

private void processMvRelatedMeta() {
List<String> dbNames = metadataMgr.listDbNames(InternalCatalog.DEFAULT_INTERNAL_CATALOG_NAME);

Expand Down
12 changes: 0 additions & 12 deletions fe/fe-core/src/main/java/com/starrocks/server/LocalMetastore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@
import com.starrocks.common.util.DynamicPartitionUtil;
import com.starrocks.common.util.PropertyAnalyzer;
import com.starrocks.common.util.TimeUtils;
import com.starrocks.common.util.UUIDUtil;
import com.starrocks.common.util.Util;
import com.starrocks.common.util.concurrent.CountingLatch;
import com.starrocks.common.util.concurrent.lock.LockType;
Expand Down Expand Up @@ -5069,17 +5068,6 @@ public void load(SRMetaBlockReader reader) throws IOException, SRMetaBlockExcept
idToDb.put(db.getId(), db);
fullNameToDb.put(db.getFullName(), db);
stateMgr.getGlobalTransactionMgr().addDatabaseTransactionMgr(db.getId());
db.getTables().stream().filter(tbl -> !tbl.isMaterializedView()).forEach(tbl -> {
try {
tbl.onReload();
if (tbl.isTemporaryTable()) {
TemporaryTableMgr temporaryTableMgr = GlobalStateMgr.getCurrentState().getTemporaryTableMgr();
temporaryTableMgr.addTemporaryTable(UUIDUtil.genUUID(), db.getId(), tbl.getName(), tbl.getId());
}
} catch (Throwable e) {
LOG.error("reload table failed: {}", tbl, e);
}
});
});

AutoIncrementInfo autoIncrementInfo = reader.readJson(AutoIncrementInfo.class);
Expand Down
45 changes: 45 additions & 0 deletions fe/fe-core/src/test/java/com/starrocks/server/GlobalStateMgrTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import com.sleepycat.je.rep.ReplicaStateException;
import com.sleepycat.je.rep.UnknownMasterException;
import com.sleepycat.je.rep.util.ReplicationGroupAdmin;
import com.starrocks.catalog.Table;
import com.starrocks.common.Config;
import com.starrocks.common.DdlException;
import com.starrocks.common.Pair;
Expand All @@ -52,8 +53,11 @@
import com.starrocks.persist.ImageFormatVersion;
import com.starrocks.persist.ImageWriter;
import com.starrocks.persist.OperationType;
import com.starrocks.qe.ConnectContext;
import com.starrocks.sql.ast.ModifyFrontendAddressClause;
import com.starrocks.sql.ast.UserIdentity;
import com.starrocks.system.Frontend;
import com.starrocks.utframe.StarRocksAssert;
import com.starrocks.utframe.UtFrameUtils;
import mockit.Expectations;
import mockit.Mock;
Expand All @@ -65,6 +69,8 @@
import org.mockito.Mockito;

import java.lang.reflect.Field;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
Expand Down Expand Up @@ -310,4 +316,43 @@ public void testErrorOccursWhileRemovingClusterIdAndRoleWhenStartAtFirstTime() {
Assert.assertEquals(removeFileErrorMessage, suppressedExceptions[0].getMessage());
}
}

@Test
public void testReloadTables() throws Exception {
ConnectContext ctx = UtFrameUtils.initCtxForNewPrivilege(UserIdentity.ROOT);
UtFrameUtils.createMinStarRocksCluster();
UtFrameUtils.setUpForPersistTest();
GlobalStateMgr currentState = GlobalStateMgr.getCurrentState();
StarRocksAssert starRocksAssert = new StarRocksAssert();

currentState.getLocalMetastore().createDb("db1");
currentState.getLocalMetastore().createDb("db2");
{
String sql = "create table db1.t1(c1 int not null, c2 int) " +
"properties('replication_num'='1', 'unique_constraints'='c1') ";
starRocksAssert.withTable(sql);
}
{
String sql = "create table db2.t1(c1 int, c2 int) properties('replication_num'='1'," +
"'foreign_key_constraints'='(c1) REFERENCES db1.t1(c1)')";
starRocksAssert.withTable(sql);
}

// move image file
String imagePath = currentState.dumpImage();
Path targetPath = Path.of(Config.meta_dir, GlobalStateMgr.IMAGE_DIR, "/v2",
Path.of(imagePath).getFileName().toString());
Files.move(Path.of(imagePath), targetPath);
// move checksum file
Path checksumPath = Path.of(Config.meta_dir, "checksum.0");
Path checksumTarget = Path.of(Config.meta_dir, GlobalStateMgr.IMAGE_DIR, "/v2", "checksum.0");
Files.move(checksumPath, checksumTarget);

GlobalStateMgr newState = new MyGlobalStateMgr(false);
newState.loadImage(Config.meta_dir + GlobalStateMgr.IMAGE_DIR);
Table table = newState.getLocalMetastore().getTable("db1", "t1");
Assert.assertNotNull(table);
table = newState.getLocalMetastore().getTable("db2", "t1");
Assert.assertEquals(1, table.getForeignKeyConstraints().size());
}
}

0 comments on commit 50c805e

Please sign in to comment.