xucq07 opened a new pull request, #8167: URL: https://github.com/apache/incubator-seata/pull/8167
- [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 `SEATA_IP` (or `--host`) is set to a host name, the xid carries the host name instead of an ip address, e.g. `seata.aaa.com:8091:123456`. `Server#start` validates the host with `NetUtil.isValidIp`, which internally calls `convertIpIfNecessary` to resolve a host name to its ip address before validating it. But the validation result is thrown away and the **original** string is stored: ```java if (NetUtil.isValidIp(parameterParser.getHost(), false)) { XID.setIpAddress(parameterParser.getHost()); // the raw host name } ``` The host name then propagates into every xid, and `XIDLoadBalance#select` has to build an `InetSocketAddress` from it on every branch register to compare against the resolved addresses in the channel list. That has three consequences: 1. A DNS lookup on the branch register hot path. The JVM positive cache defaults to 30s, so this is a recurring cost rather than a one-off. 2. If the lookup fails at select time, the `InetSocketAddress` is unresolved. `InetSocketAddress#equals` then compares host name strings against resolved invokers, never matches, and every branch register silently degrades to the random fallback — sticky routing to the tc owning the global transaction is lost. 3. If the name has multiple A records, `getByName` returns the first one, which may point at a different tc than the one that created the xid. This PR resolves the host name once, at startup, so the xid always carries an ip literal: - Store `NetUtil.convertIpIfNecessary(host)` instead of the raw host. `isValidIp` on the preceding line already performed the same resolution, so this hits the JVM cache and adds no startup cost. - Log the `host name -> ip` mapping at INFO, which makes case 3 above diagnosable. - Make `NetUtil#convertIpIfNecessary` public (it was private and only used by `isValidIp`); behaviour unchanged. `XIDLoadBalance` is deliberately left alone. Resolving there would repeat work already done by the `InetSocketAddress` constructor and would keep the DNS lookup on the hot path; fixing the value at the source removes the problem for all three cases at once. The other three `XID.setIpAddress` call sites already pass ip literals (`NetUtil.getIgnoredInterfacesLocalIp` / `NetUtil.getLocalIp`), so `Server.java` is the only place where a host name can reach an xid. ### Ⅱ. Does this pull request fix one issue? fixes #8158 Note that the root cause differs from the analysis in the issue: `new InetSocketAddress(host, port)` does resolve the name, and `InetSocketAddress#equals` compares the resolved `InetAddress` while ignoring the host name, so a host name and an ip do match in the common case. The failure only appears when resolution fails at select time or when the name resolves to a different address, as described above. ### Ⅲ. Why don't you add test cases (unit test/integration test)? Added `NetUtilTest#testConvertIpIfNecessary`, covering the ipv4/ipv6 literal pass-through, host name resolution (using `localhost`, so it does not depend on external DNS) and the unresolvable host case. ### Ⅳ. Describe how to verify it `mvn -pl common -am test -Dtest=NetUtilTest` End to end: start a server with `SEATA_IP` set to a resolvable host name and check that the generated xid carries the ip address rather than the name, and that the startup log records the resolution. ### Ⅴ. Special notes for reviews `convertIpIfNecessary` throws an unchecked `RuntimeException` wrapping `UnknownHostException` when the host name cannot be resolved, so an unresolvable `SEATA_IP` fails server startup instead of falling back to the local ip branch. That behaviour is pre-existing — `isValidIp` on the preceding line already throws for the same input — and this PR does not change it. It seems worth addressing separately. -- 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]
