-
Notifications
You must be signed in to change notification settings - Fork 8.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
optimize: branch register resource only at RM server end #5399
Conversation
没有关联issue吗 |
} | ||
|
||
// eliminate aop proxy. | ||
if (AopUtils.isAopProxy(target)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the past, the result of being processed by the RPC framework would have been either a Reference Bean or a Service Bean. However, if you are not currently using this RPC framework handling, how do you ensure that the class is not proxy or that the TwoPhaseBusinessAction has the highest priority order?
在过去,被RPC框架处理后,最终得到的结果肯定是Reference Bean或者Service Bean。但是,如果当前不再使用这种RPC框架的处理方式来判断,如何保证这个类没有被代理过或者怎么保证TwoPhaseBusinessAction的切面优先级是最高的?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
之前这种方式确实有问题,不能保证,以下以一种思路:
利用spring 的 bean definition,提前在自动代理前得到各个类的引用关系,从而提前获取依赖关系,保存名字在Set中,然后在AOP自动代理的wrapIfNecessary中,判断是否存在。
在RM端,Set中保存的是 service指向的原生bean的name。
在TM端,Set中保存的是开启事务的bean的name。
private Set<String> tccProxyTargetMethod(RemotingDesc remotingDesc) { | ||
if (!remotingDesc.isReference() || remotingDesc == null) { | ||
|
||
private Set<String> tccProxyTargetMethod(Object target) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to handle method overloading with the same method name
如何处理方法名相同都标注TwoPhaseBusinessAction注解的方法重载问题。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to handle method overloading with the same method name
如何处理方法名相同都标注TwoPhaseBusinessAction注解的方法重载问题。
这个问题将由后续pr解决
This issue will be addressed by a follow-up PR
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## 2.x #5399 +/- ##
============================================
- Coverage 48.98% 48.87% -0.12%
+ Complexity 4782 4781 -1
============================================
Files 913 915 +2
Lines 31697 31762 +65
Branches 3823 3829 +6
============================================
- Hits 15528 15523 -5
- Misses 14632 14709 +77
+ Partials 1537 1530 -7
|
This reverts commit 5cca5c4.
# Conflicts: # tcc/src/main/java/io/seata/rm/tcc/resource/parser/TccRegisterResourceParser.java
…nterceptor/parser/IfNeedEnhanceBean.java
...ion-tx-api/src/main/java/io/seata/integration/tx/api/interceptor/parser/NeedEnhanceEnum.java
Outdated
Show resolved
Hide resolved
…nterceptor/parser/NeedEnhanceEnum.java
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## 2.x #5399 +/- ##
============================================
- Coverage 50.00% 49.84% -0.16%
+ Complexity 4932 4930 -2
============================================
Files 915 917 +2
Lines 31927 31995 +68
Branches 3855 3862 +7
============================================
- Hits 15966 15949 -17
- Misses 14400 14486 +86
+ Partials 1561 1560 -1
|
# Conflicts: # integration-tx-api/src/main/java/io/seata/integration/tx/api/interceptor/parser/DefaultInterfaceParser.java
…o resolve_fence_to_rm
… be proxied by Tcc interceptor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
经过测试符合预期,但是如果在使用tcc时,在prepare上增加globaltransactional注解时,会使TwoPhaseBusinessAction注解失效,当然这个应该是另一个问题,应该后续进行关注
Tested as expected, but if you use tcc, adding globaltransactional annotations to prepare will invalidate TwoPhaseBusinessAction annotations. Of course, this should be another issue and should be followed up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
if (ifNeedEnhanceBean.getNeedEnhanceEnum().equals(NeedEnhanceEnum.SERVICE_BEAN)) { | ||
// the native bean which dubbo, sofa bean service bean referenced | ||
PropertyValue propertyValue = beanDefinition.getPropertyValues().getPropertyValue("ref"); | ||
if (propertyValue == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这些逻辑看起来应该和parser强相关,是不是该挪到parser去?虽然这样做会带来一部分重复的代码
This logic seems like it should be strongly related to the parser, so should it be moved to the parser, even though it would be a duplicate piece of code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
如果引入到parser中,可能会给parser带来beanDefinition的spring依赖,原则上可能不想让parser层引入spring相关的依赖
|
||
private NeedEnhanceEnum needEnhanceEnum; | ||
|
||
public boolean isIfNeed() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is need/isNeed() better than ifNeed/isIfNeed() ?
integration-tx-api/src/main/java/io/seata/integration/tx/api/util/ProxyUtil.java
Outdated
Show resolved
Hide resolved
…til/ProxyUtil.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Ⅰ. Describe what this PR did
optimize fence prepare from tm to rm.
At rm, only enhence the native bean, exclude all of proxy bean.
Ⅱ. Does this pull request fix one issue?
fiex #5596
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews