This is an automated email from the ASF dual-hosted git repository.
gavinchou 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 8ebb90998a5 [fix](fe) Treat an unfinished index change job of the same
index as a conflict in either direction (#69443)
8ebb90998a5 is described below
commit 8ebb90998a5837a195b067077613b1ef8f62de90
Author: Jack <[email protected]>
AuthorDate: Sun Oct 25 19:48:27 2026 +0800
[fix](fe) Treat an unfinished index change job of the same index as a
conflict in either direction (#69443)
Related PR: #68674
Problem Summary:
In local (non-cloud) mode, an unfinished index change job only conflicts
with a new one on the same partition when both go in the same direction.
An unfinished `BUILD INDEX idx` therefore does not block `DROP INDEX
idx` (full-table or `PARTITION`), and an unfinished `DROP INDEX idx
PARTITION (p)` does not block `BUILD INDEX idx` on `p`:
```sql
BUILD INDEX idx_v1 ON t; -- the job is still running
ALTER TABLE t DROP INDEX idx_v1; -- accepted
```
The two jobs are submitted to the schema change thread pool
independently, and the BE tablet locks only make them mutually
exclusive, not ordered. If the build runs after the drop, it writes
`idx_v1` back into the rowsets, although the table metadata no longer
has that index (or, for the opposite order, the index the user just
built is deleted again). Cloud mode already treats any unfinished job on
the partition as a conflict.
Root cause: `SchemaChangeHandler#hasIndexChangeJobOnPartition` matches
existing jobs in local mode with
`IndexChangeJob#hasSameAlterInvertedIndex(isDrop, indexes)`, which only
reports a job of the same direction.
Fix: `IndexChangeJob#hasAlterInvertedIndex(indexes)` matches a job that
builds or drops any of the indexes, in either direction, and
`hasIndexChangeJobOnPartition` drops its direction parameter. The light
`DROP INDEX` pre-check, `DROP INDEX ... PARTITION` and `BUILD INDEX` all
use this check, so each of them is rejected while a job of the same
index is unfinished on the partition, with the existing error message;
it can be retried once the job finishes.
### Release note
In non-cloud mode, `DROP INDEX` and `BUILD INDEX` are rejected while an
index change job of the same index in the other direction is unfinished
on the partition, instead of running both jobs in no particular order.
---
.../org/apache/doris/alter/IndexChangeJob.java | 14 +++---
.../apache/doris/alter/SchemaChangeHandler.java | 14 +++---
.../doris/alter/SchemaChangeHandlerTest.java | 52 ++++++++++++++++++++++
3 files changed, 65 insertions(+), 15 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/alter/IndexChangeJob.java
b/fe/fe-core/src/main/java/org/apache/doris/alter/IndexChangeJob.java
index f25f82cda90..facb618b372 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/alter/IndexChangeJob.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/alter/IndexChangeJob.java
@@ -173,13 +173,13 @@ public class IndexChangeJob implements Writable {
this.alterInvertedIndexes = alterInvertedIndexes;
}
- public boolean hasSameAlterInvertedIndex(boolean isDropOp, List<Index>
inputAlterInvertedIndexes) {
- if (this.isDropOp == isDropOp) {
- for (Index inputIndex : inputAlterInvertedIndexes) {
- for (Index existIndex : this.alterInvertedIndexes) {
- if (inputIndex.getIndexId() == existIndex.getIndexId()) {
- return true;
- }
+ // Whether this job builds or drops any of the given indexes, in either
direction: jobs of one index are
+ // not ordered, so a build running after a drop would add the dropped
index back, and the other way round.
+ public boolean hasAlterInvertedIndex(List<Index>
inputAlterInvertedIndexes) {
+ for (Index inputIndex : inputAlterInvertedIndexes) {
+ for (Index existIndex : this.alterInvertedIndexes) {
+ if (inputIndex.getIndexId() == existIndex.getIndexId()) {
+ return true;
}
}
}
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 7538e0a4f9f..463798b33b0 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
@@ -3888,7 +3888,7 @@ public class SchemaChangeHandler extends AlterHandler {
}
if (hasIndexChangeJobOnPartition(originIndexId,
db.getId(), olapTable.getId(),
- partitionName, alterIndexes, isDropOp)) {
+ partitionName, alterIndexes)) {
throw new DdlException("partition " + partitionName +
" has been built specified index."
+ " please check your build
stmt.");
}
@@ -3965,7 +3965,7 @@ public class SchemaChangeHandler extends AlterHandler {
// Cheap partition check first, the predicate compares index lists.
if (partitionNames.contains(indexChangeJob.getPartitionName())
&& isConflictingIndexChangeJob(indexChangeJob,
olapTable.getBaseIndexId(), db.getId(),
- olapTable.getId(), alterIndexes, true)) {
+ olapTable.getId(), alterIndexes)) {
throw new DdlException("partition " +
indexChangeJob.getPartitionName() + " of table "
+ olapTable.getName() + " has an unfinished index
change job, please retry after it finishes.");
}
@@ -3973,13 +3973,11 @@ public class SchemaChangeHandler extends AlterHandler {
}
public boolean hasIndexChangeJobOnPartition(
- long originIndexId, long dbId, long tableId, String partitionName,
- List<Index> alterIndexes, boolean isDrop) {
+ long originIndexId, long dbId, long tableId, String partitionName,
List<Index> alterIndexes) {
// TODO: this is temporary methods
for (IndexChangeJob indexChangeJob :
ImmutableList.copyOf(indexChangeJobs.values())) {
if (indexChangeJob.getPartitionName().equals(partitionName)
- && isConflictingIndexChangeJob(indexChangeJob,
originIndexId, dbId, tableId, alterIndexes,
- isDrop)) {
+ && isConflictingIndexChangeJob(indexChangeJob,
originIndexId, dbId, tableId, alterIndexes)) {
return true;
}
}
@@ -3987,12 +3985,12 @@ public class SchemaChangeHandler extends AlterHandler {
}
private static boolean isConflictingIndexChangeJob(IndexChangeJob
indexChangeJob, long originIndexId,
- long dbId, long tableId, List<Index> alterIndexes, boolean isDrop)
{
+ long dbId, long tableId, List<Index> alterIndexes) {
// if JobState is done (CANCELLED or FINISHED), also allow user to
create job again
return indexChangeJob.getOriginIndexId() == originIndexId
&& indexChangeJob.getDbId() == dbId
&& indexChangeJob.getTableId() == tableId
- && ((Config.isNotCloudMode() &&
indexChangeJob.hasSameAlterInvertedIndex(isDrop, alterIndexes))
+ && ((Config.isNotCloudMode() &&
indexChangeJob.hasAlterInvertedIndex(alterIndexes))
|| Config.isCloudMode())
&& !indexChangeJob.isDone();
}
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 b3ac191c658..d12a3f2a8be 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
@@ -1383,6 +1383,58 @@ public class SchemaChangeHandlerTest extends
TestWithFeService {
Assertions.assertEquals(indexChangeJobCount,
handler.getIndexChangeJobs().size());
}
+ @Test
+ public void testDropIndexRejectedByPendingBuildOfSameIndex() throws
Exception {
+ String tableName = "sc_drop_index_pending_build";
+ dropTable("test." + tableName, false);
+ createTable("CREATE TABLE test." + tableName + " (\n"
+ + "k1 INT,\n"
+ + "v1 STRING,\n"
+ + "INDEX idx_v1 (v1) USING INVERTED\n"
+ + ") ENGINE=OLAP\n"
+ + "DUPLICATE KEY(k1)\n"
+ + "PARTITION BY RANGE(k1) (PARTITION p1 VALUES LESS THAN
(\"10\"),"
+ + " PARTITION p2 VALUES LESS THAN (\"20\"))\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();
+ Index index = tbl.getIndexes().stream()
+ .filter(idx -> idx.getIndexName().equals("idx_v1"))
+ .findFirst().get();
+
+ // An unfinished BUILD INDEX idx_v1 on p1. Build and drop jobs of one
index are not ordered, so a
+ // build running after the drop would add the dropped index back to
the rowsets.
+ // 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("p1");
+ job.setAlterInvertedIndexInfo(false, Lists.newArrayList(index));
+ handler.getIndexChangeJobs().put(jobId, job);
+ try {
+ String[][] clauseAndError = {
+ {"DROP INDEX idx_v1", "has an unfinished index change
job"},
+ {"DROP INDEX idx_v1 PARTITION (p1)", "has been built
specified index"}};
+ for (String[] ce : clauseAndError) {
+ String sql = "ALTER TABLE test." + tableName + " " + ce[0];
+ Exception e = Assertions.assertThrows(Exception.class, () ->
alterTable(sql, connectContext));
+ Assertions.assertTrue(e.getMessage().contains(ce[1]),
e.getMessage());
+ }
+ Assertions.assertTrue(tbl.getIndexes().stream().anyMatch(idx ->
idx.getIndexName().equals("idx_v1")));
+
+ job.setJobState(IndexChangeJob.JobState.FINISHED);
+ alterTable("ALTER TABLE test." + tableName + " DROP INDEX idx_v1",
connectContext);
+ jobSize++;
+ Assertions.assertTrue(tbl.getIndexes().stream().noneMatch(idx ->
idx.getIndexName().equals("idx_v1")));
+ } finally {
+ handler.getIndexChangeJobs().remove(jobId);
+ }
+ }
+
@Test
public void testAddInvertedIndexStoresCanonicalBuiltinAnalyzer() throws
Exception {
createAnalyzerAliasTable("sc_ik_alias");
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
