Repository: apache/amoro (java)

Issue:
[Improvement]: need call cancelRenew when release a  Renewableblocker
### Search before asking

- [x] I have searched in the [issues](https://github.com/apache/amoro/issues?q=is%3Aissue) and found no similar issues.


### What would you like to be improved?

When we execute a simple Spark insert SQL, the AMS side reported this error.

![Image](https://github.com/user-attachments/assets/cc519528-e513-4c07-9c7d-2d5413bbe969)

```
2025-01-20 18:01:40,998 ERROR [thrift-server-table-metastore-11] [org.apache.amoro.server.TableManagementService] [] - Thrift service:TableManagementService.renewBlocker execute failed
org.apache.amoro.server.exception.ObjectNotExistsException: Blocker 181 of hive_catalog.amoro_v7.block_test(tableId=874643) not exists
        at org.apache.amoro.server.table.TableRuntime.renew(TableRuntime.java:621) ~[amoro-ams-server-0.7.0.1-SNAPSHOT.jar:0.7.0.1-SNAPSHOT]
        at org.apache.amoro.server.table.DefaultTableService.renewBlocker(DefaultTableService.java:278) ~[amoro-ams-server-0.7.0.1-SNAPSHOT.jar:0.7.0.1-SNAPSHOT]
        at org.apache.amoro.server.TableManagementService.renewBlocker(TableManagementService.java:153) ~[amoro-ams-server-0.7.0.1-SNAPSHOT.jar:0.7.0.1-SNAPSHOT]
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_152]
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_152]
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_152]
        at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_152]
        at org.apache.amoro.server.utils.ThriftServiceProxy.invoke(ThriftServiceProxy.java:56) [amoro-ams-server-0.7.0.1-SNAPSHOT.jar:0.7.0.1-SNAPSHOT]
        at com.sun.proxy.$Proxy47.renewBlocker(Unknown Source) [?:?]
        at org.apache.amoro.api.AmoroTableMetastore$Processor$renewBlocker.getResult(AmoroTableMetastore.java:1635) [amoro-core-0.7.0.1-SNAPSHOT.jar:0.7.0.1-SNAPSHOT]
        at org.apache.amoro.api.AmoroTableMetastore$Processor$renewBlocker.getResult(AmoroTableMetastore.java:1611) [amoro-core-0.7.0.1-SNAPSHOT.jar:0.7.0.1-SNAPSHOT]
        at org.apache.amoro.shade.thrift.org.apache.thrift.ProcessFunction.process(ProcessFunction.java:40) [amoro-shade-thrift-0.20.0-0.7-SNAPSHOT.jar:0.20.0-0.7-SNAPSHOT]
        at org.apache.amoro.shade.thrift.org.apache.thrift.TBaseProcessor.process(TBaseProcessor.java:40) [amoro-shade-thrift-0.20.0-0.7-SNAPSHOT.jar:0.20.0-0.7-SNAPSHOT]
        at org.apache.amoro.shade.thrift.org.apache.thrift.TMultiplexedProcessor.process(TMultiplexedProcessor.java:147) [amoro-shade-thrift-0.20.0-0.7-SNAPSHOT.jar:0.20.0-0.7-SNAPSHOT]
        at org.apache.amoro.shade.thrift.org.apache.thrift.server.AbstractNonblockingServer$FrameBuffer.invoke(AbstractNonblockingServer.java:492) [amoro-shade-thrift-0.20.0-0.7-SNAPSHOT.jar:0.20.0-0.7-SNAPSHOT]
        at org.apache.amoro.shade.thrift.org.apache.thrift.server.Invocation.run(Invocation.java:19) [amoro-shade-thrift-0.20.0-0.7-SNAPSHOT.jar:0.20.0-0.7-SNAPSHOT]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [?:1.8.0_152]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [?:1.8.0_152]
        at java.lang.Thread.run(Thread.java:748) [?:1.8.0_152]
```

### How should we improve?

when we release the Renewableblocker, we should check whether we need to cancelRenew

![Image](https://github.com/user-attachments/assets/0784601c-eb58-419f-9e76-14ac8655924b)

### Are you willing to submit PR?

- [x] Yes I am willing to submit a PR!

### Subtasks

_No response_

### Code of Conduct

- [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)


Pull request:
[AMORO-3410]  cancel renew procedure after releasing block
<!--
Thanks for sending a pull request!

Here are some tips for you:
  1. If this is your first time, please read our contributor guidelines: https://amoro.apache.org/how-to-contribute/
  2. If the PR is related to an issue in https://github.com/apache/amoro/issues, add '[AMORO-XXXX]' in your PR title, e.g., '[AMORO-XXXX] Your PR title ...'.
  3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][AMORO-XXXX] Your PR title ...'.
-->

## Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
  1. If you add a feature, you can talk about its use case.
  2. If you fix a bug, you can clarify why it is a bug.
  3. Use Fix/Resolve/Close #{ISSUE_NUMBER} to link this PR to its related issue
-->

Close #3410 .

## Brief change log
<!--
Clearly describe the changes made in modules, classes, methods, etc.
-->

- when we release block, we should cancel the renew future.

## How was this patch tested?

- [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [ ] Run test locally before making a pull request

## Documentation

- Does this pull request introduce a new feature? (yes / no)
- If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)


Patch:
diff --git a/amoro-format-iceberg/src/main/java/org/apache/amoro/table/blocker/BasicTableBlockerManager.java b/amoro-format-iceberg/src/main/java/org/apache/amoro/table/blocker/BasicTableBlockerManager.java
index 8db4e299f..924c4ed45 100644
--- a/amoro-format-iceberg/src/main/java/org/apache/amoro/table/blocker/BasicTableBlockerManager.java
+++ b/amoro-format-iceberg/src/main/java/org/apache/amoro/table/blocker/BasicTableBlockerManager.java
@@ -68,6 +68,11 @@ public class BasicTableBlockerManager implements TableBlockerManager {
     } catch (TException e) {
       throw new IllegalStateException(
           "failed to release " + tableIdentifier + "'s blocker " + blocker.blockerId(), e);
+    } finally {
+      // Regardless of whether the lock was successfully released, we will no longer renew the lock.
+      if (blocker instanceof RenewableBlocker) {
+        ((RenewableBlocker) blocker).cancelRenew();
+      }
     }
   }
 
diff --git a/amoro-format-iceberg/src/main/java/org/apache/amoro/table/blocker/RenewableBlocker.java b/amoro-format-iceberg/src/main/java/org/apache/amoro/table/blocker/RenewableBlocker.java
index 3f72e0e68..9bcdc97e0 100644
--- a/amoro-format-iceberg/src/main/java/org/apache/amoro/table/blocker/RenewableBlocker.java
+++ b/amoro-format-iceberg/src/main/java/org/apache/amoro/table/blocker/RenewableBlocker.java
@@ -21,6 +21,7 @@ package org.apache.amoro.table.blocker;
 import org.apache.amoro.AmsClient;
 import org.apache.amoro.api.BlockableOperation;
 import org.apache.amoro.api.NoSuchObjectException;
+import org.apache.amoro.shade.guava32.com.google.common.annotations.VisibleForTesting;
 import org.apache.amoro.shade.guava32.com.google.common.base.Preconditions;
 import org.apache.amoro.shade.guava32.com.google.common.util.concurrent.ThreadFactoryBuilder;
 import org.apache.amoro.table.TableIdentifier;
@@ -103,6 +104,11 @@ public class RenewableBlocker implements Blocker {
             .scheduleAtFixedRate(this::doRenew, interval, interval, TimeUnit.MILLISECONDS);
   }
 
+  @VisibleForTesting
+  public ScheduledFuture getRenewTaskFuture() {
+    return this.renewTaskFuture;
+  }
+
   private void doRenew() {
     try {
       this.expirationTime =


Test patch:
diff --git a/amoro-format-iceberg/src/test/java/org/apache/amoro/table/blocker/TestBasicTableBlockerManager.java b/amoro-format-iceberg/src/test/java/org/apache/amoro/table/blocker/TestBasicTableBlockerManager.java
index 3d9db1705..267dbe6f9 100644
--- a/amoro-format-iceberg/src/test/java/org/apache/amoro/table/blocker/TestBasicTableBlockerManager.java
+++ b/amoro-format-iceberg/src/test/java/org/apache/amoro/table/blocker/TestBasicTableBlockerManager.java
@@ -54,7 +54,9 @@ public class TestBasicTableBlockerManager extends TableTestBase {
     BasicTableBlockerManager blockerManager = (BasicTableBlockerManager) tableBlockerManager;
 
     Blocker block = blockerManager.block(OPERATIONS);
+    Assert.assertTrue(block instanceof RenewableBlocker);
 
     blockerManager.release(block);
+    Assert.assertTrue(((RenewableBlocker) block).getRenewTaskFuture() == null);
   }
 }