[client] Allow bootstrap endpoint with unknown server type#622
Conversation
leekeiabstraction
left a comment
leekeiabstraction
left a comment
There was a problem hiding this comment.
Thank you for your first contribution and improving the client! Left a question.
| ) | ||
| .is_ok() | ||
| ); | ||
| assert!( |
There was a problem hiding this comment.
Should a simple integration test case be made in existing IT testing with unknown type/bootstrapping against tablet server?
There was a problem hiding this comment.
Added an integration test covering bootstrapping from a tablet server endpoint. The test uses the shared cluster's tablet plaintext listener as bootstrap_servers, creates a FlussConnection, and verifies server node discovery returns both coordinator and tablet nodes.
charlesdong1991
left a comment
charlesdong1991
left a comment
There was a problem hiding this comment.
very nice PR! 👍
small tiny comments!
| pub enum ServerType { | ||
| TabletServer, | ||
| CoordinatorServer, | ||
| Unknown, |
There was a problem hiding this comment.
i think we should update api-reference for this too
There was a problem hiding this comment.
Updated the Rust API reference to include ServerType::Unknown and documented that it is used when the client has not yet determined the endpoint type, such as bootstrap endpoints.
| match type_id { | ||
| 1 => Some(ServerType::CoordinatorServer), | ||
| 2 => Some(ServerType::TabletServer), | ||
| -1 => Some(ServerType::Unknown), |
There was a problem hiding this comment.
for any unrecognized id, should we return None or Unknown?
There was a problem hiding this comment.
Aligned this with the Java client behavior. from_type_id now returns ServerType::Unknown for any unrecognized type id instead of None, matching Java's ServerType.fromTypeId. The validation path still fails when the client expects a concrete server type and the server advertises Unknown.
| | `fn server_type(&self) -> &ServerType` | Server type (`CoordinatorServer`, `TabletServer`, or `Unknown`) | | ||
| | `fn uid(&self) -> &str` | Unique identifier (e.g. `"cs-0"`, `"ts-1"`) | | ||
|
|
||
| `ServerType::Unknown` is used when the client has not yet determined the endpoint |
There was a problem hiding this comment.
i think other bindings' api reference also have it, but maybe i am wrong
3 participants
Purpose
Linked issue: close #621
Allow the Rust client to bootstrap from either coordinator or tablet server endpoints after server type validation is enabled.
Brief change log
ServerType::Unknownfor bootstrap seed nodes.ServerType::Unknownwhen creating the initial bootstrapServerNode.Unknown.Tests
cargo fmt --all -- --checkPROTOC=/opt/homebrew/bin/protoc cargo test -p fluss-rs --lib server_type_validationPROTOC=/opt/homebrew/bin/protoc cargo test -p fluss-rs --libPROTOC=/opt/homebrew/bin/protoc cargo clippy --all-targets --workspace -- -D warningsAPI and Format
This adds a public
ServerType::Unknownenum variant. It does not change the wire protocol or storage format.Documentation
No user-facing documentation change.