feat(rpc): implement WatchNode bidirectional streaming service (RFE370)#371
Conversation
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 real-time capabilities of the system by introducing a new bidirectional streaming gRPC service. This service replaces an inefficient polling mechanism, aiming to drastically reduce latency and server load by pushing updates to clients as they occur. The architectural shift involves comprehensive changes across both the backend and client components, focusing on a more reactive and efficient communication model for node and executor status. The design prioritizes a state-based approach for updates, empowering clients to intelligently process information and adapt to changes. Highlights
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
|
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-designed feature to replace polling with a bidirectional gRPC streaming service (WatchNode) for executor status updates. The changes are comprehensive, including design documents, a test plan, and modifications across the client and server to adopt the new streaming approach. The implementation of WatchRegistry for managing streams and the refactoring of the ExecutorManager to be event-driven are well-executed. However, I've identified two high-severity issues: a potential resource leak on the server due to a missing stream timeout, and a bug in the client's async task handling that will cause it to block indefinitely. Addressing these will be crucial for the stability and correctness of the new service.
…s#370) - Add WatchNode gRPC service with bidirectional streaming for executor status updates - Implement WatchRegistry in session_manager for subscription management - Add stream_handler in executor_manager for handling streaming connections - Remove polling mode, enforce streaming-only communication - Refactor to send Executor directly in WatchNodeResponse (remove ExecutorAction) - Add server-side heartbeat timeout to prevent resource leaks - Fix client-side StreamHandler to handle errors internally as self-recovering task - Add comprehensive test plan and design documentation Fixes xflops#370 Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Fixes #370