Skip to content

[rpc] Handle endpoint changes for cached server connections - #4263

Open
BackendArchitectX wants to merge 2 commits into
apache:mainfrom
BackendArchitectX:fix-4256-stale-tabletserver-endpoint
Open

[rpc] Handle endpoint changes for cached server connections#4263
BackendArchitectX wants to merge 2 commits into
apache:mainfrom
BackendArchitectX:fix-4256-stale-tabletserver-endpoint

Conversation

@BackendArchitectX

@BackendArchitectX BackendArchitectX commented Sep 9, 2026

Copy link
Copy Markdown

Purpose

Linked issue: #4256

NettyClient currently caches connections only by server UID. During a rolling upgrade, the same TabletServer UID may be advertised with a different host or port while the previous endpoint remains reachable.

In that case, even after metadata is refreshed with the new endpoint, connection lookup can continue returning the connection to the previous endpoint. Requests may therefore keep reaching the wrong TabletServer.

This change makes the physical RPC connection identity endpoint-aware by using server UID, host, and port.

This is related to #4216, which also handles same-UID endpoint changes but replaces the existing connection. This change keeps connections to different physical endpoints independently keyed so the RPC layer does not have to decide which reachable endpoint is newer.

Brief change log

  • Key cached Netty connections by server UID, host, and port instead of server UID alone.
  • Allow different physical endpoints for the same logical server UID to use independent connections.
  • Make readiness checks endpoint-specific using ServerNode.
  • Add disconnect(ServerNode) for disconnecting a specific endpoint.
  • Keep disconnect(serverUid) as a logical-server operation that closes all connections associated with that UID.
  • Use endpoint-specific connection metric identities so multiple connections sharing a UID do not overwrite each other's metric bookkeeping.
  • Add regression coverage verifying that a changed endpoint for the same UID is actually used while the previous endpoint remains reachable.
  • Add coverage for endpoint-specific readiness and disconnect behavior.
  • Add lifecycle coverage verifying that disconnecting a UID closes all associated endpoint connections.

Tests

  • .\mvnw.cmd test -Dtest=NettyClientTest -pl fluss-rpc

    • 10 tests passed
    • 0 failures
    • 0 errors
  • .\mvnw.cmd verify -pl fluss-rpc -Dtest="!ApiErrorTest"

    • 90 tests run
    • 0 failures
    • 0 errors
    • 2 skipped
  • Checkstyle passed with 0 violations.

  • Spotless passed.

  • git diff --check passed.

A full fluss-rpc verify on Windows also encounters the existing ApiErrorTest#testStringifyException line-ending assertion, which expects LF while the Windows JVM stack trace uses CRLF. This test is unrelated to this change.

The regression coverage intentionally isolates the transport-level condition: two reachable TabletServers use the same UID with different endpoints, and requests using the updated ServerNode reach the updated endpoint rather than the previously cached one.

The tests also verify that readiness and endpoint-specific disconnect operate on the exact serverUid + host + port connection, while UID-level disconnect continues to close all physical connections associated with the logical server.

API and Format

No user-facing API, RPC protocol, or storage format changes. The internal RpcClient lifecycle API is aligned with endpoint-aware connection identity.

Documentation

No documentation changes. This fixes existing RPC connection behavior when a server endpoint changes.

Generative AI disclosure

Yes - ChatGPT (GPT-5.6 Sol) was used for root-cause analysis, implementation review, and test planning.

@BackendArchitectX

Copy link
Copy Markdown
Author

@wuchong @loserwang1024 @swuferhong @luoyuxia, would appreciate your thoughts on the connection-handling approach in this PR. It overlaps with the same-UID endpoint handling discussed in #4216, but keeps physical connections keyed by UID + host + port instead of replacing the existing UID connection. Any feedback on the preferred direction would be very helpful.

@gyang94

gyang94 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

@BackendArchitectX Thanks! Using serverId + host + port as the composite key for the cache seems like a good approach. It will help differentiate servers with both legacy and new host addresses.
I am thinking that do we need to add methods in NettyClient like isReady(ServerNode node) and disconnect(ServerNode node), to match the new key semantics. What do you think?

Comment thread fluss-rpc/src/main/java/org/apache/fluss/rpc/netty/client/NettyClient.java Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants