morningman commented on code in PR #23760:
URL: https://github.com/apache/doris/pull/23760#discussion_r1314262634
##########
fe/fe-core/src/main/java/org/apache/doris/alter/IndexChangeJob.java:
##########
@@ -321,10 +322,31 @@ protected void runRunningJob() throws
AlterCancelException {
LOG.info("inverted index job finished: {}", jobId);
}
+ /**
+ * cancelImpl() can be called any time any place.
+ * We need to clean any possible residual of this job.
+ */
protected boolean cancelImpl(String errMsg) {
+ if (jobState.isFinalState()) {
+ return false;
+ }
+
+ cancelInternal();
+
+ jobState = JobState.CANCELLED;
+ this.errMsg = errMsg;
+ this.finishedTimeMs = System.currentTimeMillis();
+ LOG.info("cancel index job {}, err: {}", jobId, errMsg);
Review Comment:
print log after `Env.getCurrentEnv().getEditLog().logIndexChangeJob(this);`
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java:
##########
@@ -2370,6 +2378,69 @@ public void cancel(CancelStmt stmt) throws DdlException {
}
}
+ private void cancelIndexJob(CancelAlterTableStmt cancelAlterTableStmt)
throws DdlException {
+ String dbName = cancelAlterTableStmt.getDbName();
+ String tableName = cancelAlterTableStmt.getTableName();
+ Preconditions.checkState(!Strings.isNullOrEmpty(dbName));
+ Preconditions.checkState(!Strings.isNullOrEmpty(tableName));
+
+ Database db =
Env.getCurrentInternalCatalog().getDbOrDdlException(dbName);
+
+ List<IndexChangeJob> jobList = new ArrayList<>();
+
+ OlapTable olapTable;
+ try {
+ olapTable = (OlapTable) db.getTableOrMetaException(tableName,
Table.TableType.OLAP);
+ } catch (MetaNotFoundException e) {
+ throw new DdlException(e.getMessage());
+ }
+ olapTable.writeLock();
+ try {
+ // if (olapTable.getState() != OlapTableState.SCHEMA_CHANGE
+ // && olapTable.getState() !=
OlapTableState.WAITING_STABLE) {
+ // throw new DdlException("Table[" + tableName + "] is not
under SCHEMA_CHANGE.");
+ // }
+
+ // find from index change jobs first
+ if (cancelAlterTableStmt.getAlterJobIdList() != null
+ && cancelAlterTableStmt.getAlterJobIdList().size() > 0) {
+ for (Long jobId : cancelAlterTableStmt.getAlterJobIdList()) {
+ IndexChangeJob job = indexChangeJobs.get(jobId);
+ if (job == null) {
+ continue;
+ }
+ jobList.add(job);
+ LOG.debug("add build index job {} on table {} for specific
id", jobId, tableName);
+ }
+ } else {
+ for (IndexChangeJob job : indexChangeJobs.values()) {
+ if (!job.isDone() && job.getTableId() ==
olapTable.getId()) {
+ jobList.add(job);
+ LOG.debug("add build index job {} on table {} for
all", job.getJobId(), tableName);
+ }
+ }
+ }
+ } finally {
+ olapTable.writeUnlock();
+ }
+
+ // alter job v2's cancel must be called outside the table lock
+ if (jobList.size() > 0) {
+ for (IndexChangeJob job : jobList) {
+ long jobId = job.getJobId();
+ LOG.debug("cancel build index job {} on table {}", jobId,
tableName);
+ if (!job.cancel("user cancelled")) {
+ LOG.info("cancel build index job {} on table {} failed",
jobId, tableName);
Review Comment:
```suggestion
LOG.warn("cancel build index job {} on table {} failed",
jobId, tableName);
```
##########
fe/fe-core/src/main/java/org/apache/doris/alter/IndexChangeJob.java:
##########
@@ -321,10 +322,31 @@ protected void runRunningJob() throws
AlterCancelException {
LOG.info("inverted index job finished: {}", jobId);
}
+ /**
+ * cancelImpl() can be called any time any place.
+ * We need to clean any possible residual of this job.
+ */
protected boolean cancelImpl(String errMsg) {
+ if (jobState.isFinalState()) {
+ return false;
+ }
+
+ cancelInternal();
+
+ jobState = JobState.CANCELLED;
+ this.errMsg = errMsg;
+ this.finishedTimeMs = System.currentTimeMillis();
+ LOG.info("cancel index job {}, err: {}", jobId, errMsg);
Review Comment:
Missing `replayCancelled()`
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeHandler.java:
##########
@@ -2370,6 +2378,69 @@ public void cancel(CancelStmt stmt) throws DdlException {
}
}
+ private void cancelIndexJob(CancelAlterTableStmt cancelAlterTableStmt)
throws DdlException {
+ String dbName = cancelAlterTableStmt.getDbName();
+ String tableName = cancelAlterTableStmt.getTableName();
+ Preconditions.checkState(!Strings.isNullOrEmpty(dbName));
+ Preconditions.checkState(!Strings.isNullOrEmpty(tableName));
+
+ Database db =
Env.getCurrentInternalCatalog().getDbOrDdlException(dbName);
+
+ List<IndexChangeJob> jobList = new ArrayList<>();
+
+ OlapTable olapTable;
+ try {
+ olapTable = (OlapTable) db.getTableOrMetaException(tableName,
Table.TableType.OLAP);
Review Comment:
You can use `getTableOrException`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
