You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "sixlei (via GitHub)" <gi...@apache.org> on 2024/08/03 14:15:47 UTC
[PR] bugfix: fix when doing global rollback ,may cause an endless loop [incubator-seata]
sixlei opened a new pull request, #6569:
URL: https://github.com/apache/incubator-seata/pull/6569
<!-- Please make sure you have read and understood the contributing guidelines -->
- [ ] I have registered the PR [changes](../changes).
### Ⅰ. Describe what this PR did
fix issue #6561
### Ⅱ. Does this pull request fix one issue?
<!-- If that, add "fixes #xxx" below in the next line, for example, fixes #97. -->
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix when doing global rollback ,may cause an endless loop [incubator-seata]
Posted by "sixlei (via GitHub)" <gi...@apache.org>.
sixlei closed pull request #6569: bugfix: fix when doing global rollback ,may cause an endless loop
URL: https://github.com/apache/incubator-seata/pull/6569
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix when doing global rollback ,may cause an endless loop [incubator-seata]
Posted by "sixlei (via GitHub)" <gi...@apache.org>.
sixlei commented on code in PR #6569:
URL: https://github.com/apache/incubator-seata/pull/6569#discussion_r1670382778
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/AbstractUndoLogManager.java:
##########
@@ -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);
Review Comment:
> 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.
<img width="808" alt="Snipaste_2024-07-09_19-43-48" src="https://github.com/apache/incubator-seata/assets/48412175/5ffb6bde-e3ab-45b2-94c8-97b197c319d4">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix when doing global rollback ,may cause an endless loop [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #6569:
URL: https://github.com/apache/incubator-seata/pull/6569#discussion_r1629042530
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/AbstractUndoLogManager.java:
##########
@@ -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);
Review Comment:
Why is it modified here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix when doing global rollback ,may cause an endless loop [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on PR #6569:
URL: https://github.com/apache/incubator-seata/pull/6569#issuecomment-2266726278
Could you please resolve the code conflicts first?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix when doing global rollback ,may cause an endless loop [incubator-seata]
Posted by "sixlei (via GitHub)" <gi...@apache.org>.
sixlei commented on code in PR #6569:
URL: https://github.com/apache/incubator-seata/pull/6569#discussion_r1670373704
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/AbstractUndoLogManager.java:
##########
@@ -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);
Review Comment:
> 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.
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/AbstractUndoLogManager.java:
##########
@@ -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);
Review Comment:
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix when doing global rollback ,may cause an endless loop [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #6569:
URL: https://github.com/apache/incubator-seata/pull/6569#discussion_r1704859517
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/AbstractUndoLogManager.java:
##########
@@ -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);
+ conn.commit();
+ if (LOGGER.isInfoEnabled()) {
+ LOGGER.info("xid {} branch {}, undo_log added with {}", xid, branchId,
+ State.GlobalFinished.name());
+ }
+ } catch (SQLIntegrityConstraintViolationException e) {
Review Comment:
你只是使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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix when doing global rollback ,may cause an endless loop [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #6569:
URL: https://github.com/apache/incubator-seata/pull/6569#discussion_r1702787291
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/AbstractUndoLogManager.java:
##########
@@ -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);
Review Comment:
> > 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.
>
> <img alt="Snipaste_2024-07-09_19-43-48" width="808" src="https://private-user-images.githubusercontent.com/48412175/346945446-5ffb6bde-e3ab-45b2-94c8-97b197c319d4.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjI2OTI4MzUsIm5iZiI6MTcyMjY5MjUzNSwicGF0aCI6Ii80ODQxMjE3NS8zNDY5NDU0NDYtNWZmYjZiZGUtZTNhYi00NWIyLTk0YzgtOTdiMTk3YzMxOWQ0LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA4MDMlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwODAzVDEzNDIxNVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWUzYjA1MTM2NmQyNzFlNzkyMGRkNjUwZWNkYjY0Mzc0MmJiZTkxNjI5N2FmNmUxM2FlMjExMTAxNzg2MjhmZWMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.GDWMSPrYkc0w7rnt4pN5SD84NXu1AYKsP6SPHqWPfwQ">
Sorry, I just saw your message now. I'll review the code as soon as possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix when doing global rollback ,may cause an endless loop [incubator-seata]
Posted by "sixlei (via GitHub)" <gi...@apache.org>.
sixlei commented on code in PR #6569:
URL: https://github.com/apache/incubator-seata/pull/6569#discussion_r1670351394
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/AbstractUndoLogManager.java:
##########
@@ -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);
Review Comment:
> 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix when doing global rollback ,may cause an endless loop [incubator-seata]
Posted by "sixlei (via GitHub)" <gi...@apache.org>.
sixlei commented on code in PR #6569:
URL: https://github.com/apache/incubator-seata/pull/6569#discussion_r1634051263
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/AbstractUndoLogManager.java:
##########
@@ -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);
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org