Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimize: undo_log table check optimize #6031

Merged
merged 15 commits into from
Nov 26, 2023
1 change: 1 addition & 0 deletions changes/en-us/2.0.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ The version is updated as follows:
- [[#5951](https://github.com/seata/seata/pull/5951)] remove un support config in jdk17
- [[#5959](https://github.com/seata/seata/pull/5959)] modify code style and remove unused import
- [[#6002](https://github.com/seata/seata/pull/6002)] remove fst serialization
- [[#6031](https://github.com/seata/seata/pull/6031)] add a check for the existence of the undolog table


### security:
Expand Down
1 change: 1 addition & 0 deletions changes/zh-cn/2.0.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ Seata 是一款开源的分布式事务解决方案,提供高性能和简单
- [[#5951](https://github.com/seata/seata/pull/5951)] 删除在 jdk17 中不支持的配置项
- [[#5959](https://github.com/seata/seata/pull/5959)] 修正代码风格问题及去除无用的类引用
- [[#6002](https://github.com/seata/seata/pull/6002)] 移除fst序列化模块
- [[#6031](https://github.com/seata/seata/pull/6031)] 添加undo_log表的存在性校验


### security:
Expand Down
17 changes: 17 additions & 0 deletions core/src/main/java/io/seata/core/constants/DBType.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

import io.seata.common.util.StringUtils;

import java.util.Optional;

/**
* database type
*
Expand Down Expand Up @@ -209,4 +211,19 @@ public static DBType valueof(String dbType) {
throw new IllegalArgumentException("unknown dbtype:" + dbType);
}


/**
* optional of db type.
*
* @param dbType the db type
* @return optional of the db type
*/
public static Optional<DBType> optionalof(String dbType) {
for (DBType dt : values()) {
if (StringUtils.equalsIgnoreCase(dt.name(), dbType)) {
return Optional.of(dt);
}
}
return Optional.empty();
}
}
23 changes: 0 additions & 23 deletions rm-datasource/src/main/java/io/seata/rm/RMHandlerAT.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import java.sql.SQLException;
import java.text.ParseException;
import java.util.Date;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import io.seata.common.util.DateUtil;
import io.seata.core.model.BranchType;
Expand All @@ -44,8 +42,6 @@ public class RMHandlerAT extends AbstractRMHandler {

private static final int LIMIT_ROWS = 3000;

private final Map<String, Boolean> undoLogTableExistRecord = new ConcurrentHashMap<>();

@Override
public void handle(UndoLogDeleteRequest request) {
String resourceId = request.getResourceId();
Expand All @@ -56,12 +52,6 @@ public void handle(UndoLogDeleteRequest request) {
return;
}

boolean hasUndoLogTable = undoLogTableExistRecord.computeIfAbsent(resourceId, id -> checkUndoLogTableExist(dataSourceProxy));
if (!hasUndoLogTable) {
LOGGER.debug("resource({}) has no undo_log table, UndoLogDeleteRequest will be ignored", resourceId);
return;
}

Date division = getLogCreated(request.getSaveDays());

UndoLogManager manager = getUndoLogManager(dataSourceProxy);
Expand All @@ -80,19 +70,6 @@ public void handle(UndoLogDeleteRequest request) {
}
}

boolean checkUndoLogTableExist(DataSourceProxy dataSourceProxy) {
UndoLogManager manager = getUndoLogManager(dataSourceProxy);
try (Connection connection = getConnection(dataSourceProxy)) {
if (connection == null) {
return false;
}
return manager.hasUndoLogTable(connection);
} catch (Exception e) {
// should never happen, hasUndoLogTable method had catch all Exception
return false;
}
}

Connection getConnection(DataSourceProxy dataSourceProxy) {
try {
return dataSourceProxy.getPlainConnection();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,25 @@

import javax.sql.DataSource;

import io.seata.common.ConfigurationKeys;
import io.seata.common.Constants;
import io.seata.common.loader.EnhancedServiceNotFoundException;
import io.seata.config.ConfigurationFactory;
import io.seata.core.constants.DBType;
import io.seata.core.context.RootContext;
import io.seata.core.model.BranchType;
import io.seata.core.model.Resource;
import io.seata.rm.DefaultResourceManager;
import io.seata.rm.datasource.sql.struct.TableMetaCacheFactory;
import io.seata.rm.datasource.undo.UndoLogManager;
import io.seata.rm.datasource.undo.UndoLogManagerFactory;
import io.seata.rm.datasource.util.JdbcUtils;
import io.seata.sqlparser.util.JdbcConstants;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static io.seata.common.DefaultValues.DEFAULT_TRANSACTION_UNDO_LOG_TABLE;

/**
* The type Data source proxy.
*
Expand Down Expand Up @@ -92,6 +100,9 @@ private void init(DataSource dataSource, String resourceGroupId) {
getMySQLAdaptiveType(connection);
}
version = selectDbVersion(connection);

checkUndoLogTableExist(connection);

} catch (SQLException e) {
throw new IllegalStateException("can not init dataSource", e);
}
Expand Down Expand Up @@ -120,6 +131,32 @@ private void getMySQLAdaptiveType(Connection connection) {
}
}

/**
* check existence of undolog table
*
* if the table not exist fast fail, or else keep silence
*
* @param conn db connection
*/
private void checkUndoLogTableExist(Connection conn) {
if (DBType.optionalof(dbType).isPresent()) {
UndoLogManager undoLogManager;
try {
undoLogManager = UndoLogManagerFactory.getUndoLogManager(dbType);
} catch (EnhancedServiceNotFoundException e) {
LOGGER.error("can't find undoLogManager service provider for dbtype: {}", dbType);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我建议这里直接抛异常出去,说明用户再使用一个我们不支持的数据库,要提示这个数据库不支持
I suggest throwing an exception directly here, indicating that if the user uses a database that we do not support, we need to prompt that the database is not supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

没有抛出去,是因为很多单测使用了数据库代理,会导致单测失败。
It is not thrown out because many unit tests use database proxy, which can be lead it fail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

没有抛出去,是因为很多单测使用了数据库代理,会导致单测失败。 It is not thrown out because many unit tests use database proxy, which can be lead it fail.

有没有什么办法在单测的时候不进行校验undolog table?或者增加一个开关,这个开关允许用户配置check,如果用户关闭,就不进行check,然后我们默认开启,在单测的时候关闭check
Is there any way to not check undolog table during single test or add a switch that allows user to configure check and if user turns it off, no check is performed, then we turn it on by default and turn off check during single test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update mockito version to 4.5.1 same as seata server's mockito dependency to support static mock. when unit test. mock undolog table check.

return;
}
boolean undoLogTableExist = undoLogManager.hasUndoLogTable(conn);
if (!undoLogTableExist) {
String undoLogTableName = ConfigurationFactory.getInstance()
.getConfig(ConfigurationKeys.TRANSACTION_UNDO_LOG_TABLE, DEFAULT_TRANSACTION_UNDO_LOG_TABLE);
String errMsg = String.format("in AT mode, %s table not exist", undoLogTableName);
throw new IllegalStateException(errMsg);
}
}
}

/**
* publish tableMeta refresh event
*/
Expand Down
13 changes: 0 additions & 13 deletions rm-datasource/src/test/java/io/seata/rm/RMHandlerATTest.java
funky-eyes marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,9 @@ void hasUndoLogTableTest() {
for (int i = 0; i < testTimes; i++) {
handler.handle(request);
}
verify(handler, times(1)).checkUndoLogTableExist(any());
verify(handler, times(testTimes)).deleteUndoLog(any(), any(), any());
}

@Test
void noUndoLogTableTest() {
RMHandlerAT handler = buildHandler(false);
UndoLogDeleteRequest request = buildRequest();
int testTimes = 5;
for (int i = 0; i < testTimes; i++) {
handler.handle(request);
}
verify(handler, times(1)).checkUndoLogTableExist(any());
verify(handler, never()).deleteUndoLog(any(), any(), any());
}

private RMHandlerAT buildHandler(boolean hasUndoLogTable) {
RMHandlerAT handler = spy(new RMHandlerAT());
DataSourceManager dataSourceManager = mock(DataSourceManager.class);
Expand Down
Loading