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

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

Conversation

sixlei
Copy link
Contributor

@sixlei sixlei commented May 22, 2024

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

fix issue #6561

Ⅱ. Does this pull request fix one issue?

fix issue #6561
将对undolog的SQLIntegrityConstraintViolationException异常的try catch范围缩小,只处理undo_log的插入并发,如果有业务回滚时的并发插入异常,直接抛出,按照脏写处理即可。

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@sixlei sixlei changed the title bugfix: fix when doing global rollbacking ,may cause an endless loop bugfix: fix when doing global rollback ,may cause an endless loop May 22, 2024
@funky-eyes funky-eyes added this to the 2.x Backlog milestone May 22, 2024
@funky-eyes funky-eyes added type: bug Category issues or prs related to bug. module/rm-datasource rm-datasource module labels May 22, 2024
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.

@funky-eyes
Copy link
Contributor

Due to the author's prolonged lack of response, I will close this PR and seek another developer to address this issue.

@funky-eyes funky-eyes closed this Jun 21, 2024
@funky-eyes funky-eyes reopened this Aug 3, 2024
@funky-eyes
Copy link
Contributor

Could you please resolve the code conflicts first?

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/rm-datasource rm-datasource module type: bug Category issues or prs related to bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants