Skip to content

Commit

Permalink
[Enhancement] Reduce lock time of schema change in scenarios with a l…
Browse files Browse the repository at this point in the history 
…arge number of columns (backport #52800) (#52842)

Co-authored-by: trueeyu <[email protected]>
  • Loading branch information
@mergify @trueeyu
mergify[bot] and trueeyu authored Nov 13, 2024
1 parent b9fdc98 commit f2abc0e
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 4 deletions.
47 changes: 44 additions & 3 deletions fe/fe-core/src/main/java/com/starrocks/alter/SchemaChangeHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import com.starrocks.catalog.constraint.ForeignKeyConstraint;
import com.starrocks.catalog.constraint.UniqueConstraint;
import com.starrocks.common.AnalysisException;
import com.starrocks.common.CaseSensibility;
import com.starrocks.common.Config;
import com.starrocks.common.DdlException;
import com.starrocks.common.ErrorCode;
Expand Down Expand Up @@ -1521,13 +1522,17 @@ private SchemaChangeData finalAnalyze(Database db, OlapTable olapTable,
}

// 2. check compatible
Map<String, Column> originSchemaMap = buildSchemaMapFromList(originSchema, true,
CaseSensibility.COLUMN.getCaseSensibility());

for (Column alterColumn : alterSchema) {
if (modifyFieldColumns.contains(alterColumn.getName())) {
continue;
}
Optional<Column> col = originSchema.stream().filter(c -> c.nameEquals(alterColumn.getName(), true)).findFirst();
if (col.isPresent() && !alterColumn.equals(col.get())) {
col.get().checkSchemaChangeAllowed(alterColumn);
Column col = getColumnFromSchemaMap(originSchemaMap, alterColumn.getName(), true,
CaseSensibility.COLUMN.getCaseSensibility());
if (col != null && !alterColumn.equals(col)) {
col.checkSchemaChangeAllowed(alterColumn);
}
}

Expand Down Expand Up @@ -1598,6 +1603,42 @@ private SchemaChangeData finalAnalyze(Database db, OlapTable olapTable,
return dataBuilder.build();
}

protected static Map<String, Column> buildSchemaMapFromList(List<Column> schema, boolean ignorePrefix,
boolean caseSensibility) {
Map<String, Column> schemaMap = Maps.newHashMap();
if (ignorePrefix) {
if (caseSensibility) {
schema.forEach(col -> schemaMap.put(Column.removeNamePrefix(col.getName()), col));
} else {
schema.forEach(col -> schemaMap.put(Column.removeNamePrefix(col.getName()).toLowerCase(), col));
}
} else {
if (caseSensibility) {
schema.forEach(col -> schemaMap.put(col.getName(), col));
} else {
schema.forEach(col -> schemaMap.put(col.getName().toLowerCase(), col));
}
}
return schemaMap;
}

protected static Column getColumnFromSchemaMap(Map<String, Column> originSchemaMap, String col,
boolean ignorePrefix, boolean caseSensibility) {
if (caseSensibility) {
if (ignorePrefix) {
return originSchemaMap.get(Column.removeNamePrefix(col));
} else {
return originSchemaMap.get(col);
}
} else {
if (ignorePrefix) {
return originSchemaMap.get(Column.removeNamePrefix(col).toLowerCase());
} else {
return originSchemaMap.get(col.toLowerCase());
}
}
}

private void checkPartitionColumnChange(OlapTable olapTable, List<Column> alterSchema, long alterIndexId)
throws DdlException {
// Since partition key and distribution key's schema change can only happen in base index,
Expand Down
46 changes: 45 additions & 1 deletion fe/fe-core/src/test/java/com/starrocks/alter/SchemaChangeHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.starrocks.catalog.MaterializedIndexMeta;
import com.starrocks.catalog.OlapTable;
import com.starrocks.catalog.OlapTable.OlapTableState;
import com.starrocks.catalog.Type;
import com.starrocks.common.Config;
import com.starrocks.common.DdlException;
import com.starrocks.common.util.concurrent.lock.LockType;
Expand Down Expand Up @@ -125,7 +126,7 @@ private void waitAlterJobDone(Map<Long, AlterJobV2> alterJobs) throws Exception
Thread.sleep(1000);
}
LOG.info("alter job {} is done. state: {}", alterJobV2.getJobId(), alterJobV2.getJobState());
Assert.assertEquals(AlterJobV2.JobState.FINISHED, alterJobV2.getJobState());
Assertions.assertEquals(AlterJobV2.JobState.FINISHED, alterJobV2.getJobState());

Database db = GlobalStateMgr.getCurrentState().getDb(alterJobV2.getDbId());
OlapTable tbl = (OlapTable) db.getTable(alterJobV2.getTableId());
Expand All @@ -135,6 +136,49 @@ private void waitAlterJobDone(Map<Long, AlterJobV2> alterJobs) throws Exception
}
}

@Test
public void testBuildSchemaMapAndGet() {
LinkedList<Column> schemaList = new LinkedList<>();
String colName1 = "__starrocks_shadow_c1";
String colName2 = "__starrocks_shadow_c2";
Column col1 = new Column(colName1, Type.INT);
Column col2 = new Column(colName2, Type.INT);
schemaList.add(col1);
schemaList.add(col2);

Map<String, Column> schemaMap = SchemaChangeHandler.buildSchemaMapFromList(schemaList, true, true);
Column col = SchemaChangeHandler.getColumnFromSchemaMap(schemaMap, "c1", true, true);
Assertions.assertEquals(col.getName(), colName1);
col = SchemaChangeHandler.getColumnFromSchemaMap(schemaMap, colName1, true, true);
Assertions.assertEquals(col.getName(), colName1);
col = SchemaChangeHandler.getColumnFromSchemaMap(schemaMap, "__starrocks_shadow_C2", true, true);
Assertions.assertNull(col);

schemaMap = SchemaChangeHandler.buildSchemaMapFromList(schemaList, true, false);
col = SchemaChangeHandler.getColumnFromSchemaMap(schemaMap, "c1", true, false);
Assertions.assertEquals(col.getName(), colName1);
col = SchemaChangeHandler.getColumnFromSchemaMap(schemaMap, colName1, true, false);
Assertions.assertEquals(col.getName(), colName1);
col = SchemaChangeHandler.getColumnFromSchemaMap(schemaMap, "__starrocks_shadow_C2", true, false);
Assertions.assertEquals(col.getName(), colName2);

schemaMap = SchemaChangeHandler.buildSchemaMapFromList(schemaList, false, true);
col = SchemaChangeHandler.getColumnFromSchemaMap(schemaMap, "c1", false, true);
Assertions.assertNull(col);
col = SchemaChangeHandler.getColumnFromSchemaMap(schemaMap, colName1, false, true);
Assertions.assertEquals(col.getName(), colName1);
col = SchemaChangeHandler.getColumnFromSchemaMap(schemaMap, "__starrocks_shadow_C2", false, true);
Assertions.assertNull(col);

schemaMap = SchemaChangeHandler.buildSchemaMapFromList(schemaList, false, false);
col = SchemaChangeHandler.getColumnFromSchemaMap(schemaMap, "c1", false, false);
Assertions.assertNull(col);
col = SchemaChangeHandler.getColumnFromSchemaMap(schemaMap, colName1, false, false);
Assertions.assertEquals(col.getName(), colName1);
col = SchemaChangeHandler.getColumnFromSchemaMap(schemaMap, "__starrocks_shadow_C2", false, false);
Assertions.assertEquals(col.getName(), colName2);
}

@Test
public void testAggAddOrDropColumn() throws Exception {
LOG.info("dbName: {}", GlobalStateMgr.getCurrentState().getLocalMetastore().listDbNames());
Expand Down

0 comments on commit f2abc0e

Please sign in to comment.