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.

Sign up for GitHub

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

Jump to bottom

bugfix: fix when doing global rollback ,may cause an endless loop #6569

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from
Open

bugfix: fix when doing global rollback ,may cause an endless loop #6569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changes/en-us/2.x.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Add changes here for all PR submitted to the 2.x branch.
- [[#6554](https://github.com/apache/incubator-seata/pull/6554)] fix unfixed serializer
- [[#6555](https://github.com/apache/incubator-seata/pull/6555)] businessActionContext is compatible with io seata
- [[#6553](https://github.com/apache/incubator-seata/pull/6553)] fix saga "cannot matching status"
- [[#6569](https://github.com/apache/incubator-seata/pull/6569)] fix when doing global rollback ,may cause an endless loop


### optimize:
Expand Down Expand Up @@ -208,5 +209,6 @@ Thanks to these contributors for their code commits. Please report an unintended
- [TakeActionNow2019](https://github.com/TakeActionNow2019)
- [sunxunle](https://github.com/sunxunle)
- [bageyang](https://github.com/bageyang)
- [sixl](https://github.com/sixlei)

Also, we receive many valuable issues, questions and advices from our community. Thanks for you all.
2 changes: 2 additions & 0 deletions changes/zh-cn/2.x.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
- [[#6554](https://github.com/apache/incubator-seata/pull/6554)] 修复序列化器不固定使用对应配置序列化器的问题
- [[#6555](https://github.com/apache/incubator-seata/pull/6555)] 修复businessActionContext对io seata包的不兼容
- [[#6553](https://github.com/apache/incubator-seata/pull/6553)] 修复执行完 'ServiceTask' 后无法应用任何评估器的问题
- [[#6569](https://github.com/apache/incubator-seata/pull/6569)] 修复做全局回滚时，可能造成死循环的问题

### optimize:
- [[#6031](https://github.com/apache/incubator-seata/pull/6031)] 添加undo_log表的存在性校验
Expand Down Expand Up @@ -204,5 +205,6 @@
- [TakeActionNow2019](https://github.com/TakeActionNow2019)
- [sunxunle](https://github.com/sunxunle)
- [bageyang](https://github.com/bageyang)
- [sixl](https://github.com/sixlei)

同时，我们收到了社区反馈的很多有价值的issue和建议，非常感谢大家。
24 changes: 13 additions & 11 deletions rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/AbstractUndoLogManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,20 +339,22 @@ public void undo(DataSourceProxy dataSourceProxy, String xid, long branchId) thr
State.GlobalFinished.name());
}
} else {
insertUndoLogWithGlobalFinished(xid, branchId, UndoLogParserFactory.getInstance(), conn);
conn.commit();
if (LOGGER.isInfoEnabled()) {
LOGGER.info("xid {} branch {}, undo_log added with {}", xid, branchId,
State.GlobalFinished.name());
try {
insertUndoLogWithGlobalFinished(xid, branchId, UndoLogParserFactory.getInstance(), conn);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it modified here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The changes here are to narrow the scope of the catch for SQLIntegrityConstraintViolationException at the outermost level, only handling conflicts with the unique index on the undo_log. If the unique index conflict also occurs when executing the rollback undoExecutor.executeOn for the business, it will enter a deadlock directly here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The changes here are to narrow the scope of the catch for SQLIntegrityConstraintViolationException at the outermost level, only handling conflicts with the unique index on the undo_log. If the unique index conflict also occurs when executing the rollback undoExecutor.executeOn for the business, it will enter a deadlock directly here.

I am very sorry, I replied to this paragraph one day after you replied, but it has been pending. I didn't know I had to click submitted. disturb you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The changes here are to narrow the scope of the catch for SQLIntegrityConstraintViolationException at the outermost level, only handling conflicts with the unique index on the undo_log. If the unique index conflict also occurs when executing the rollback undoExecutor.executeOn for the business, it will enter a deadlock directly here.

Snipaste_2024-07-09_19-43-48

Copy link
Contributor

Choose a reason for hiding this comment

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

The changes here are to narrow the scope of the catch for SQLIntegrityConstraintViolationException at the outermost level, only handling conflicts with the unique index on the undo_log. If the unique index conflict also occurs when executing the rollback undoExecutor.executeOn for the business, it will enter a deadlock directly here.

Snipaste_2024-07-09_19-43-48

Sorry, I just saw your message now. I'll review the code as soon as possible.

conn.commit();
if (LOGGER.isInfoEnabled()) {
LOGGER.info("xid {} branch {}, undo_log added with {}", xid, branchId,
State.GlobalFinished.name());
}
} catch (SQLIntegrityConstraintViolationException e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

你只是使SQLIntegrityConstraintViolationException的范围缩小了，但是并没将因为唯一索引导致的SQLIntegrityConstraintViolationException 转换成SQLUndoDirtyException或者说抛出BranchRollbackFailed_Unretriable状态的BranchTransactionException，目前的改动我认为是无效的

You have merely narrowed the scope of SQLIntegrityConstraintViolationException without converting SQLIntegrityConstraintViolationException caused by unique indexes into SQLUndoDirtyException or throwing BranchTransactionException with BranchRollbackFailed_Unretriable state. I find the current changes ineffective.

// Possible undo_log has been inserted into the database by other processes, retrying rollback undo_log
if (LOGGER.isInfoEnabled()) {
LOGGER.info("xid {} branch {}, undo_log inserted, retry rollback", xid, branchId);
}
continue;
}
}

return;
} catch (SQLIntegrityConstraintViolationException e) {
// Possible undo_log has been inserted into the database by other processes, retrying rollback undo_log
if (LOGGER.isInfoEnabled()) {
LOGGER.info("xid {} branch {}, undo_log inserted, retry rollback", xid, branchId);
}
} catch (Throwable e) {
if (conn != null) {
try {
Expand Down
Loading