fresh-borzoni commented on PR #369:
URL: https://github.com/apache/fluss-rust/pull/369#issuecomment-3994170938
Thanks for working on this @toxicteddy00077!
The direction is right, we want to cache the admin like Java does. But we
need to fix one thing first: FlussAdmin currently stores a concrete
ServerConnection at construction time, which means a cached admin would be
stuck with a dead connection if the coordinator restarts. Java avoids this
because its FlussAdmin resolves the coordinator per-call via
GatewayClientProxy.
Here's how to do it:
\\ admin.rs:
* Remove admin_gateway field, remove both #[allow(dead_code)], add
#[derive(Clone)] (now safe — struct is just two Arcs)
* Make new() sync, just store the arcs
* Add a helper that resolves the coordinator connection per-call, smth like
this
```rust
async fn admin_gateway(&self) -> Result<ServerConnection> {
let coordinator = self
.metadata
.get_cluster()
.get_coordinator_server()
.ok_or_else(|| Error::UnexpectedError {
message: "Coordinator server not found in cluster
metadata".to_string(),
source: None,
})?;
self.rpc_client.get_connection(coordinator).await
}
```
* Update all methods, mechanical
// before
self.admin_gateway.request(SomeRequest::new(...)).await?;
// after
self.admin_gateway().await?.request(SomeRequest::new(...)).await?;
// connection.rs:
Add cache field admin_client: RwLock<Option<FlussAdmin>> and use
double-checked locking in get_admin(), same pattern as
`get_or_create_writer_client()` right above it.
Just get_admin() should return Result and and stay async and doesn't await
internally, so that it's compatible with previous signature.
But if we can still break the signature, I would just go with Arc and sync
method, and no Clone derive with Arc
cc @luoyuxia
Public API stays async fn get_admin(&self) -> Result<FlussAdmin>, bindings
untouched.
--
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]
