[ISSUE #9943] Resolve issue Config cas update not work (#9952)

* fix issue 9943 * add some description * add some log for roll back transaction * add some log for roll back transaction * set default rollBackOnUpdateFail false. * format style * fix some bug
alibaba · Mar 3, 2023 · e58c444 · e58c444
1 parent acc8cbe
commit e58c444
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 29 deletions.
diff --git a/...java/com/alibaba/nacos/config/server/service/repository/embedded/BaseDatabaseOperate.java b/...java/com/alibaba/nacos/config/server/service/repository/embedded/BaseDatabaseOperate.java
@@ -28,6 +28,7 @@
 import org.springframework.jdbc.CannotGetJdbcConnectionException;
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.jdbc.core.RowMapper;
+import org.springframework.transaction.IllegalTransactionStateException;
 import org.springframework.transaction.support.TransactionTemplate;
 
 import java.util.List;
@@ -210,36 +211,50 @@ default Boolean update(TransactionTemplate transactionTemplate, JdbcTemplate jdb
      */
     default Boolean update(TransactionTemplate transactionTemplate, JdbcTemplate jdbcTemplate,
             List<ModifyRequest> contexts, BiConsumer<Boolean, Throwable> consumer) {
-        return transactionTemplate.execute(status -> {
-            String[] errSql = new String[] {null};
-            Object[][] args = new Object[][] {null};
-            try {
-                contexts.forEach(pair -> {
-                    errSql[0] = pair.getSql();
-                    args[0] = pair.getArgs();
-                    LoggerUtils.printIfDebugEnabled(LogUtil.DEFAULT_LOG, "current sql : {}", errSql[0]);
-                    LoggerUtils.printIfDebugEnabled(LogUtil.DEFAULT_LOG, "current args : {}", args[0]);
-                    jdbcTemplate.update(pair.getSql(), pair.getArgs());
-                });
-                if (consumer != null) {
-                    consumer.accept(Boolean.TRUE, null);
-                }
-                return Boolean.TRUE;
-            } catch (BadSqlGrammarException | DataIntegrityViolationException e) {
-                FATAL_LOG.error("[db-error] sql : {}, args : {}, error : {}", errSql[0], args[0], e.toString());
-                if (consumer != null) {
-                    consumer.accept(Boolean.FALSE, e);
+        boolean updateResult = Boolean.FALSE;
+        try {
+            updateResult = transactionTemplate.execute(status -> {
+                String[] errSql = new String[] {null};
+                Object[][] args = new Object[][] {null};
+                try {
+                    contexts.forEach(pair -> {
+                        errSql[0] = pair.getSql();
+                        args[0] = pair.getArgs();
+                        boolean rollBackOnUpdateFail = pair.isRollBackOnUpdateFail();
+                        LoggerUtils.printIfDebugEnabled(LogUtil.DEFAULT_LOG, "current sql : {}", errSql[0]);
+                        LoggerUtils.printIfDebugEnabled(LogUtil.DEFAULT_LOG, "current args : {}", args[0]);
+                        int row = jdbcTemplate.update(pair.getSql(), pair.getArgs());
+                        if (rollBackOnUpdateFail && row < 1) {
+                            LoggerUtils.printIfDebugEnabled(LogUtil.DEFAULT_LOG, "SQL update affected {} rows ", row);
+                            throw new IllegalTransactionStateException("Illegal transaction");
+                        }
+                    });
+                    if (consumer != null) {
+                        consumer.accept(Boolean.TRUE, null);
+                    }
+                    return Boolean.TRUE;
+                } catch (BadSqlGrammarException | DataIntegrityViolationException e) {
+                    FATAL_LOG.error("[db-error] sql : {}, args : {}, error : {}", errSql[0], args[0], e.toString());
+                    if (consumer != null) {
+                        consumer.accept(Boolean.FALSE, e);
+                    }
+                    return Boolean.FALSE;
+                } catch (CannotGetJdbcConnectionException e) {
+                    FATAL_LOG.error("[db-error] sql : {}, args : {}, error : {}", errSql[0], args[0], e.toString());
+                    throw e;
+                } catch (DataAccessException e) {
+                    FATAL_LOG.error("[db-error] DataAccessException sql : {}, args : {}, error : {}", errSql[0],
+                            args[0], ExceptionUtil.getAllExceptionMsg(e));
+                    throw e;
                 }
-                return Boolean.FALSE;
-            } catch (CannotGetJdbcConnectionException e) {
-                FATAL_LOG.error("[db-error] sql : {}, args : {}, error : {}", errSql[0], args[0], e.toString());
-                throw e;
-            } catch (DataAccessException e) {
-                FATAL_LOG.error("[db-error] DataAccessException sql : {}, args : {}, error : {}", errSql[0], args[0],
-                        ExceptionUtil.getAllExceptionMsg(e));
-                throw e;
+            });
+        } catch (IllegalTransactionStateException e) {
+            LoggerUtils.printIfDebugEnabled(LogUtil.DEFAULT_LOG, "Roll back transaction for {} ", e.getMessage());
+            if (consumer != null) {
+                consumer.accept(Boolean.FALSE, e);
             }
-        });
+        }
+        return updateResult;
     }
 
     /**

diff --git a/...ba/nacos/config/server/service/repository/embedded/EmbeddedStoragePersistServiceImpl.java b/...ba/nacos/config/server/service/repository/embedded/EmbeddedStoragePersistServiceImpl.java
@@ -1794,7 +1794,7 @@ private void updateConfigInfoAtomicCas(final ConfigInfo configInfo, final String
                 use, effect, type, schema, configInfo.getDataId(), configInfo.getGroup(), tenantTmp,
                 configInfo.getMd5()};
 
-        EmbeddedStorageContextUtils.addSqlContext(sql, args);
+        EmbeddedStorageContextUtils.addSqlContext(true, sql, args);
     }
 
     @Override

diff --git a/...rc/main/java/com/alibaba/nacos/config/server/service/sql/EmbeddedStorageContextUtils.java b/...rc/main/java/com/alibaba/nacos/config/server/service/sql/EmbeddedStorageContextUtils.java
@@ -56,6 +56,24 @@ public static void addSqlContext(String sql, Object... args) {
         SQL_CONTEXT.set(requests);
     }
 
+    /**
+     * Add sql context.
+     *
+     * @param rollbackOnUpdateFail  roll back when update fail
+     * @param sql  sql
+     * @param args argument list
+     */
+    public static void addSqlContext(boolean rollbackOnUpdateFail, String sql, Object... args) {
+        ArrayList<ModifyRequest> requests = SQL_CONTEXT.get();
+        ModifyRequest context = new ModifyRequest();
+        context.setExecuteNo(requests.size());
+        context.setSql(sql);
+        context.setArgs(args);
+        context.setRollBackOnUpdateFail(rollbackOnUpdateFail);
+        requests.add(context);
+        SQL_CONTEXT.set(requests);
+    }
+
     /**
      * Put extend info.
      *

diff --git a/config/src/main/java/com/alibaba/nacos/config/server/service/sql/ModifyRequest.java b/config/src/main/java/com/alibaba/nacos/config/server/service/sql/ModifyRequest.java
@@ -33,6 +33,8 @@ public class ModifyRequest implements Serializable {
 
     private String sql;
 
+    private boolean rollBackOnUpdateFail = Boolean.FALSE;
+
     private Object[] args;
 
     public ModifyRequest() {
@@ -66,6 +68,14 @@ public void setArgs(Object[] args) {
         this.args = args;
     }
 
+    public boolean isRollBackOnUpdateFail() {
+        return rollBackOnUpdateFail;
+    }
+
+    public void setRollBackOnUpdateFail(boolean rollBackOnUpdateFail) {
+        this.rollBackOnUpdateFail = rollBackOnUpdateFail;
+    }
+
     @Override
     public String toString() {
         return "SQL{" + "executeNo=" + executeNo + ", sql='" + sql + '\'' + ", args=" + Arrays.toString(args) + '}';