WangzJi opened a new pull request, #8183: URL: https://github.com/apache/incubator-seata/pull/8183
- [x] I have read the [CONTRIBUTING.md](https://github.com/apache/incubator-seata/blob/2.x/CONTRIBUTING.md) guidelines. - [x] I have registered the PR [changes](https://github.com/apache/incubator-seata/tree/2.x/changes). ### Ⅰ. Describe what this PR did When a RM/TM registers to a TC, the TC always echoes its own version back in the register response (`AbstractIdentifyResponse#version`, transferred by both the v0/v1 and the v2 codec as well as by the protobuf convertor). The client, however, only wrote that version into a log line, so upper layers had no way to know which features the target server supports. This is the client side counterpart of `RpcContext#getVersion()`, which the server has been using for compatibility decisions such as `ServerOnRequestProcessor` and `AbstractNettyRemotingServer`. This PR adds that missing context: - **Add `ServerVersionHolder`** (`org.apache.seata.core.protocol`), keyed by the server address used to connect, so that any module can ask whether a given TC is above or equal to a version. An unknown version is deliberately treated as "not satisfied", so a feature relying on the check stays disabled until the version is really known. - **Record the server version on both RM and TM registration.** `RmNettyRemotingClient` fed a private map introduced by #8020, while `TmNettyRemotingClient` did not record the version at all. Both now write to the holder. - **Remove the package private `serverVersionMap` in `NettyClientChannelManager`.** It was only reachable from within `org.apache.seata.core.rpc.netty` and only fed by the RM path, so it could not serve feature detection outside the rpc layer. Its only consumer, the `UnregisterRMRequest` version check, now uses the holder. - **Pass the peer version to `registerChannel`.** The client used to store *its own* version in `Version#VERSION_MAP` keyed by the *server* address, while the server stores the *peer* version keyed by the client address. `Version#getChannelVersion` now means "the version of the peer" on both sides. Entries in the holder are written on every successful registration and are never removed on disconnection. This is intentional: TM and RM reach the same servers through two independent channel managers, so removing an entry when one of them disconnects would break feature detection for the other one. A version is an intrinsic attribute of the server at a given address, and a stale entry can never be read for a live request, because a request always goes through a channel whose registration has just refreshed the entry. ### Ⅱ. Does this pull request fix one issue? No. ### Ⅲ. Why don't you add test cases (unit test/integration test)? Test cases are added: - `ServerVersionHolderTest`: put/get, overwrite on re-registration, blank input ignored, unknown server treated as not satisfied, `clear`. - `TmNettyClientTest#onRegisterMsgSuccessRecordsServerVersionTest`: new, covers the previously missing TM path. - `RmNettyClientTest`: asserts that the peer version is the one handed to `registerChannel` and that it lands in the holder; the `UnregisterRMRequest` version cases now drive the holder instead of stubbing the removed manager method. - `NettyClientChannelManagerTest`: the pure version cases moved to `ServerVersionHolderTest`, the destroy/cleanup cases keep asserting the channel, pool key and lock cleanup. ### Ⅳ. Describe how to verify it ``` ./mvnw -pl core -am test -Dtest='ServerVersionHolderTest,VersionTest,NettyClientChannelManagerTest,RmNettyClientTest,TmNettyClientTest' ``` For a behavioural check, start a TC and a client, and observe that after registration `ServerVersionHolder.getServerVersion(serverAddress)` returns the TC version for both TM and RM connections, and that the `UnregisterRMRequest` on client destroy is still sent only to servers of 2.6.0 and above. ### Ⅴ. Special notes for reviews The change to `registerChannel` is the only behavioural change outside the new holder, and it is safe: the sole client side reader of `Version#VERSION_MAP` is `MsgVersionHelper#versionNotSupport`, whose `SKIP_MSG_CODE_V0` contains only `TYPE_RM_DELETE_UNDOLOG`, a message sent by the TC to the RM and never in the opposite direction. The check is therefore inert on the client side today, and correcting the stored version aligns the two sides instead of changing runtime behaviour. `ServerVersionHolder` is kept separate from `Version` on purpose. `Version#VERSION_MAP` is keyed by `channel.remoteAddress()`, whereas the holder is keyed by the server address obtained from the registry; the two are not always equal (NAT, proxy, container port mapping), so merging them into one class would invite mixing the keys up. `Version#VERSION_2_6_0` is promoted from private to public so that the comparison target can be expressed as a constant at the call site. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
