[BugFix] fix ingestion hang because of alter job timeout (#55207)

(cherry picked from commit 834532c)
StarRocks · Jan 20, 2025 · cf2241b · cf2241b
1 parent b81bf9b
commit cf2241b
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 16 deletions.
diff --git a/fe/fe-core/src/main/java/com/starrocks/alter/AlterJobV2.java b/fe/fe-core/src/main/java/com/starrocks/alter/AlterJobV2.java
@@ -223,8 +223,10 @@ public long getWarehouseId() {
      */
     public synchronized void run() {
         if (isTimeout()) {
-            cancelHook(cancelImpl("Timeout"));
-            return;
+            if (cancelInternal("Timeout")) {
+                // If this job can't be cancelled, we should execute it.
+                return;
+            }
         }
 
         // create connectcontext
@@ -255,15 +257,19 @@ public synchronized void run() {
                 } // else: handle the new state
             }
         } catch (AlterCancelException e) {
-            cancelHook(cancelImpl(e.getMessage()));
+            cancelInternal(e.getMessage());
         }
     }
 
+    protected boolean cancelInternal(String errMsg) {
+        boolean cancelled = cancelImpl(errMsg);
+        cancelHook(cancelled);
+        return cancelled;
+    }
+
     public boolean cancel(String errMsg) {
         synchronized (this) {
-            boolean cancelled = cancelImpl(errMsg);
-            cancelHook(cancelled);
-            return cancelled;
+            return cancelInternal(errMsg);
         }
     }
 

diff --git a/fe/fe-core/src/main/java/com/starrocks/alter/LakeTableSchemaChangeJob.java b/fe/fe-core/src/main/java/com/starrocks/alter/LakeTableSchemaChangeJob.java
@@ -938,9 +938,7 @@ public final boolean cancel(String errMsg) {
                 createReplicaLatch.countDownToZero(new Status(TStatusCode.OK, ""));
             }
             synchronized (this) {
-                boolean cancelled = cancelImpl(errMsg);
-                cancelHook(cancelled);
-                return cancelled;
+                return cancelInternal(errMsg);
             }
         } finally {
             isCancelling.set(false);

diff --git a/fe/fe-core/src/main/java/com/starrocks/alter/RollupJobV2.java b/fe/fe-core/src/main/java/com/starrocks/alter/RollupJobV2.java
@@ -770,9 +770,7 @@ public final boolean cancel(String errMsg) {
                 createReplicaLatch.countDownToZero(new Status(TStatusCode.OK, ""));
             }
             synchronized (this) {
-                boolean cancelled = cancelImpl(errMsg);
-                cancelHook(cancelled);
-                return cancelled;
+                return cancelInternal(errMsg);
             }
         } finally {
             isCancelling.set(false);

diff --git a/fe/fe-core/src/main/java/com/starrocks/alter/SchemaChangeHandler.java b/fe/fe-core/src/main/java/com/starrocks/alter/SchemaChangeHandler.java
@@ -2103,7 +2103,7 @@ public AlterJobV2 createAlterMetaJob(AlterClause alterClause, Database db, OlapT
             long timeoutSecond = PropertyAnalyzer.analyzeTimeout(properties, Config.alter_table_timeout_second);
             alterMetaJob = new LakeTableAlterMetaJob(GlobalStateMgr.getCurrentState().getNextId(),
                     db.getId(),
-                    olapTable.getId(), olapTable.getName(), timeoutSecond,
+                    olapTable.getId(), olapTable.getName(), timeoutSecond * 1000 /* should be ms*/,
                     TTabletMetaType.ENABLE_PERSISTENT_INDEX, enablePersistentIndex, persistentIndexType);
         } else {
             // shouldn't happen

diff --git a/fe/fe-core/src/main/java/com/starrocks/alter/SchemaChangeJobV2.java b/fe/fe-core/src/main/java/com/starrocks/alter/SchemaChangeJobV2.java
@@ -946,9 +946,7 @@ public final boolean cancel(String errMsg) {
                 createReplicaLatch.countDownToZero(new Status(TStatusCode.OK, ""));
             }
             synchronized (this) {
-                boolean cancelled = cancelImpl(errMsg);
-                cancelHook(cancelled);
-                return cancelled;
+                return cancelInternal(errMsg);
             }
         } finally {
             isCancelling.set(false);