-
Notifications
You must be signed in to change notification settings - Fork 313
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
feat: flow recreate on reboot #4509
Conversation
WalkthroughThe updates enhance flow management across multiple components, integrating a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StartCommand
participant FlowMetadataManager
participant FlownodeBuilder
User->>StartCommand: Initiate Flow
StartCommand->>FlowMetadataManager: Create Instance
StartCommand->>FlownodeBuilder: Pass FlowMetadataManager
FlownodeBuilder->>FlowMetadataManager: Access Metadata
FlownodeBuilder->>User: Execute Flow Operations
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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4509 +/- ##
==========================================
- Coverage 84.97% 84.73% -0.24%
==========================================
Files 1081 1084 +3
Lines 193362 193935 +573
==========================================
+ Hits 164314 164340 +26
- Misses 29048 29595 +547 |
We must be focused on such basic functions. Keep pushing and make the flow better. |
@coderabbitai review |
Actions performedReview triggered.
|
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.yaml
Review profile: CHILL
Files selected for processing (7)
- src/cmd/src/flownode.rs (2 hunks)
- src/cmd/src/standalone.rs (2 hunks)
- src/flow/src/error.rs (2 hunks)
- src/flow/src/server.rs (7 hunks)
- tests-integration/src/standalone.rs (1 hunks)
- tests/cases/standalone/common/flow/flow_basic.result (1 hunks)
- tests/cases/standalone/common/flow/flow_basic.sql (1 hunks)
Additional comments not posted (16)
tests/cases/standalone/common/flow/flow_basic.sql (2)
17-17
: Verify the purpose and placement of the SQL comment.The comment
-- SQLNESS ARG restart=true
indicates a new argument for the SQL execution environment. Ensure that it is correctly placed and does not interfere with the SQL statements.
18-20
: Verify the correctness and consistency of the data entries.The INSERT statement adds new data entries to the
numbers_input_basic
table. Ensure that the data entries are correct and consistent with the table schema.tests/cases/standalone/common/flow/flow_basic.result (2)
27-27
: Verify the purpose and placement of the SQL comment.The comment
-- SQLNESS ARG restart=true
indicates a new argument for the SQL execution environment. Ensure that it is correctly placed and does not interfere with the expected results.
28-28
: Verify the correctness and consistency of the expected results.The expected results include the affected rows and the results of the
flush_flow
andSELECT
statements. Ensure that the expected results are correct and consistent with the SQL statements inflow_basic.sql
.src/flow/src/error.rs (2)
86-92
: Verify the correctness and integration of the new error variant.The new error variant
ListFlows
includes three fields:id
,source
, andlocation
. Ensure that it is correctly defined and integrated into theError
enum.
225-226
: Verify the correctness and consistency of the modifications.The modifications include the addition of the
ListFlows
variant to the match arm that returns aStatusCode::TableNotFound
. Ensure that the modifications are correctly implemented and consistent with the error handling logic.tests-integration/src/standalone.rs (1)
159-159
: Ensureflow_metadata_manager
is correctly integrated.The addition of
flow_metadata_manager.clone()
to theFlownodeBuilder
constructor is consistent with the PR objectives. Ensure that this parameter is correctly utilized within the builder to manage flow metadata.src/cmd/src/flownode.rs (3)
27-27
: Import statement forFlowMetadataManager
.The import statement for
FlowMetadataManager
is correctly added.
300-300
: Initialization ofFlowMetadataManager
.The initialization of
FlowMetadataManager
usingcached_meta_backend
is consistent with the intended functionality. Ensure thatcached_meta_backend
is correctly configured and passed to the manager.
306-306
: Passingflow_metadata_manager
toFlownodeBuilder
.The
flow_metadata_manager
is correctly passed as a parameter to theFlownodeBuilder
. This aligns with the PR objectives of enhancing flow management.src/flow/src/server.rs (5)
32-32
: Import statement forFlowMetadataManagerRef
.The import statement for
FlowMetadataManagerRef
is correctly added.
245-245
: Addition offlow_metadata_manager
field toFlownodeBuilder
.The
flow_metadata_manager
field is correctly added to theFlownodeBuilder
struct. This enhances the builder's ability to manage flow metadata.
256-256
: Updating the constructor to initializeflow_metadata_manager
.The constructor for
FlownodeBuilder
is correctly updated to initialize theflow_metadata_manager
field.
292-294
: Error handling in flow recovery.The error handling for flow recovery is correctly implemented using the
Snafu
library. This ensures robust management of potential failures.
309-386
: Implementation ofrecover_flows
method.The
recover_flows
method is well-implemented, providing a robust approach to recovering flow tasks based on the node's operational mode. Consider parallelizing the recovery process in the future for improved performance.src/cmd/src/standalone.rs (1)
479-485
: LGTM! Verify the impact of the moved instantiation.The code changes are approved.
However, ensure that the moved instantiation of
flow_metadata_manager
does not introduce any issues or dependencies in the codebase.Verification successful
No issues found with the moved instantiation of
flow_metadata_manager
.The moved instantiation of
flow_metadata_manager
does not introduce any issues or dependencies in the codebase. The change improves code clarity and maintainability.
- The
flow_metadata_manager
is instantiated and used consistently across various files without any dependency issues.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of the moved instantiation of `flow_metadata_manager`. # Test: Search for the `flow_metadata_manager` usage. Expect: No issues or dependencies introduced. rg --type rust -A 5 $'flow_metadata_manager'Length of output: 30599
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
recreate flow on flownode on reboot
Checklist
Summary by CodeRabbit
New Features
FlowMetadataManager
into theStartCommand
functionality.recover_flows
, inFlownodeBuilder
to handle flow recovery tasks.Bug Fixes
ListFlows
variant to the error reporting system.Chores