bugfix: fix the case that could not retry acquire global lock (#3133)

changes/1.5.0.md

@@ -49,6 +49,7 @@ Seata 是一款开源的分布式事务解决方案，提供高性能和简单
   - [[#3540](https://github.com/seata/seata/pull/3540)] 修复server发布打包时缺失文件
   - [[#3573](https://github.com/seata/seata/pull/3573)] 修复 README.md 文件中设计器路径错误
   - [[#3597](https://github.com/seata/seata/pull/3597)] 修复可能导致NPE的问题
+  - [[#3133](https://github.com/seata/seata/pull/3133)] 修复某些场景下无法重试全局锁的问题
 
   ### optimize： 
 

changes/en-us/1.5.0.md

@@ -87,6 +87,7 @@
   - [[#3551](https://github.com/seata/seata/pull/3551)] make RETRY_DEAD_THRESHOLD bigger and configurable
   - [[#3589](https://github.com/seata/seata/pull/3589)] Changed exception check by JUnit API usage
   - [[#3601](https://github.com/seata/seata/pull/3601)] make `LoadBalanceProperties` compatible with `spring-boot:2.x` and above
+  - [[#3133](https://github.com/seata/seata/pull/3133)] fix the case that could not retry acquire global lock
   - [[#3631](https://github.com/seata/seata/pull/3631)] optimize  nacos-config.py  parameter
   - [[#3638](https://github.com/seata/seata/pull/3638)] optimize the error when use update or delete with join in sql
 

rm-datasource/src/main/java/io/seata/rm/datasource/ConnectionProxy.java

@@ -50,13 +50,14 @@ public class ConnectionProxy extends AbstractConnectionProxy {
 
     private ConnectionContext context = new ConnectionContext();
 
+    private final LockRetryPolicy lockRetryPolicy = new LockRetryPolicy(this);
+
     private static final int REPORT_RETRY_COUNT = ConfigurationFactory.getInstance().getInt(
         ConfigurationKeys.CLIENT_REPORT_RETRY_COUNT, DEFAULT_CLIENT_REPORT_RETRY_COUNT);
 
     public static final boolean IS_REPORT_SUCCESS_ENABLE = ConfigurationFactory.getInstance().getBoolean(
         ConfigurationKeys.CLIENT_REPORT_SUCCESS_ENABLE, DEFAULT_CLIENT_REPORT_SUCCESS_ENABLE);
 
-    private final static LockRetryPolicy LOCK_RETRY_POLICY = new LockRetryPolicy();
 
     /**
      * Instantiates a new Connection proxy.
@@ -181,7 +182,7 @@ public class ConnectionProxy extends AbstractConnectionProxy {
     @Override
     public void commit() throws SQLException {
         try {
-            LOCK_RETRY_POLICY.execute(() -> {
+            lockRetryPolicy.execute(() -> {
                 doCommit();
                 return null;
             });
@@ -325,10 +326,21 @@ public class ConnectionProxy extends AbstractConnectionProxy {
         protected static final boolean LOCK_RETRY_POLICY_BRANCH_ROLLBACK_ON_CONFLICT = ConfigurationFactory
             .getInstance().getBoolean(ConfigurationKeys.CLIENT_LOCK_RETRY_POLICY_BRANCH_ROLLBACK_ON_CONFLICT, DEFAULT_CLIENT_LOCK_RETRY_POLICY_BRANCH_ROLLBACK_ON_CONFLICT);
 
+        protected final ConnectionProxy connection;
+
+        public LockRetryPolicy(ConnectionProxy connection) {
+            this.connection = connection;
+        }
+
         public <T> T execute(Callable<T> callable) throws Exception {
-            if (LOCK_RETRY_POLICY_BRANCH_ROLLBACK_ON_CONFLICT) {
+            // the only case that not need to retry acquire lock hear is
+            //    LOCK_RETRY_POLICY_BRANCH_ROLLBACK_ON_CONFLICT == true && connection#autoCommit == true
+            // because it has retry acquire lock when AbstractDMLBaseExecutor#executeAutoCommitTrue
+            if (LOCK_RETRY_POLICY_BRANCH_ROLLBACK_ON_CONFLICT && connection.getContext().isAutoCommitChanged()) {
                 return callable.call();
             } else {
+                // LOCK_RETRY_POLICY_BRANCH_ROLLBACK_ON_CONFLICT == false
+                // or LOCK_RETRY_POLICY_BRANCH_ROLLBACK_ON_CONFLICT == true && autoCommit == false
                 return doRetryOnLockConflict(callable);
             }
         }

rm-datasource/src/main/java/io/seata/rm/datasource/exec/AbstractDMLBaseExecutor.java

@@ -172,10 +172,9 @@ public abstract class AbstractDMLBaseExecutor<T, S extends Statement> extends Ba
     protected abstract TableRecords afterImage(TableRecords beforeImage) throws SQLException;
 
     private static class LockRetryPolicy extends ConnectionProxy.LockRetryPolicy {
-        private final ConnectionProxy connection;
 
         LockRetryPolicy(final ConnectionProxy connection) {
-            this.connection = connection;
+            super(connection);
         }
 
         @Override

rm-datasource/src/test/java/io/seata/rm/datasource/ConnectionProxyTest.java

@@ -22,6 +22,8 @@ import io.seata.core.model.ResourceManager;
 import io.seata.rm.DefaultResourceManager;
 import io.seata.rm.datasource.exec.LockConflictException;
 import io.seata.rm.datasource.exec.LockWaitTimeoutException;
+import io.seata.rm.datasource.mock.MockConnection;
+import io.seata.rm.datasource.mock.MockDriver;
 import io.seata.rm.datasource.undo.SQLUndoLog;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
@@ -72,11 +74,11 @@ public class ConnectionProxyTest {
     public void testLockRetryPolicyRollbackOnConflict() throws Exception {
         boolean oldBranchRollbackFlag = (boolean) branchRollbackFlagField.get(null);
         branchRollbackFlagField.set(null, true);
-        ConnectionProxy connectionProxy = new ConnectionProxy(dataSourceProxy, null);
+        ConnectionProxy connectionProxy = new ConnectionProxy(dataSourceProxy, new MockConnection(new MockDriver(), "", null));
         connectionProxy.bind(TEST_XID);
         connectionProxy.appendUndoLog(new SQLUndoLog());
         connectionProxy.appendLockKey(lockKey);
-        Assertions.assertThrows(LockConflictException.class, connectionProxy::commit);
+        Assertions.assertThrows(LockWaitTimeoutException.class, connectionProxy::commit);
         branchRollbackFlagField.set(null, oldBranchRollbackFlag);
     }