You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "PeppaO (via GitHub)" <gi...@apache.org> on 2025/07/09 05:42:06 UTC
[PR] feature: Reuse connection to merge branch transactions [incubator-seata]
PeppaO opened a new pull request, #7509:
URL: https://github.com/apache/incubator-seata/pull/7509
<!--
Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF licenses this file to You under the Apache License, Version 2.0
(the "License"); you may not use this file except in compliance with
the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<!-- Please make sure you have read and understood the contributing guidelines -->
- [ ] I have registered the PR [changes](../changes).
### Ⅰ. Describe what this PR did
https://github.com/apache/incubator-seata/discussions/7357
### Ⅱ. Does this pull request fix one issue?
<!-- If that, add "fixes #xxx" below in the next line, for example, fixes #97. -->
### Ⅲ. 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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#issuecomment-3051225304
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `8.04598%` with `80 lines` in your changes missing coverage. Please review.
> Project coverage is 60.29%. Comparing base [(`a948dc7`)](https://app.codecov.io/gh/apache/incubator-seata/commit/a948dc79c3bc55235c1c7d13fcc90116954b745d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`f607ee1`)](https://app.codecov.io/gh/apache/incubator-seata/commit/f607ee14b85d6049c099d934121f289abdbccbe7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 3 commits behind head on 2.x.
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...eata/integration/tx/api/combine/CombineAspect.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&filepath=integration-tx-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fintegration%2Ftx%2Fapi%2Fcombine%2FCombineAspect.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW50ZWdyYXRpb24tdHgtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zZWF0YS9pbnRlZ3JhdGlvbi90eC9hcGkvY29tYmluZS9Db21iaW5lQXNwZWN0LmphdmE=) | 0.00% | [34 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...rm/datasource/combine/CombineConnectionHolder.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&filepath=rm-datasource%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Fdatasource%2Fcombine%2FCombineConnectionHolder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvcm0vZGF0YXNvdXJjZS9jb21iaW5lL0NvbWJpbmVDb25uZWN0aW9uSG9sZGVyLmphdmE=) | 0.00% | [19 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ache/seata/rm/datasource/xa/DataSourceProxyXA.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&filepath=rm-datasource%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Fdatasource%2Fxa%2FDataSourceProxyXA.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvcm0vZGF0YXNvdXJjZS94YS9EYXRhU291cmNlUHJveHlYQS5qYXZh) | 16.66% | [8 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...he/seata/rm/datasource/combine/CombineContext.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&filepath=rm-datasource%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Fdatasource%2Fcombine%2FCombineContext.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvcm0vZGF0YXNvdXJjZS9jb21iaW5lL0NvbWJpbmVDb250ZXh0LmphdmE=) | 35.71% | [9 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ache/seata/rm/datasource/xa/ConnectionProxyXA.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&filepath=rm-datasource%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Fdatasource%2Fxa%2FConnectionProxyXA.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvcm0vZGF0YXNvdXJjZS94YS9Db25uZWN0aW9uUHJveHlYQS5qYXZh) | 0.00% | [3 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ava/org/apache/seata/core/context/RootContext.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&filepath=core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fcore%2Fcontext%2FRootContext.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvY29yZS9jb250ZXh0L1Jvb3RDb250ZXh0LmphdmE=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ation/tx/api/combine/CombineAutoConfiguration.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&filepath=integration-tx-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fintegration%2Ftx%2Fapi%2Fcombine%2FCombineAutoConfiguration.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW50ZWdyYXRpb24tdHgtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zZWF0YS9pbnRlZ3JhdGlvbi90eC9hcGkvY29tYmluZS9Db21iaW5lQXV0b0NvbmZpZ3VyYXRpb24uamF2YQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 2.x #7509 +/- ##
============================================
- Coverage 60.36% 60.29% -0.08%
+ Complexity 662 658 -4
============================================
Files 1284 1302 +18
Lines 48465 49166 +701
Branches 5694 5789 +95
============================================
+ Hits 29258 29646 +388
- Misses 16594 16880 +286
- Partials 2613 2640 +27
```
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...ava/org/apache/seata/core/context/RootContext.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&filepath=core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fcore%2Fcontext%2FRootContext.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvY29yZS9jb250ZXh0L1Jvb3RDb250ZXh0LmphdmE=) | `80.95% <0.00%> (-1.31%)` | :arrow_down: |
| [...ation/tx/api/combine/CombineAutoConfiguration.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&filepath=integration-tx-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fintegration%2Ftx%2Fapi%2Fcombine%2FCombineAutoConfiguration.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW50ZWdyYXRpb24tdHgtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zZWF0YS9pbnRlZ3JhdGlvbi90eC9hcGkvY29tYmluZS9Db21iaW5lQXV0b0NvbmZpZ3VyYXRpb24uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ache/seata/rm/datasource/xa/ConnectionProxyXA.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&filepath=rm-datasource%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Fdatasource%2Fxa%2FConnectionProxyXA.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvcm0vZGF0YXNvdXJjZS94YS9Db25uZWN0aW9uUHJveHlYQS5qYXZh) | `48.20% <0.00%> (-1.54%)` | :arrow_down: |
| [...he/seata/rm/datasource/combine/CombineContext.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&filepath=rm-datasource%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Fdatasource%2Fcombine%2FCombineContext.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvcm0vZGF0YXNvdXJjZS9jb21iaW5lL0NvbWJpbmVDb250ZXh0LmphdmE=) | `35.71% <35.71%> (ø)` | |
| [...ache/seata/rm/datasource/xa/DataSourceProxyXA.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&filepath=rm-datasource%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Fdatasource%2Fxa%2FDataSourceProxyXA.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvcm0vZGF0YXNvdXJjZS94YS9EYXRhU291cmNlUHJveHlYQS5qYXZh) | `64.40% <16.66%> (-12.68%)` | :arrow_down: |
| [...rm/datasource/combine/CombineConnectionHolder.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&filepath=rm-datasource%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Fdatasource%2Fcombine%2FCombineConnectionHolder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvcm0vZGF0YXNvdXJjZS9jb21iaW5lL0NvbWJpbmVDb25uZWN0aW9uSG9sZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...eata/integration/tx/api/combine/CombineAspect.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&filepath=integration-tx-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fintegration%2Ftx%2Fapi%2Fcombine%2FCombineAspect.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW50ZWdyYXRpb24tdHgtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zZWF0YS9pbnRlZ3JhdGlvbi90eC9hcGkvY29tYmluZS9Db21iaW5lQXNwZWN0LmphdmE=) | `0.00% <0.00%> (ø)` | |
... and [25 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-seata/pull/7509/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
[](https://app.codecov.io/gh/apache/incubator-seata/pull/7509?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "PeppaO (via GitHub)" <gi...@apache.org>.
PeppaO commented on code in PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#discussion_r2250434292
##########
integration-tx-api/src/main/java/org/apache/seata/integration/tx/api/interceptor/handler/CombineTransactionalInterceptorHandler.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.integration.tx.api.interceptor.handler;
+
+import org.apache.seata.config.CachedConfigurationChangeListener;
+import org.apache.seata.config.ConfigurationChangeEvent;
+import org.apache.seata.core.context.RootContext;
+import org.apache.seata.core.model.BranchType;
+import org.apache.seata.integration.tx.api.interceptor.InvocationHandlerType;
+import org.apache.seata.integration.tx.api.interceptor.InvocationWrapper;
+import org.apache.seata.integration.tx.api.interceptor.SeataInterceptorPosition;
+import org.apache.seata.integration.tx.api.util.ClassUtils;
+import org.apache.seata.rm.datasource.combine.CombineConnectionHolder;
+import org.apache.seata.rm.datasource.combine.CombineContext;
+import org.apache.seata.rm.datasource.xa.ConnectionProxyXA;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Method;
+import java.util.Set;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+
+/**
+ * The type Combine transactional interceptor handler.
+ *
+ */
+public class CombineTransactionalInterceptorHandler extends AbstractProxyInvocationHandler
+ implements CachedConfigurationChangeListener {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(CombineTransactionalInterceptorHandler.class);
+
+ private Set<String> methodsToProxy;
+
+ private static volatile ScheduledThreadPoolExecutor executor;
+
+ public CombineTransactionalInterceptorHandler(Set<String> methodsToProxy) {
+ this.methodsToProxy = methodsToProxy;
+ }
+
+ @Override
+ protected Object doInvoke(InvocationWrapper invocation) throws Throwable {
+ Class<?> targetClass = invocation.getTarget().getClass();
+ Method specificMethod = ClassUtils.getMostSpecificMethod(invocation.getMethod(), targetClass);
+ if (specificMethod != null && !specificMethod.getDeclaringClass().equals(Object.class)) {
+ return handleCombineTransactional(invocation);
+ }
+ return invocation.proceed();
+ }
+
+ private Object handleCombineTransactional(final InvocationWrapper methodInvocation) throws Throwable {
+ if (!RootContext.inGlobalTransaction()) {
+ // not in transaction, or this interceptor is disabled
+ return methodInvocation.proceed();
+ }
+ RootContext.bindBranchType(BranchType.XA);
+ if (!CombineContext.set()) {
+ // The same global transaction, the aspect does not need to enter
+ return methodInvocation.proceed();
+ }
+
+ try {
+ // First cut entry
+ Object result = methodInvocation.proceed();
+
+ // doCleanupAfterCompletion marks the end of the transaction, resets and closes the connection
+ CombineContext.clear();
+ // doCommit
+ for (ConnectionProxyXA conn : CombineConnectionHolder.getDsConn()) {
+ conn.commit();
+ }
+ return result;
+ } catch (Exception e) {
+ LOGGER.error(
+ String.format(
+ "@CombineTransactional failed to handle,xid: %s occur exp msg: %s",
+ RootContext.getXID(), e.getMessage()),
+ e);
+ CombineContext.clear();
+ // doRollback
+ for (ConnectionProxyXA conn : CombineConnectionHolder.getDsConn()) {
+ conn.rollback();
+ }
+ throw e;
+ } finally {
+ CombineContext.clear();
+ for (ConnectionProxyXA conn : CombineConnectionHolder.getDsConn()) {
+ try {
+ // Reset autocommit (if not autocommitting)
+ if (!conn.getAutoCommit()) {
+ conn.setAutoCommit(true);
+ }
+ } catch (Throwable t) {
+ // Record the exception of resetting the auto-commit, but do not interrupt and continue to try to
+ // close
+ LOGGER.error("Failed to reset autoCommit to true for connection: {}", conn, t);
+ }
+ try {
+ if (conn.isClosed()) {
+ LOGGER.warn("Connection is closed: {}", conn);
+ }
+ conn.close();
+ } catch (Throwable t) {
+ // Record the exception of closing the connection, but do not interrupt the loop and continue to
+ // process the next connection
+ LOGGER.error("Failed to close connection: {}", conn, t);
+ }
+ }
+ // Clean up local cache connections
+ CombineConnectionHolder.clear();
+ RootContext.unbindBranchType();
+ }
+ }
+
+ @Override
+ public void onChangeEvent(ConfigurationChangeEvent event) {}
+
+ @Override
+ public Set<String> getMethodsToProxy() {
+ return methodsToProxy;
+ }
+
+ @Override
+ public SeataInterceptorPosition getPosition() {
+ return SeataInterceptorPosition.AfterTransaction;
+ }
+
+ @Override
+ public String type() {
+ return InvocationHandlerType.CombineTransactional.name();
+ }
+
+ @Override
+ public int order() {
+ return 1;
Review Comment:
yes，globalTrasnactional‘s order num is 0
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "PeppaO (via GitHub)" <gi...@apache.org>.
PeppaO commented on code in PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#discussion_r2256426154
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/combine/CombineConnectionHolder.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.rm.datasource.combine;
+
+import org.apache.seata.core.context.RootContext;
+import org.apache.seata.rm.datasource.xa.ConnectionProxyXA;
+
+import javax.sql.DataSource;
+import java.sql.SQLException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class CombineConnectionHolder {
+ private static final ThreadLocal<Map<String, Map<Object, ConnectionProxyXA>>> CONNECTION_HOLDER =
Review Comment:
I think there may be asynchronous or parallel branches.
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "PeppaO (via GitHub)" <gi...@apache.org>.
PeppaO commented on PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#issuecomment-3055640944
> Please update all comments written in Chinese to English.
done
> mvn spotless:apply
done
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#discussion_r2256184792
##########
integration-tx-api/src/main/java/org/apache/seata/integration/tx/api/interceptor/handler/CombineTransactionalInterceptorHandler.java:
##########
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.integration.tx.api.interceptor.handler;
+
+import org.apache.seata.config.CachedConfigurationChangeListener;
+import org.apache.seata.config.ConfigurationChangeEvent;
+import org.apache.seata.core.context.RootContext;
+import org.apache.seata.integration.tx.api.interceptor.InvocationHandlerType;
+import org.apache.seata.integration.tx.api.interceptor.InvocationWrapper;
+import org.apache.seata.integration.tx.api.interceptor.SeataInterceptorPosition;
+import org.apache.seata.integration.tx.api.util.ClassUtils;
+import org.apache.seata.rm.datasource.combine.CombineConnectionHolder;
+import org.apache.seata.rm.datasource.xa.ConnectionProxyXA;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Method;
+import java.util.Set;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+
+/**
+ * The type Combine transactional interceptor handler.
+ *
+ */
+public class CombineTransactionalInterceptorHandler extends AbstractProxyInvocationHandler
Review Comment:
Why implement CachedConfigurationChangeListener?
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/combine/CombineConnectionHolder.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.rm.datasource.combine;
+
+import org.apache.seata.core.context.RootContext;
+import org.apache.seata.rm.datasource.xa.ConnectionProxyXA;
+
+import javax.sql.DataSource;
+import java.sql.SQLException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class CombineConnectionHolder {
+ private static final ThreadLocal<Map<String, Map<Object, ConnectionProxyXA>>> CONNECTION_HOLDER =
+ ThreadLocal.withInitial(ConcurrentHashMap::new);
+
+ public static ConnectionProxyXA get(DataSource dataSource) {
+ Map<Object, ConnectionProxyXA> connMap = CONNECTION_HOLDER.get().get(RootContext.getXID());
+ if (connMap != null) {
+ return connMap.get(dataSource);
+ }
+ return null;
+ }
+
+ public static Collection<ConnectionProxyXA> getDsConn() {
+ Map<Object, ConnectionProxyXA> connectionMap = CONNECTION_HOLDER.get().get(RootContext.getXID());
+ return connectionMap != null ? connectionMap.values() : Collections.emptyList();
+ }
+
+ public static void putConnection(DataSource dataSource, ConnectionProxyXA connection) throws SQLException {
+ Map<String, Map<Object, ConnectionProxyXA>> concurrentHashMap = CONNECTION_HOLDER.get();
+ String xid = RootContext.getXID();
+ Map<Object, ConnectionProxyXA> connectionProxyMap =
+ concurrentHashMap.computeIfAbsent(xid, k -> new ConcurrentHashMap<>());
Review Comment:
ditto
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/xa/ConnectionProxyXA.java:
##########
@@ -239,6 +244,9 @@ public void commit() throws SQLException {
@Override
public void rollback() throws SQLException {
+ if (combine) {
Review Comment:
If a rollback is issued during a two-phase commit, would there be any issues if the branch is still running?
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/combine/CombineConnectionHolder.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.rm.datasource.combine;
+
+import org.apache.seata.core.context.RootContext;
+import org.apache.seata.rm.datasource.xa.ConnectionProxyXA;
+
+import javax.sql.DataSource;
+import java.sql.SQLException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class CombineConnectionHolder {
+ private static final ThreadLocal<Map<String, Map<Object, ConnectionProxyXA>>> CONNECTION_HOLDER =
Review Comment:
Why use ConcurrentHashMap? Will there be concurrent situations?
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "chuzhe-as-a-dev (via GitHub)" <gi...@apache.org>.
chuzhe-as-a-dev commented on code in PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#discussion_r2480145849
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/xa/ConnectionProxyXA.java:
##########
@@ -161,6 +163,7 @@ public void xaRollback(String xid, long branchId, String applicationData) throws
* @throws XAException XAException
*/
public void xaRollback(XAXid xaXid) throws XAException {
+ xaEnd(xaXid, XAResource.TMFAIL);
Review Comment:
Hi! Can you explain a bit why this `xaEnd()` is added?
I'm experiencing issues with this line ending a MySQL branch, where the DB throws an exception saying that end is not allowed when the branch is in a NON-EXISTING state.
Without this line, everything works just fine since Seata 2.0.0.
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "PeppaO (via GitHub)" <gi...@apache.org>.
PeppaO commented on code in PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#discussion_r2250707781
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/combine/CombineContext.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.rm.datasource.combine;
+
+import org.apache.seata.core.context.RootContext;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class CombineContext {
Review Comment:
done
##########
integration-tx-api/src/main/java/org/apache/seata/spring/annotation/CombineTransactional.java:
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.spring.annotation;
+
+import java.lang.annotation.*;
+
+@Target({ElementType.TYPE, ElementType.METHOD})
+@Retention(RetentionPolicy.RUNTIME)
+@Documented
+public @interface CombineTransactional {}
Review Comment:
done
##########
integration-tx-api/src/main/java/org/apache/seata/integration/tx/api/interceptor/handler/CombineTransactionalInterceptorHandler.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.integration.tx.api.interceptor.handler;
+
+import org.apache.seata.config.CachedConfigurationChangeListener;
+import org.apache.seata.config.ConfigurationChangeEvent;
+import org.apache.seata.core.context.RootContext;
+import org.apache.seata.core.model.BranchType;
+import org.apache.seata.integration.tx.api.interceptor.InvocationHandlerType;
+import org.apache.seata.integration.tx.api.interceptor.InvocationWrapper;
+import org.apache.seata.integration.tx.api.interceptor.SeataInterceptorPosition;
+import org.apache.seata.integration.tx.api.util.ClassUtils;
+import org.apache.seata.rm.datasource.combine.CombineConnectionHolder;
+import org.apache.seata.rm.datasource.combine.CombineContext;
+import org.apache.seata.rm.datasource.xa.ConnectionProxyXA;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Method;
+import java.util.Set;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+
+/**
+ * The type Combine transactional interceptor handler.
+ *
+ */
+public class CombineTransactionalInterceptorHandler extends AbstractProxyInvocationHandler
+ implements CachedConfigurationChangeListener {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(CombineTransactionalInterceptorHandler.class);
+
+ private Set<String> methodsToProxy;
+
+ private static volatile ScheduledThreadPoolExecutor executor;
+
+ public CombineTransactionalInterceptorHandler(Set<String> methodsToProxy) {
+ this.methodsToProxy = methodsToProxy;
+ }
+
+ @Override
+ protected Object doInvoke(InvocationWrapper invocation) throws Throwable {
+ Class<?> targetClass = invocation.getTarget().getClass();
+ Method specificMethod = ClassUtils.getMostSpecificMethod(invocation.getMethod(), targetClass);
+ if (specificMethod != null && !specificMethod.getDeclaringClass().equals(Object.class)) {
+ return handleCombineTransactional(invocation);
+ }
+ return invocation.proceed();
+ }
+
+ private Object handleCombineTransactional(final InvocationWrapper methodInvocation) throws Throwable {
+ if (!RootContext.inGlobalTransaction()) {
+ // not in transaction, or this interceptor is disabled
+ return methodInvocation.proceed();
+ }
+ RootContext.bindBranchType(BranchType.XA);
+ if (!CombineContext.set()) {
Review Comment:
done
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes merged PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "PeppaO (via GitHub)" <gi...@apache.org>.
PeppaO commented on code in PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#discussion_r2256429787
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/xa/ConnectionProxyXA.java:
##########
@@ -239,6 +244,9 @@ public void commit() throws SQLException {
@Override
public void rollback() throws SQLException {
+ if (combine) {
Review Comment:
There may be a situation where the connection is not reset and the process is automatically submitted. This has been resolved
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "PeppaO (via GitHub)" <gi...@apache.org>.
PeppaO commented on code in PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#discussion_r2256431501
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/combine/CombineConnectionHolder.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.rm.datasource.combine;
+
+import org.apache.seata.core.context.RootContext;
+import org.apache.seata.rm.datasource.xa.ConnectionProxyXA;
+
+import javax.sql.DataSource;
+import java.sql.SQLException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class CombineConnectionHolder {
+ private static final ThreadLocal<Map<String, Map<Object, ConnectionProxyXA>>> CONNECTION_HOLDER =
+ ThreadLocal.withInitial(ConcurrentHashMap::new);
+
+ public static ConnectionProxyXA get(DataSource dataSource) {
+ Map<Object, ConnectionProxyXA> connMap = CONNECTION_HOLDER.get().get(RootContext.getXID());
+ if (connMap != null) {
+ return connMap.get(dataSource);
+ }
+ return null;
+ }
+
+ public static Collection<ConnectionProxyXA> getDsConn() {
+ Map<Object, ConnectionProxyXA> connectionMap = CONNECTION_HOLDER.get().get(RootContext.getXID());
+ return connectionMap != null ? connectionMap.values() : Collections.emptyList();
+ }
+
+ public static void putConnection(DataSource dataSource, ConnectionProxyXA connection) throws SQLException {
+ Map<String, Map<Object, ConnectionProxyXA>> concurrentHashMap = CONNECTION_HOLDER.get();
+ String xid = RootContext.getXID();
+ Map<Object, ConnectionProxyXA> connectionProxyMap =
+ concurrentHashMap.computeIfAbsent(xid, k -> new ConcurrentHashMap<>());
Review Comment:
done
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "YongGoose (via GitHub)" <gi...@apache.org>.
YongGoose commented on PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#issuecomment-3055267748
Please update all comments written in Chinese to English.
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "PeppaO (via GitHub)" <gi...@apache.org>.
PeppaO commented on code in PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#discussion_r2233880917
##########
integration-tx-api/pom.xml:
##########
@@ -61,7 +61,14 @@
<groupId>net.bytebuddy</groupId>
<artifactId>byte-buddy</artifactId>
</dependency>
-
+ <dependency>
+ <groupId>org.springframework</groupId>
Review Comment:
done
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#discussion_r2250317477
##########
integration-tx-api/src/main/java/org/apache/seata/spring/annotation/CombineTransactional.java:
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.spring.annotation;
+
+import java.lang.annotation.*;
Review Comment:
Do not use * for imports.
##########
integration-tx-api/src/main/java/org/apache/seata/integration/tx/api/interceptor/handler/CombineTransactionalInterceptorHandler.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.integration.tx.api.interceptor.handler;
+
+import org.apache.seata.config.CachedConfigurationChangeListener;
+import org.apache.seata.config.ConfigurationChangeEvent;
+import org.apache.seata.core.context.RootContext;
+import org.apache.seata.core.model.BranchType;
+import org.apache.seata.integration.tx.api.interceptor.InvocationHandlerType;
+import org.apache.seata.integration.tx.api.interceptor.InvocationWrapper;
+import org.apache.seata.integration.tx.api.interceptor.SeataInterceptorPosition;
+import org.apache.seata.integration.tx.api.util.ClassUtils;
+import org.apache.seata.rm.datasource.combine.CombineConnectionHolder;
+import org.apache.seata.rm.datasource.combine.CombineContext;
+import org.apache.seata.rm.datasource.xa.ConnectionProxyXA;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Method;
+import java.util.Set;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+
+/**
+ * The type Combine transactional interceptor handler.
+ *
+ */
+public class CombineTransactionalInterceptorHandler extends AbstractProxyInvocationHandler
+ implements CachedConfigurationChangeListener {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(CombineTransactionalInterceptorHandler.class);
+
+ private Set<String> methodsToProxy;
+
+ private static volatile ScheduledThreadPoolExecutor executor;
+
+ public CombineTransactionalInterceptorHandler(Set<String> methodsToProxy) {
+ this.methodsToProxy = methodsToProxy;
+ }
+
+ @Override
+ protected Object doInvoke(InvocationWrapper invocation) throws Throwable {
+ Class<?> targetClass = invocation.getTarget().getClass();
+ Method specificMethod = ClassUtils.getMostSpecificMethod(invocation.getMethod(), targetClass);
+ if (specificMethod != null && !specificMethod.getDeclaringClass().equals(Object.class)) {
+ return handleCombineTransactional(invocation);
+ }
+ return invocation.proceed();
+ }
+
+ private Object handleCombineTransactional(final InvocationWrapper methodInvocation) throws Throwable {
+ if (!RootContext.inGlobalTransaction()) {
+ // not in transaction, or this interceptor is disabled
+ return methodInvocation.proceed();
+ }
+ RootContext.bindBranchType(BranchType.XA);
+ if (!CombineContext.set()) {
+ // The same global transaction, the aspect does not need to enter
+ return methodInvocation.proceed();
+ }
+
+ try {
+ // First cut entry
+ Object result = methodInvocation.proceed();
+
+ // doCleanupAfterCompletion marks the end of the transaction, resets and closes the connection
+ CombineContext.clear();
+ // doCommit
+ for (ConnectionProxyXA conn : CombineConnectionHolder.getDsConn()) {
+ conn.commit();
+ }
+ return result;
+ } catch (Exception e) {
+ LOGGER.error(
+ String.format(
+ "@CombineTransactional failed to handle,xid: %s occur exp msg: %s",
+ RootContext.getXID(), e.getMessage()),
+ e);
+ CombineContext.clear();
+ // doRollback
+ for (ConnectionProxyXA conn : CombineConnectionHolder.getDsConn()) {
+ conn.rollback();
+ }
+ throw e;
+ } finally {
+ CombineContext.clear();
+ for (ConnectionProxyXA conn : CombineConnectionHolder.getDsConn()) {
+ try {
+ // Reset autocommit (if not autocommitting)
+ if (!conn.getAutoCommit()) {
+ conn.setAutoCommit(true);
+ }
+ } catch (Throwable t) {
+ // Record the exception of resetting the auto-commit, but do not interrupt and continue to try to
+ // close
+ LOGGER.error("Failed to reset autoCommit to true for connection: {}", conn, t);
+ }
+ try {
+ if (conn.isClosed()) {
+ LOGGER.warn("Connection is closed: {}", conn);
+ }
+ conn.close();
+ } catch (Throwable t) {
+ // Record the exception of closing the connection, but do not interrupt the loop and continue to
+ // process the next connection
+ LOGGER.error("Failed to close connection: {}", conn, t);
+ }
+ }
+ // Clean up local cache connections
+ CombineConnectionHolder.clear();
+ RootContext.unbindBranchType();
+ }
+ }
+
+ @Override
+ public void onChangeEvent(ConfigurationChangeEvent event) {}
+
+ @Override
+ public Set<String> getMethodsToProxy() {
+ return methodsToProxy;
+ }
+
+ @Override
+ public SeataInterceptorPosition getPosition() {
+ return SeataInterceptorPosition.AfterTransaction;
+ }
+
+ @Override
+ public String type() {
+ return InvocationHandlerType.CombineTransactional.name();
+ }
+
+ @Override
+ public int order() {
+ return 1;
Review Comment:
`
Collections.sort(invocationHandlerList, Comparator.comparingInt(ProxyInvocationHandler::order));
`
If used together with the `@GlobalTransactional` annotation, it should execute after it.
##########
integration-tx-api/src/main/java/org/apache/seata/spring/annotation/CombineTransactional.java:
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.spring.annotation;
+
+import java.lang.annotation.*;
+
+@Target({ElementType.TYPE, ElementType.METHOD})
+@Retention(RetentionPolicy.RUNTIME)
+@Documented
+public @interface CombineTransactional {}
Review Comment:
It is necessary to add a comment indicating that only the XA mode is currently supported.
##########
integration-tx-api/src/main/java/org/apache/seata/integration/tx/api/interceptor/handler/CombineTransactionalInterceptorHandler.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.integration.tx.api.interceptor.handler;
+
+import org.apache.seata.config.CachedConfigurationChangeListener;
+import org.apache.seata.config.ConfigurationChangeEvent;
+import org.apache.seata.core.context.RootContext;
+import org.apache.seata.core.model.BranchType;
+import org.apache.seata.integration.tx.api.interceptor.InvocationHandlerType;
+import org.apache.seata.integration.tx.api.interceptor.InvocationWrapper;
+import org.apache.seata.integration.tx.api.interceptor.SeataInterceptorPosition;
+import org.apache.seata.integration.tx.api.util.ClassUtils;
+import org.apache.seata.rm.datasource.combine.CombineConnectionHolder;
+import org.apache.seata.rm.datasource.combine.CombineContext;
+import org.apache.seata.rm.datasource.xa.ConnectionProxyXA;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Method;
+import java.util.Set;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+
+/**
+ * The type Combine transactional interceptor handler.
+ *
+ */
+public class CombineTransactionalInterceptorHandler extends AbstractProxyInvocationHandler
+ implements CachedConfigurationChangeListener {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(CombineTransactionalInterceptorHandler.class);
+
+ private Set<String> methodsToProxy;
+
+ private static volatile ScheduledThreadPoolExecutor executor;
+
+ public CombineTransactionalInterceptorHandler(Set<String> methodsToProxy) {
+ this.methodsToProxy = methodsToProxy;
+ }
+
+ @Override
+ protected Object doInvoke(InvocationWrapper invocation) throws Throwable {
+ Class<?> targetClass = invocation.getTarget().getClass();
+ Method specificMethod = ClassUtils.getMostSpecificMethod(invocation.getMethod(), targetClass);
+ if (specificMethod != null && !specificMethod.getDeclaringClass().equals(Object.class)) {
+ return handleCombineTransactional(invocation);
+ }
+ return invocation.proceed();
+ }
+
+ private Object handleCombineTransactional(final InvocationWrapper methodInvocation) throws Throwable {
+ if (!RootContext.inGlobalTransaction()) {
+ // not in transaction, or this interceptor is disabled
+ return methodInvocation.proceed();
+ }
+ RootContext.bindBranchType(BranchType.XA);
+ if (!CombineContext.set()) {
Review Comment:
Why not use RootContext directly? For example, add a method like inCombineTransaction to RootContext.
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/combine/CombineContext.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.rm.datasource.combine;
+
+import org.apache.seata.core.context.RootContext;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class CombineContext {
Review Comment:
Can CombineContext be merged with CombineConnectionHolder? In fact, if the current connection exists in CombineConnectionHolder, it already indicates that transaction commit/rollback needs to be merged.
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on code in PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#discussion_r2211938968
##########
integration-tx-api/pom.xml:
##########
@@ -61,7 +61,14 @@
<groupId>net.bytebuddy</groupId>
<artifactId>byte-buddy</artifactId>
</dependency>
-
+ <dependency>
+ <groupId>org.springframework</groupId>
Review Comment:
It should not rely on Spring, but instead refer to the existing proxy approach and use byte-buddy for proxying.
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "PeppaO (via GitHub)" <gi...@apache.org>.
PeppaO commented on code in PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#discussion_r2250706730
##########
integration-tx-api/src/main/java/org/apache/seata/spring/annotation/CombineTransactional.java:
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.spring.annotation;
+
+import java.lang.annotation.*;
Review Comment:
done
--
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] feature: Reuse connection to merge branch transactions [incubator-seata]
Posted by "PeppaO (via GitHub)" <gi...@apache.org>.
PeppaO commented on code in PR #7509:
URL: https://github.com/apache/incubator-seata/pull/7509#discussion_r2256427211
##########
integration-tx-api/src/main/java/org/apache/seata/integration/tx/api/interceptor/handler/CombineTransactionalInterceptorHandler.java:
##########
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.integration.tx.api.interceptor.handler;
+
+import org.apache.seata.config.CachedConfigurationChangeListener;
+import org.apache.seata.config.ConfigurationChangeEvent;
+import org.apache.seata.core.context.RootContext;
+import org.apache.seata.integration.tx.api.interceptor.InvocationHandlerType;
+import org.apache.seata.integration.tx.api.interceptor.InvocationWrapper;
+import org.apache.seata.integration.tx.api.interceptor.SeataInterceptorPosition;
+import org.apache.seata.integration.tx.api.util.ClassUtils;
+import org.apache.seata.rm.datasource.combine.CombineConnectionHolder;
+import org.apache.seata.rm.datasource.xa.ConnectionProxyXA;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Method;
+import java.util.Set;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+
+/**
+ * The type Combine transactional interceptor handler.
+ *
+ */
+public class CombineTransactionalInterceptorHandler extends AbstractProxyInvocationHandler
Review Comment:
Already deleted
--
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