(amoro) branch master updated: [Hotfix] Throw exception and ignore result when optimizing commit conflict (#2720)

[baiyangtx]

This is an automated email from the ASF dual-hosted git repository.

baiyangtx pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/amoro.git


The following commit(s) were added to refs/heads/master by this push:
     new 8b9bd5767 [Hotfix] Throw exception and ignore result when optimizing 
commit conflict (#2720)
8b9bd5767 is described below

commit 8b9bd5767d51c4fb5b58f3b418c66bfd72193513
Author: baiyangtx <xiangneb...@163.com>
AuthorDate: Mon Apr 15 10:38:52 2024 +0800

    [Hotfix] Throw exception and ignore result when optimizing commit conflict 
(#2720)
    
    * fix optimizing commit status handle
    
    * fix reviews
    
    ---------
    
    Co-authored-by: ZhouJinsong <zhoujinsong0...@163.com>
---
 .../arctic/server/optimizing/OptimizingQueue.java  | 30 +++++++++++-----------
 .../server/optimizing/TestOptimizingQueue.java     |  6 ++---
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git 
a/ams/server/src/main/java/com/netease/arctic/server/optimizing/OptimizingQueue.java
 
b/ams/server/src/main/java/com/netease/arctic/server/optimizing/OptimizingQueue.java
index 833a06df7..bb08695cf 100644
--- 
a/ams/server/src/main/java/com/netease/arctic/server/optimizing/OptimizingQueue.java
+++ 
b/ams/server/src/main/java/com/netease/arctic/server/optimizing/OptimizingQueue.java
@@ -550,23 +550,23 @@ public class OptimizingQueue extends PersistentBase {
           LOG.warn("{} has already committed, give up", 
tableRuntime.getTableIdentifier());
           throw new IllegalStateException("repeat commit, and last error " + 
failedReason);
         }
-        hasCommitted = true;
-        buildCommit().commit();
-        status = Status.SUCCESS;
-        endTime = System.currentTimeMillis();
-        persistProcessCompleted(true);
-      } catch (Exception e) {
-        LOG.error("{} Commit optimizing failed ", 
tableRuntime.getTableIdentifier(), e);
-        status = Status.FAILED;
-        failedReason = ExceptionUtil.getErrorMessage(e, 4000);
-        endTime = System.currentTimeMillis();
-        persistProcessCompleted(false);
+        try {
+          hasCommitted = true;
+          buildCommit().commit();
+          status = Status.SUCCESS;
+          endTime = System.currentTimeMillis();
+          persistProcessCompleted(true);
+        } catch (Exception e) {
+          LOG.error("{} Commit optimizing failed ", 
tableRuntime.getTableIdentifier(), e);
+          status = Status.FAILED;
+          failedReason = ExceptionUtil.getErrorMessage(e, 4000);
+          endTime = System.currentTimeMillis();
+          persistProcessCompleted(false);
+        } finally {
+          clearProcess(this);
+        }
       } finally {
-        clearProcess(this);
         lock.unlock();
-        if (this.status == OptimizingProcess.Status.FAILED) {
-          cancelTasks();
-        }
       }
     }
 
diff --git 
a/ams/server/src/test/java/com/netease/arctic/server/optimizing/TestOptimizingQueue.java
 
b/ams/server/src/test/java/com/netease/arctic/server/optimizing/TestOptimizingQueue.java
index 95080d93f..c644753f6 100644
--- 
a/ams/server/src/test/java/com/netease/arctic/server/optimizing/TestOptimizingQueue.java
+++ 
b/ams/server/src/test/java/com/netease/arctic/server/optimizing/TestOptimizingQueue.java
@@ -222,9 +222,9 @@ public class TestOptimizingQueue extends AMSTableTestBase {
     Assert.assertEquals(OptimizingProcess.Status.SUCCESS, 
optimizingProcess.getStatus());
     
Assert.assertNull(tableRuntimeMeta.getTableRuntime().getOptimizingProcess());
 
-    // 8.commit again
-    optimizingProcess.commit();
-    Assert.assertEquals(OptimizingProcess.Status.FAILED, 
optimizingProcess.getStatus());
+    // 8.commit again, throw exceptions, and status not changed.
+    Assert.assertThrows(IllegalStateException.class, 
optimizingProcess::commit);
+    Assert.assertEquals(OptimizingProcess.Status.SUCCESS, 
optimizingProcess.getStatus());
 
     // 9.close
     optimizingProcess.close();