-
Notifications
You must be signed in to change notification settings - Fork 711
feat: add multiple tokio runtime #4035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes significantly enhance the configurability and performance of the application by updating the Changes
Sequence Diagram(s)sequenceDiagram
participant Main
participant Job
participant GRPC_Server
Main->>Job: init()
Main->>GRPC_Server: init_common_grpc_server()
Main->>GRPC_Server: Signal readiness
Main->>Job: Start worker threads
Main->>Job: Handle shutdown
Main->>GRPC_Server: Handle shutdown
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/config/src/config.rs (3 hunks)
- src/main.rs (9 hunks)
Additional context used
Path-based instructions (2)
src/main.rs (1)
Pattern
**/*.rs: You are a smart rustlang pull request reviewer.
You are going to review all the rustlang files.
Be concise, and add a brief explanation to your suggestionsMake sure the responses are not too verbose and keep the suggestions to the point i.e. actionable.
src/config/src/config.rs (1)
Pattern
**/*.rs: You are a smart rustlang pull request reviewer.
You are going to review all the rustlang files.
Be concise, and add a brief explanation to your suggestionsMake sure the responses are not too verbose and keep the suggestions to the point i.e. actionable.
Additional comments not posted (9)
src/main.rs (7)
160-231: Ensure proper error handling in the backend job initialization.The backend job initialization logic is well-structured, but it's crucial to ensure that all potential errors are handled gracefully. Consider adding more detailed error logging for each initialization step.
233-264: Ensure proper error handling in the gRPC server initialization.The gRPC server initialization logic is well-structured, but it's crucial to ensure that all potential errors are handled gracefully. Consider adding more detailed error logging for each initialization step.
306-313: Ensure proper resource cleanup during shutdown.The shutdown procedures for the gRPC server and backend jobs are well-structured. Ensure that all resources are properly cleaned up, and consider adding more detailed logging for each shutdown step.
Line range hint
333-393: Ensure proper error handling in the common gRPC server initialization.The common gRPC server initialization logic is well-structured and asynchronous. Ensure that all potential errors are handled gracefully, and consider adding more detailed error logging for each initialization step.
Line range hint
397-433: Ensure proper error handling in the router gRPC server initialization.The router gRPC server initialization logic is well-structured and asynchronous. Ensure that all potential errors are handled gracefully, and consider adding more detailed error logging for each initialization step.
500-500: Ensure proper error handling in the HTTP server initialization.The HTTP server initialization logic is well-structured. Ensure that all potential errors are handled gracefully, and consider adding more detailed error logging for each initialization step.
578-578: Ensure proper error handling in the HTTP server initialization without tracing.The HTTP server initialization logic is well-structured. Ensure that all potential errors are handled gracefully, and consider adding more detailed error logging for each initialization step.
src/config/src/config.rs (2)
Line range hint
840-865: Ensure proper initialization of new configuration fields.The new fields for gRPC and job runtime configurations enhance the configurability of the application. Ensure that these fields are correctly initialized and used throughout the codebase.
1213-1224: Ensure default values for new configuration fields are correctly set.The default values for the new gRPC and job runtime configuration fields are set in the
initfunction. Ensure that these values are correctly applied and used throughout the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
src/common/infra/ofga.rs (2)
Line range hint
65-67:
Improve error handling inset_ofga_modelcall.Consider handling the case where
store_idis empty more gracefully, possibly by providing a more descriptive error message or taking additional actions.- if store_id.is_empty() { - log::error!("OFGA store id is empty"); - } + if store_id.is_empty() { + log::error!("Failed to set OFGA model: store id is empty"); + // Additional actions can be taken here if necessary + }
Line range hint
117-119:
Enhance logging for lock release failure.Currently, the error message for lock release failure is generic. Consider providing more context in the log message.
- .expect("Failed to release lock"); + .expect("Failed to release lock for openFGA model");
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/common/infra/cluster/etcd.rs (1 hunks)
- src/common/infra/cluster/nats.rs (1 hunks)
- src/common/infra/ofga.rs (1 hunks)
- src/infra/src/dist_lock.rs (1 hunks)
- src/service/usage/stats.rs (1 hunks)
Files skipped from review due to trivial changes (4)
- src/common/infra/cluster/etcd.rs
- src/common/infra/cluster/nats.rs
- src/infra/src/dist_lock.rs
- src/service/usage/stats.rs
Additional context used
Path-based instructions (1)
src/common/infra/ofga.rs (1)
Pattern
**/*.rs: You are a smart rustlang pull request reviewer.
You are going to review all the rustlang files.
Be concise, and add a brief explanation to your suggestionsMake sure the responses are not too verbose and keep the suggestions to the point i.e. actionable.
Additional comments not posted (1)
src/common/infra/ofga.rs (1)
62-62: Ensure the correctness of the additional parameter indist_lock::lock.The additional
Noneparameter in thedist_lock::lockcall appears to be an optional configuration or setting. Verify that this change is intentional and that thedist_lock::lockfunction handles this parameter correctly.Verification successful
Ensure the correctness of the additional parameter in
dist_lock::lock.The additional
Noneparameter in thedist_lock::lockcall is correctly handled by the function definition. Ensure that passingNonefornode_idsin this context is intentional and does not lead to unintended behavior.
src/common/infra/ofga.rsline 62: Verify the context to ensureNoneis appropriate.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the `dist_lock::lock` function handles the additional parameter correctly. # Test: Search for the `dist_lock::lock` function definition. Expect: The function definition includes handling for the additional parameter. rg --type rust -A 5 $'fn lock'Length of output: 2006
Summary by CodeRabbit
New Features
Bug Fixes
Style