feat: add multi-address load balancing and failover support#106
feat: add multi-address load balancing and failover support#106
Conversation
- Add multi-address URL parser (common/urlParser.ts) supporting IPv4/IPv6 Format: ws://user:pass@host1:port1,host2:port2,[::1]:port3?retries=5 - Add ConnectionManager (client/wsConnectionManager.ts) with: - Random host selection for initial connection - Failover with round-robin across hosts - Exponential backoff retry (retries, retry_backoff_ms, retry_backoff_max_ms) - Inflight request tracking and resend on reconnect - Enhance WebSocketConnector with onClose callback for fault detection - Refactor WsClient to use ConnectionManager for multi-address connections - Update WSConfig to carry ParsedMultiAddress - Update getUrl() to parse multi-address URLs - Update TmqConfig to support multi-address URLs - Add unit tests for URL parser and ConnectionManager Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the WebSocket client's robustness and availability by introducing comprehensive multi-address support with automatic load balancing and failover capabilities. It allows the client to connect to multiple server addresses, intelligently select an initial connection, and seamlessly switch to an alternative host with retry logic if the current connection fails. This change improves the client's resilience against network issues or server outages, ensuring a more stable and continuous operation. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant and well-structured functionality for multi-address load balancing and failover, effectively encapsulating complex logic with a robust ConnectionManager and custom URL parser. However, a high-severity SQL injection vulnerability was identified where the database name extracted from the connection URL is used unsafely in internal SQL queries. It is recommended to validate the database name and use proper escaping or parameterized queries. Additionally, there are a few suggestions for code cleanup and stricter error handling to further improve the implementation.
| if (parsed.pathname && parsed.pathname !== "/") { | ||
| wsConfig.setDb(parsed.pathname.slice(1)); | ||
| } |
There was a problem hiding this comment.
The database name is extracted from the URL pathname and stored in the WSConfig object without any validation or sanitization. This database name is later used in nodejs/src/sql/wsSql.ts (e.g., in stmtInit and open methods) to construct SQL queries by direct string concatenation. An attacker who can control the connection URL can inject malicious SQL commands, potentially leading to unauthorized data access or manipulation on the TDengine server. For example, a database name like mydb' OR '1'='1 could bypass intended filters in metadata queries.
| if (isNaN(port)) { | ||
| // Might be hostname without port that contains something odd, treat whole as host | ||
| results.push({ host: trimmed, port: DEFAULT_PORT }); | ||
| } else { | ||
| results.push({ host, port }); | ||
| } |
There was a problem hiding this comment.
The current logic for handling an invalid port for a hostname or IPv4 address is to treat the entire segment (e.g., 1.2.3.4:abc) as the hostname and use a default port. This can hide typos in the URL and lead to confusing connection errors. For consistency with the IPv6 parsing logic, which throws an error for an invalid port, and to provide clearer feedback to the user, it would be better to throw an error here if the port is not a valid number.
if (isNaN(port)) {
throw new TDWebSocketClientError(
ErrorCode.ERR_INVALID_URL,
`Invalid port in: ${trimmed}`
);
} else {
results.push({ host, port });
}| private _isReconnecting: boolean = false; | ||
| private _reconnectPromise: Promise<WebSocketConnector> | null = null; |
There was a problem hiding this comment.
These private properties, _isReconnecting and _reconnectPromise, are declared but never used. They appear to be remnants of a previous implementation and can be safely removed to improve code clarity. The _reconnectMutex already handles the concurrency control for reconnections.
nodejs/src/client/wsConnector.ts
Outdated
| // ？？？ | ||
| // 连接建立成功后，发送连接成功的消息，触发后续的事件回调 |
There was a problem hiding this comment.
These comments, including // ？？？, seem to be temporary developer notes. They should be removed from the final code to improve readability and maintainability. Similar notes are present on lines 174-180 and 199-201 and should also be cleaned up.
…etConnector 1. Simplify urlParser: replace bracket-aware scanning functions with straightforward @/slash/query position-based host:port extraction. All existing test cases still pass. 2. Refactor WebSocketConnector: change constructor from URL to string, parse multi-address internally via parseMultiAddressUrl(). Merge ConnectionManager failover (round-robin + exponential backoff), reconnect, and inflight request tracking directly into WebSocketConnector. 3. Simplify WsClient: remove ConnectionManager dependency and dual code paths (multi-address vs single-address). WsClient now passes raw URL string to WebSocketConnector, which handles everything uniformly. 4. Update consumers: wsSql, wsTmq/TmqConfig, wsConnectorPool, config, utils all updated to use string URLs instead of URL objects. 5. Delete wsConnectionManager.ts — all its behavior now lives in WebSocketConnector. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 participant
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.