This is an automated email from the ASF dual-hosted git repository.
airborne12 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 2fb774e8cd1 [fix](fe) Check conflicting index change jobs before
dropping index metadata (#68674)
2fb774e8cd1 is described below
commit 2fb774e8cd11e737a328cf8f3656453b0259dd82
Author: Jack <[email protected]>
AuthorDate: Thu Oct 22 10:54:55 2026 +0800
[fix](fe) Check conflicting index change jobs before dropping index
metadata (#68674)
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
On the light index change path, `DROP INDEX` can fail with `partition
xxx has been built specified index` when an unfinished IndexChangeJob
conflicts on the same partition. Although the statement returns an
error, the index metadata has already been removed and `SHOW INDEX` no
longer lists it, so the client cannot tell whether the DDL took effect.
Reproduce:
- Cloud mode (any unfinished index change job on the partition
conflicts): create a table, then quickly run `CREATE INDEX iv ... USING
INVERTED; DROP INDEX iv; CREATE INDEX ng ... USING NGRAM_BF; DROP INDEX
ng;`. The last `DROP INDEX` fails, yet `SHOW INDEX` is empty afterwards.
- Local mode (only a job for the same index and direction conflicts):
`ALTER TABLE t DROP INDEX idx PARTITION (p1)` and, while that job is
unfinished, `ALTER TABLE t DROP INDEX idx`.
Root cause: in `SchemaChangeHandler#modifyTableLightSchemaChange`, a
light `DROP INDEX` first calls `updateBaseIndexSchema` and writes the
edit log, and only afterwards submits its delete jobs in
`buildOrDeleteTableInvertedIndices`, which runs the conflict check
(`hasIndexChangeJobOnPartition`) and, in cloud mode, the compute group
lookup (`getCloudClusterName`). When either throws, the metadata change
is not rolled back.
Fix: `SchemaChangeHandler#process` runs both checks for every full-table
`DROP INDEX` that takes the light index change path before any ALTER
clause is processed, so a rejected statement leaves the table unchanged.
They have to run before the clause loop rather than just before
`updateBaseIndexSchema`, because clauses such as `DROP COLUMN` update
table metadata in place (the bloom filter columns) while they are
processed. The light-drop eligibility rule is extracted into
`isLightDropIndexSupported` and shared by the pre-check and the clause
loop. The conflict check reuses `hasIndexChangeJobOnPartition` with the
existing error message; the later check in
`buildOrDeleteTableInvertedIndices` is unchanged.
The same pre-check also rejects a light `ADD INDEX` and a light
full-table `DROP INDEX` in one statement (`Can not add and drop indexes
in one ALTER statement, please run them in separate statements.`). The
light path builds the delete jobs of all altered indexes with one
direction flag decided by the last index clause, so `DROP INDEX old, ADD
INDEX new` left the files of `old` behind and `ADD INDEX new, DROP INDEX
old` also deleted the files of `new`. Only the case where both clauses
take the light path is rejected; a statement whose ADD or DROP is not
light keeps its behavior (e.g. replacing NGRAM_BF with BLOOMFILTER in
local mode).
---
.../apache/doris/alter/SchemaChangeHandler.java | 78 +++++++++++++++----
.../doris/alter/SchemaChangeHandlerTest.java | 90 ++++++++++++++++++++++
2 files changed, 153 insertions(+), 15 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
index 5afc1db6b30..5d8f17fbef4 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java
@@ -2388,6 +2388,11 @@ public class SchemaChangeHandler extends AlterHandler {
}
}
+ // A light DROP INDEX publishes the index metadata before it
submits its delete jobs. Run the checks
+ // that can reject those jobs before any clause below touches
table metadata (processDropColumn
+ // updates the bloom filter columns in place), so a rejected
statement leaves the table unchanged.
+ checkLightDropIndexPreconditions(db, olapTable, alterOps,
enableAddIndexForNewData);
+
for (AlterOp alterOp : alterOps) {
Map<String, String> properties = alterOp.getProperties();
// alter table properties
@@ -2641,21 +2646,7 @@ public class SchemaChangeHandler extends AlterHandler {
break;
}
}
- // Inverted index supports light schema change in both
cloud and local mode.
- // NGRAM_BF and BfIndex support it only in cloud mode.
- boolean supportLightIndexChange = false;
- if (found != null) {
- if (Config.isCloudMode()) {
- supportLightIndexChange =
enableAddIndexForNewData
- && (found.getIndexType() ==
IndexType.NGRAM_BF
- || found.getIndexType() ==
IndexType.BLOOMFILTER
- || found.getIndexType() ==
IndexType.INVERTED);
- } else {
- supportLightIndexChange = found.getIndexType()
== IndexType.INVERTED
- || found.getIndexType() ==
IndexType.ANN;
- }
- }
- if (supportLightIndexChange) {
+ if (found != null && isLightDropIndexSupported(found,
enableAddIndexForNewData)) {
alterIndexes.add(found);
isDropIndex = true;
lightIndexChange = true;
@@ -3921,6 +3912,63 @@ public class SchemaChangeHandler extends AlterHandler {
}
}
+ // Inverted index supports light schema change in both cloud and local
mode.
+ // NGRAM_BF and BfIndex support it only in cloud mode.
+ private static boolean isLightDropIndexSupported(Index index, boolean
enableAddIndexForNewData) {
+ if (Config.isCloudMode()) {
+ return enableAddIndexForNewData
+ && (index.getIndexType() == IndexType.NGRAM_BF
+ || index.getIndexType() == IndexType.BLOOMFILTER
+ || index.getIndexType() == IndexType.INVERTED);
+ }
+ return index.getIndexType() == IndexType.INVERTED ||
index.getIndexType() == IndexType.ANN;
+ }
+
+ // Throws if a full-table DROP INDEX in alterOps takes the light index
change path and would then fail to
+ // submit its delete jobs: no compute group, or an unfinished index change
job on the dropped index.
+ // Also rejects a light DROP INDEX together with a light ADD INDEX: their
delete jobs carry one direction
+ // for all altered indexes, so the added index would be deleted or the
dropped one would be kept.
+ private void checkLightDropIndexPreconditions(Database db, OlapTable
olapTable, List<AlterOp> alterOps,
+ boolean enableAddIndexForNewData) throws UserException {
+ if (!Config.enable_light_index_change) {
+ return;
+ }
+ List<Index> droppedIndexes = new ArrayList<>();
+ for (AlterOp alterOp : alterOps) {
+ if (!(alterOp instanceof DropIndexOp) || ((DropIndexOp)
alterOp).hasPartitionSpec()) {
+ continue;
+ }
+ String indexName = ((DropIndexOp) alterOp).getIndexName();
+ olapTable.getIndexes().stream()
+ .filter(index ->
index.getIndexName().equalsIgnoreCase(indexName))
+ .filter(index -> isLightDropIndexSupported(index,
enableAddIndexForNewData))
+ .findFirst()
+ .ifPresent(droppedIndexes::add);
+ }
+ if (droppedIndexes.isEmpty()) {
+ return;
+ }
+ if (alterOps.stream().anyMatch(alterOp -> alterOp instanceof
CreateIndexOp
+ && ((CreateIndexOp)
alterOp).getIndex().isLightAddIndexSupported(enableAddIndexForNewData))) {
+ throw new DdlException("Can not add and drop indexes in one ALTER
statement,"
+ + " please run them in separate statements.");
+ }
+ getCloudClusterName();
+ checkNoIndexChangeJobOnPartitions(db, olapTable, droppedIndexes,
olapTable.getPartitionNames());
+ }
+
+ // Throws if any of the partitions still has an unfinished index change
job on the base index.
+ private void checkNoIndexChangeJobOnPartitions(Database db, OlapTable
olapTable, List<Index> alterIndexes,
+ Set<String> partitionNames) throws DdlException {
+ for (String partitionName : partitionNames) {
+ if (hasIndexChangeJobOnPartition(olapTable.getBaseIndexId(),
db.getId(), olapTable.getId(),
+ partitionName, alterIndexes, true)) {
+ throw new DdlException("partition " + partitionName + " has
been built specified index."
+ + " please check your build stmt.");
+ }
+ }
+ }
+
public boolean hasIndexChangeJobOnPartition(
long originIndexId, long dbId, long tableId, String partitionName,
List<Index> alterIndexes, boolean isDrop) {
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java
b/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java
index 6ea6512b0db..1189f5a5903 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/alter/SchemaChangeHandlerTest.java
@@ -1293,6 +1293,96 @@ public class SchemaChangeHandlerTest extends
TestWithFeService {
}
}
+ @Test
+ public void testDropIndexRejectedByIndexChangeJobKeepsMetadata() throws
Exception {
+ String tableName = "sc_drop_index_conflict";
+ dropTable("test." + tableName, false);
+ createTable("CREATE TABLE test." + tableName + " (\n"
+ + "k1 INT,\n"
+ + "v1 STRING,\n"
+ + "v2 STRING,\n"
+ + "INDEX idx_v1 (v1) USING INVERTED\n"
+ + ") ENGINE=OLAP\n"
+ + "DUPLICATE KEY(k1)\n"
+ + "DISTRIBUTED BY HASH(k1) BUCKETS 1\n"
+ + "PROPERTIES ('replication_num' = '1', 'light_schema_change'
= 'true', "
+ + "'bloom_filter_columns' = 'v2');");
+
+ Database db =
Env.getCurrentInternalCatalog().getDbOrMetaException("test");
+ OlapTable tbl = (OlapTable) db.getTableOrMetaException(tableName,
Table.TableType.OLAP);
+ SchemaChangeHandler handler =
Env.getCurrentEnv().getSchemaChangeHandler();
+ Index index = tbl.getIndexes().stream()
+ .filter(idx -> idx.getIndexName().equals("idx_v1"))
+ .findFirst().get();
+ int schemaVersion =
tbl.getIndexMetaByIndexId(tbl.getBaseIndexId()).getSchemaVersion();
+
+ // An unfinished drop job on the same index, e.g. left by DROP INDEX
... PARTITION.
+ // It is only registered in indexChangeJobs so that the job scheduler
does not run or cancel it.
+ long jobId = Env.getCurrentEnv().getNextId();
+ IndexChangeJob job = new IndexChangeJob(jobId, db.getId(),
tbl.getId(), tbl.getName(), 1000L, 0,
+ Lists.newArrayList(), Lists.newArrayList());
+ job.setOriginIndexId(tbl.getBaseIndexId());
+ job.setPartitionName(tbl.getPartitionNames().iterator().next());
+ job.setAlterInvertedIndexInfo(true, Lists.newArrayList(index));
+ handler.getIndexChangeJobs().put(jobId, job);
+ try {
+ String dropIndexStmt = "ALTER TABLE test." + tableName + " DROP
INDEX idx_v1";
+ Exception e = Assertions.assertThrows(Exception.class, () ->
alterTable(dropIndexStmt, connectContext));
+ Assertions.assertTrue(e.getMessage().contains("has been built
specified index"), e.getMessage());
+ Assertions.assertTrue(tbl.getIndexes().stream().anyMatch(idx ->
idx.getIndexName().equals("idx_v1")));
+ Assertions.assertEquals(schemaVersion,
tbl.getIndexMetaByIndexId(tbl.getBaseIndexId()).getSchemaVersion());
+
+ // The check must run before DROP COLUMN updates the bloom filter
columns in place.
+ String dropColumnAndIndexStmt = "ALTER TABLE test." + tableName +
" DROP COLUMN v2, DROP INDEX idx_v1";
+ e = Assertions.assertThrows(Exception.class, () ->
alterTable(dropColumnAndIndexStmt, connectContext));
+ Assertions.assertTrue(e.getMessage().contains("has been built
specified index"), e.getMessage());
+ Assertions.assertEquals(Sets.newHashSet("v2"),
tbl.getCopiedBfColumns());
+ Assertions.assertNotNull(tbl.getColumn("v2"));
+
+ job.setJobState(IndexChangeJob.JobState.FINISHED);
+ alterTable(dropIndexStmt, connectContext);
+ jobSize++;
+ Assertions.assertTrue(tbl.getIndexes().stream().noneMatch(idx ->
idx.getIndexName().equals("idx_v1")));
+ } finally {
+ handler.getIndexChangeJobs().remove(jobId);
+ }
+ }
+
+ @Test
+ public void testLightAddAndDropIndexInOneStatementRejected() throws
Exception {
+ String tableName = "sc_light_add_and_drop_index";
+ dropTable("test." + tableName, false);
+ createTable("CREATE TABLE test." + tableName + " (\n"
+ + "k1 INT,\n"
+ + "v1 STRING,\n"
+ + "v2 STRING,\n"
+ + "INDEX idx_v1 (v1) USING INVERTED\n"
+ + ") ENGINE=OLAP\n"
+ + "DUPLICATE KEY(k1)\n"
+ + "DISTRIBUTED BY HASH(k1) BUCKETS 1\n"
+ + "PROPERTIES ('replication_num' = '1', 'light_schema_change'
= 'true');");
+
+ Database db =
Env.getCurrentInternalCatalog().getDbOrMetaException("test");
+ OlapTable tbl = (OlapTable) db.getTableOrMetaException(tableName,
Table.TableType.OLAP);
+ SchemaChangeHandler handler =
Env.getCurrentEnv().getSchemaChangeHandler();
+ int schemaVersion =
tbl.getIndexMetaByIndexId(tbl.getBaseIndexId()).getSchemaVersion();
+ int indexChangeJobCount = handler.getIndexChangeJobs().size();
+
+ // Both clauses take the light index change path, whose delete jobs
carry a single direction.
+ for (String clauses : Lists.newArrayList(
+ "DROP INDEX idx_v1, ADD INDEX idx_v2 (v2) USING INVERTED",
+ "ADD INDEX idx_v2 (v2) USING INVERTED, DROP INDEX idx_v1")) {
+ String sql = "ALTER TABLE test." + tableName + " " + clauses;
+ Exception e = Assertions.assertThrows(Exception.class, () ->
alterTable(sql, connectContext));
+ Assertions.assertTrue(e.getMessage().contains("Can not add and
drop indexes in one ALTER statement"),
+ e.getMessage());
+ }
+ Assertions.assertEquals(Lists.newArrayList("idx_v1"),
+
tbl.getIndexes().stream().map(Index::getIndexName).collect(Collectors.toList()));
+ Assertions.assertEquals(schemaVersion,
tbl.getIndexMetaByIndexId(tbl.getBaseIndexId()).getSchemaVersion());
+ Assertions.assertEquals(indexChangeJobCount,
handler.getIndexChangeJobs().size());
+ }
+
@Test
public void testAddInvertedIndexStoresCanonicalBuiltinAnalyzer() throws
Exception {
createAnalyzerAliasTable("sc_ik_alias");
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
