Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bugfix: in dubbo 3.x version, the consumer can't generate tcc proxy. #6101

Merged
merged 27 commits into from
Dec 18, 2023
Merged

bugfix: in dubbo 3.x version, the consumer can't generate tcc proxy. #6101

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
ef782b3
undo_log table check optimize
laywin Nov 14, 2023
1df81eb
optimize & add modify record
laywin Nov 14, 2023
8e92325
adjust indentation
laywin Nov 14, 2023
7482281
undo_log table check optimize
laywin Nov 14, 2023
522c386
optimize & add modify record
laywin Nov 14, 2023
0444780
adjust indentation
laywin Nov 14, 2023
881aaf4
add unit test
laywin Nov 21, 2023
b8f822c
move pr record
laywin Nov 21, 2023
fc2e79f
Merge remote-tracking branch 'origin/2.x' into 2.x
laywin Nov 21, 2023
e36326e
modify unit test
laywin Nov 21, 2023
0bf3847
add Github ID
laywin Nov 23, 2023
47d150d
Merge branch '2.x' into 2.x
funky-eyes Nov 25, 2023
12f8109
1. update mockito version to 4.5.1 same as seata server to support st…
laywin Nov 26, 2023
c8a6115
Merge remote-tracking branch 'origin/2.x' into 2.x
laywin Nov 26, 2023
c5761c7
import optimize
laywin Nov 26, 2023
572192b
1. add dubbo 3.x's proxy name to optimize proxy judge.
laywin Dec 3, 2023
53f3070
Merge branch '2.x' of github.com:seata/seata into 2.x
laywin Dec 3, 2023
2a01c04
add change registry info
laywin Dec 3, 2023
6513a35
add try other app logic in TCC mode when TC send order of two phase
laywin Dec 3, 2023
184e05a
fix unit test
laywin Dec 3, 2023
27f1db4
rollback tryotherapp param
laywin Dec 4, 2023
61df543
rm Unused import
laywin Dec 4, 2023
5efb325
optimize tcc judge
laywin Dec 6, 2023
988bb7a
1. rm try other app when send tc's order
laywin Dec 7, 2023
056c3f0
Merge branch '2.x' into 2.x
funky-eyes Dec 16, 2023
60b4adf
add the try other app back && allow at mode to enable
laywin Dec 17, 2023
a6af930
add try other app back
laywin Dec 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/en-us/2.x.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Add changes here for all PR submitted to the 2.x branch.
- [[#6075](https://github.com/seata/seata/pull/6075)] fix missing table alias for on update column of image SQL
- [[#6086](https://github.com/seata/seata/pull/6086)] fix oracle column alias cannot find
- [[#6085](https://github.com/seata/seata/pull/6085)] fix jdk9+ compile error
- [[#6101](https://github.com/seata/seata/pull/6101)] fix the consumer can't generate tcc proxy in dubbo 3.x version

### optimize:
- [[#6061](https://github.com/seata/seata/pull/6061)] merge the rpcMergeMessageSend threads of rm and tm and increase the thread hibernation duration
Expand Down
1 change: 1 addition & 0 deletions changes/zh-cn/2.x.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- [[#6075](https://github.com/seata/seata/pull/6075)] 修复镜像SQL对于on update列没有添加表别名的问题
- [[#6086](https://github.com/seata/seata/pull/6086)] 修复oracle alias 解析异常
- [[#6085](https://github.com/seata/seata/pull/6085)] 修复jdk9+版本编译后，引入后ByteBuffer#flip NoSuchMethodError的问题
- [[#6101](https://github.com/seata/seata/pull/6101)] 修复在dubbo 3.x的版本中, 消费者端不能生成tcc代理的问题

### optimize:
- [[#6061](https://github.com/seata/seata/pull/6061)] 合并rm和tm的rpcMergeMessageSend线程，增加线程休眠时长
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/io/seata/core/rpc/RemotingServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package io.seata.core.rpc;

import io.netty.channel.Channel;
import io.seata.core.model.BranchType;
import io.seata.core.protocol.RpcMessage;
import io.seata.core.rpc.processor.RemotingProcessor;

Expand All @@ -37,11 +38,11 @@ public interface RemotingServer {
* @param resourceId rm client resourceId
* @param clientId rm client id
* @param msg transaction message {@code io.seata.core.protocol}
* @param tryOtherApp try other app
* @param branchType branchType
* @return client result message
* @throws TimeoutException TimeoutException
*/
Object sendSyncRequest(String resourceId, String clientId, Object msg, boolean tryOtherApp) throws TimeoutException;
Object sendSyncRequest(String resourceId, String clientId, Object msg, BranchType branchType) throws TimeoutException;

/**
* server send sync request.
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/io/seata/core/rpc/netty/AbstractNettyRemotingServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import io.netty.handler.timeout.IdleState;
import io.netty.handler.timeout.IdleStateEvent;
import io.seata.common.util.NetUtil;
import io.seata.core.model.BranchType;
import io.seata.core.protocol.HeartbeatMessage;
import io.seata.core.protocol.ProtocolConstants;
import io.seata.core.protocol.RpcMessage;
Expand Down Expand Up @@ -63,9 +64,9 @@ public AbstractNettyRemotingServer(ThreadPoolExecutor messageExecutor, NettyServ
}

@Override
public Object sendSyncRequest(String resourceId, String clientId, Object msg, boolean tryOtherApp)
public Object sendSyncRequest(String resourceId, String clientId, Object msg, BranchType branchType)
throws TimeoutException {
Channel channel = ChannelManager.getChannel(resourceId, clientId, tryOtherApp);
Channel channel = ChannelManager.getChannel(resourceId, clientId, branchType);
if (channel == null) {
throw new RuntimeException("rm client is not connected. dbkey:" + resourceId + ",clientId:" + clientId);
}
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/io/seata/core/rpc/netty/ChannelManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import io.seata.common.util.CollectionUtils;
import io.seata.common.util.NetUtil;
import io.seata.common.util.StringUtils;
import io.seata.core.model.BranchType;
import io.seata.core.protocol.IncompatibleVersionException;
import io.seata.core.protocol.RegisterRMRequest;
import io.seata.core.protocol.RegisterTMRequest;
Expand Down Expand Up @@ -287,7 +288,7 @@ private static Channel getChannelFromSameClientMap(Map<Integer, RpcContext> clie
* @param clientId Client ID - ApplicationId:IP:Port
* @return Corresponding channel, NULL if not found.
*/
public static Channel getChannel(String resourceId, String clientId, boolean tryOtherApp) {
public static Channel getChannel(String resourceId, String clientId, BranchType branchType) {
Channel resultChannel = null;

String[] clientIdInfo = readClientId(clientId);
Expand Down Expand Up @@ -399,7 +400,7 @@ public static Channel getChannel(String resourceId, String clientId, boolean try
}
}

if (resultChannel == null && tryOtherApp) {
if (resultChannel == null && BranchType.TCC == branchType) {
Copy link
Contributor

Choose a reason for hiding this comment

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

为什么要改变入参，而不是在由调用方决定是否开启tryOtherApp
Why change the entry parameter when it's up to the caller to turn on tryOtherApp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i feel the place convention over configuration is better, method signature is better understood without loss of flexibility.
这个地方感觉用 预定大于配置 的方法更好一点，不失灵活性的同时方法签名也更好理解.

Copy link
Contributor

Choose a reason for hiding this comment

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

i feel the place convention over configuration is better, method signature is better understood without loss of flexibility. 这个地方感觉用 预定大于配置 的方法更好一点，不失灵活性的同时方法签名也更好理解.

I'm not sure if there are any other branch types that need this in the future, but if they all rely on an internal if || it's just too low.
未来不确定有没有其他的分支类型需要这种，如果都靠内部一个if || 这种写法太low了

Copy link
Member

Choose a reason for hiding this comment

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

What about tryOtherApp in AT transaction mode? The original logic is that only AT mode can go here, now only TCC mode can go 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.

What about tryOtherApp in AT transaction mode? The original logic is that only AT mode can go here, now only TCC mode can go here?

yeah, because of TCC rpc mode regiter branch in constomer, send order of TC two stage to provider. so it can't find the channel as normal, so tcc model will try other app. @funky-eyes 's opinion there may be a risk in at to find a same resource diff app channel, ex: two instance have status.

是的，因为TCC RPC模式是在消费者端注册分支，TC在二阶段下发指令给服务者端。导致通过消费者端的app去找服务者端的channel的时候会失败，所以tcc 模式会尝试其他app channel. @funky-eyes 的建议在AT模式下发指令到相同资源不同app的端可能存在风险, 比如两台实例可能有状态的情况下.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i feel the place convention over configuration is better, method signature is better understood without loss of flexibility. 这个地方感觉用 预定大于配置 的方法更好一点，不失灵活性的同时方法签名也更好理解.

I'm not sure if there are any other branch types that need this in the future, but if they all rely on an internal if || it's just too low. 未来不确定有没有其他的分支类型需要这种，如果都靠内部一个if || 这种写法太low了

no problem, i will rollback.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about tryOtherApp in AT transaction mode? The original logic is that only AT mode can go here, now only TCC mode can go here?

yeah, because of TCC rpc mode regiter branch in constomer, send order of TC two stage to provider. so it can't find the channel as normal, so tcc model will try other app. @funky-eyes 's opinion there may be a risk in at to find a same resource diff app channel, ex: two instance have status.

是的，因为TCC RPC模式是在消费者端注册分支，TC在二阶段下发指令给服务者端。导致通过消费者端的app去找服务者端的channel的时候会失败，所以tcc 模式会尝试其他app channel. @funky-eyes 的建议在AT模式下发指令到相同资源不同app的端可能存在风险, 比如两台实例可能有状态的情况下.

Maybe this is not the scope of this bugfix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about tryOtherApp in AT transaction mode? The original logic is that only AT mode can go here, now only TCC mode can go here?

yeah, because of TCC rpc mode regiter branch in constomer, send order of TC two stage to provider. so it can't find the channel as normal, so tcc model will try other app. @funky-eyes 's opinion there may be a risk in at to find a same resource diff app channel, ex: two instance have status.
是的，因为TCC RPC模式是在消费者端注册分支，TC在二阶段下发指令给服务者端。导致通过消费者端的app去找服务者端的channel的时候会失败，所以tcc 模式会尝试其他app channel. @funky-eyes 的建议在AT模式下发指令到相同资源不同app的端可能存在风险, 比如两台实例可能有状态的情况下.

Maybe this is not the scope of this bugfix.

tcc rpc mode can't find the channel, if no this change.

resultChannel = tryOtherApp(applicationIdMap, targetApplicationId);

if (resultChannel == null) {
Expand Down
4 changes: 3 additions & 1 deletion integration-tx-api/src/main/java/io/seata/integration/tx/api/util/DubboUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ private DubboUtil() {
private static final String ALIBABA_DUBBO_PROXY_NAME_PREFIX = "com.alibaba.dubbo.common.bytecode.proxy";
private static final String APACHE_DUBBO_PROXY_NAME_PREFIX = "org.apache.dubbo.common.bytecode.proxy";

private static final String DUBBO_3_X_PARTIAL_PROXY_NAME = "DubboProxy";
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you only use contains? and not startsWith or endsWith?
只能使用contains？而不是startsWith或者endsWith吗？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

there is no fixed prefix or suffix in generated proxy name, i will try later to concat a prefix to judge.
生成代理名字的时候没有固定的前缀和后缀，我看能不能凭借一个前缀来判断.

Copy link
Member

Choose a reason for hiding this comment

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

Deeply analyzing the source code is really excellent.


/**
* get the interface class of the dubbo proxy which be generated by javaassist
*
Expand Down Expand Up @@ -69,6 +71,6 @@ public static Class<?> getAssistInterface(Object proxyBean)
}

public static boolean isDubboProxyName(String name) {
return name.startsWith(ALIBABA_DUBBO_PROXY_NAME_PREFIX) || name.startsWith(APACHE_DUBBO_PROXY_NAME_PREFIX);
return name.startsWith(ALIBABA_DUBBO_PROXY_NAME_PREFIX) || name.startsWith(APACHE_DUBBO_PROXY_NAME_PREFIX) || name.contains(DUBBO_3_X_PARTIAL_PROXY_NAME);
}
}
4 changes: 2 additions & 2 deletions server/src/main/java/io/seata/server/coordinator/AbstractCore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ protected BranchStatus branchCommitSend(BranchCommitRequest request, GlobalSessi
BranchSession branchSession) throws IOException, TimeoutException {

BranchCommitResponse response = (BranchCommitResponse) remotingServer.sendSyncRequest(
branchSession.getResourceId(), branchSession.getClientId(), request, branchSession.isAT());
branchSession.getResourceId(), branchSession.getClientId(), request, branchSession.getBranchType());
return response.getBranchStatus();
}

Expand All @@ -217,7 +217,7 @@ protected BranchStatus branchRollbackSend(BranchRollbackRequest request, GlobalS
BranchSession branchSession) throws IOException, TimeoutException {

BranchRollbackResponse response = (BranchRollbackResponse) remotingServer.sendSyncRequest(
branchSession.getResourceId(), branchSession.getClientId(), request, branchSession.isAT());
branchSession.getResourceId(), branchSession.getClientId(), request, branchSession.getBranchType());
return response.getBranchStatus();
}

Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/io/seata/server/coordinator/DefaultCoordinatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ static Stream<Arguments> xidAndBranchIdProviderForRollback() throws Exception {
public static class MockServerMessageSender implements RemotingServer {

@Override
public Object sendSyncRequest(String resourceId, String clientId, Object message, boolean tryOtherApp)
public Object sendSyncRequest(String resourceId, String clientId, Object message, BranchType branchType)
throws TimeoutException {
if (message instanceof BranchCommitRequest) {
final BranchCommitResponse branchCommitResponse = new BranchCommitResponse();
Expand Down
Loading