You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "litiliu (via GitHub)" <gi...@apache.org> on 2026/09/03 06:44:24 UTC
[PR] [client] Recover admin writes after coordinator leader failover [fluss]
litiliu opened a new pull request, #4216:
URL: https://github.com/apache/fluss/pull/4216
<!-- Generated-by: GitHub Copilot (Claude Opus 4.8) following the guidelines -->
### Purpose
Linked issue: close #4027
A long-lived Java `Admin` client caches the coordinator leader in `MetadataUpdater`. After coordinator leadership moves to a standby, coordinator write operations (e.g. `dropDatabase`, `createTable`, `alterTable`) keep being sent to the old coordinator and fail with `NotCoordinatorLeaderException` until the connection is recreated.
Two root causes:
1. `FlussAdmin` wrapped only the read-only gateway with `RetryableGatewayClientProxy`; the write gateway never refreshed metadata after a failover.
2. Even after refreshing to the new leader's address, `NettyClient` reused the connection cached under the coordinator uid `cs-0` (both coordinators share id `0`), so requests kept hitting the old leader that is still alive as a standby.
This is an alternative implementation of #4200 and incorporates @loserwang1024's review feedback there: refresh metadata on any recoverable error, but auto-retry only the provably-safe `NotCoordinatorLeaderException`.
### Brief change log
- `RetryableGatewayClientProxy` now takes **separate `refreshPredicate` and `retryPredicate`**. On failure it refreshes metadata when `refreshPredicate` matches, and additionally retries once only when `retryPredicate` matches. Read-only gateways keep using `RetriableException` for both (unchanged behavior).
- `FlussAdmin` write gateway:
- `refreshPredicate` = `NotCoordinatorLeaderException || RetriableException` — refreshes metadata on any recoverable error, including `NetworkException`/`TimeoutException` when the old coordinator's IP is not reused after an upgrade.
- `retryPredicate` = `NotCoordinatorLeaderException` only — safe to auto-retry because `FlussRequestHandler` rejects the request before invoking the write API, so the mutation was never executed. Network/timeout failures refresh metadata but surface the original error for a manual retry (non-idempotent-safe).
- `NettyClient` recreates the connection when the address behind a server uid changes (a coordinator failover reuses `cs-0` at a new host/port) and closes the stale connection.
### Tests
Run: `./mvnw -pl fluss-rpc,fluss-client -am -Dtest=RetryableGatewayClientProxyTest,NettyClientTest,CoordinatorFailoverAdminITCase -DfailIfNoSpecifiedTests=false test`
- `RetryableGatewayClientProxyTest`: retry on the safe error; refresh-but-no-retry on network errors; no refresh/retry when neither predicate matches.
- `NettyClientTest#testReconnectWhenServerAddressChangesForSameUid`: a same-uid address change reconnects instead of reusing the stale connection.
- `CoordinatorFailoverAdminITCase#testAdminWriteRecoversAfterCoordinatorFailover`: keeps one `Admin` open across a coordinator leader failover and verifies `dropDatabase` succeeds afterward.
### API and Format
No public API or storage format changes. `RetryableGatewayClientProxy` (`@Internal`) gains a two-predicate factory overload; the existing signature is preserved.
### Documentation
No. Bug fix; no user-facing feature or documentation change.
---
**Generative AI disclosure:** Yes — GitHub Copilot (Claude Opus 4.8) was used to help author this PR.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [client] Recover admin writes after coordinator leader failover [fluss]
Posted by "BackendArchitectX (via GitHub)" <gi...@apache.org>.
BackendArchitectX commented on PR #4216:
URL: https://github.com/apache/fluss/pull/4216#issuecomment-5603897700
@litiliu, I'm working on #4256 in #4263 and noticed that #4216 explicitly intends the NettyClient address-change handling to cover TabletServer rolling upgrades as well.
The two PRs therefore overlap on the same transport behavior but use different policies: #4216 replaces and closes the connection for a UID, while #4263 keys physical connections by UID + host + port.
I'd like to avoid competing fixes. Would you be open to coordinating on which direction should land? I'm happy to adapt #4263 or move its deterministic regression coverage to the preferred implementation.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [client] Recover admin writes after coordinator leader failover [fluss]
Posted by "litiliu (via GitHub)" <gi...@apache.org>.
litiliu closed pull request #4216: [client] Recover admin writes after coordinator leader failover
URL: https://github.com/apache/fluss/pull/4216
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [client] Recover admin writes after coordinator leader failover [fluss]
Posted by "litiliu (via GitHub)" <gi...@apache.org>.
litiliu commented on PR #4216:
URL: https://github.com/apache/fluss/pull/4216#issuecomment-5611608501
@BackendArchitectX Thanks for pointing this out. After comparing the two approaches, I agree that keying physical connections by UID + host + port is cleaner at the transport layer. NettyClient receives a complete ServerNode and should not need to decide which endpoint is newer. It also avoids concurrent requests based on old and new metadata repeatedly replacing each other’s connections. Unused old connections will eventually be cleaned up by the configured idle timeout.
I suggest letting #4263 own the endpoint-change handling and its deterministic regression coverage. I’ll update #4216 to remove the overlapping NettyClient changes while retaining the Admin metadata refresh/retry policy and the coordinator failover integration test.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [client] Recover admin writes after coordinator leader failover [fluss]
Posted by "litiliu (via GitHub)" <gi...@apache.org>.
litiliu commented on PR #4216:
URL: https://github.com/apache/fluss/pull/4216#issuecomment-5534751831
@loserwang1024 Could you take a look on this?
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org