feat: provide a simple way to create metaclient #4257
Conversation
fengjiachun
requested review from
zhongzc,
waynexia,
MichaelScofield and
a team
as code owners
WalkthroughThe change focuses on refactoring and renaming functions related to Changes
Poem
Tip AI model upgrade
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
src/frontend/src/instance.rs (1)
135-135: Add documentation for thecluster_idvariable.The
cluster_idvariable is currently set to0with a TODO comment. It would be helpful to add documentation explaining why it's reserved and when it should be enabled.let cluster_id = 0; // TODO: jeremy: It is currently a reserved field and has not been enabled. // This variable is reserved for future use when cluster ID functionality is implemented.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- src/cmd/src/datanode.rs (1 hunks)
- src/cmd/src/flownode.rs (1 hunks)
- src/datanode/src/heartbeat.rs (3 hunks)
- src/flow/src/heartbeat.rs (2 hunks)
- src/frontend/src/instance.rs (2 hunks)
- src/meta-client/src/lib.rs (2 hunks)
Additional comments not posted (9)
src/meta-client/src/lib.rs (3)
17-22: Imports look good.The import statements for
ChannelConfig,ChannelManager, andMetaClientBuilderare necessary for building and configuring the meta client.
63-68: EnumMetaClientTypeis well-defined.The
MetaClientTypeenum correctly differentiates between the types of meta clients and includes necessary member IDs.
70-114: Functioncreate_meta_clientis well-structured.The
create_meta_clientfunction correctly utilizes theMetaClientTypeenum to create and configure meta clients. The use ofChannelConfigandChannelManageris appropriate.src/flow/src/heartbeat.rs (2)
32-32: Imports look good.The import statements for
MetaClientOptionsandMetaClientTypeare necessary for creating and configuring the meta client.
238-249: Functionnew_meta_clientis well-structured.The
new_meta_clientfunction correctly utilizes thecreate_meta_clientfunction to create a meta client for the flownode.src/cmd/src/flownode.rs (1)
229-229: Function call tonew_meta_clientis correct.The
new_meta_clientfunction call correctly passes the necessary parameters to create a meta client for the flownode.src/datanode/src/heartbeat.rs (2)
28-29: Imports look good.The import statements for
MetaClientOptionsandMetaClientTypeare necessary for creating and configuring the meta client.
345-357: Functionnew_meta_clientis well-structured.The
new_meta_clientfunction correctly utilizes thecreate_meta_clientfunction to create a meta client for the datanode.src/cmd/src/datanode.rs (1)
286-286: Function call tonew_meta_clientis correct.The
new_meta_clientfunction call correctly passes the necessary parameters to create a meta client for the datanode.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4257 +/- ##
==========================================
- Coverage 84.97% 84.71% -0.26%
==========================================
Files 1054 1054
Lines 187154 187120 -34
==========================================
- Hits 159033 158523 -510
- Misses 28121 28597 +476 |
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- src/cmd/src/datanode.rs (2 hunks)
- src/cmd/src/error.rs (2 hunks)
- src/cmd/src/flownode.rs (2 hunks)
- src/cmd/src/frontend.rs (2 hunks)
- src/datanode/src/datanode.rs (3 hunks)
- src/datanode/src/heartbeat.rs (6 hunks)
- src/flow/src/heartbeat.rs (2 hunks)
- src/frontend/src/instance.rs (2 hunks)
- src/meta-client/src/lib.rs (2 hunks)
- tests-integration/src/cluster.rs (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- src/cmd/src/datanode.rs
- src/cmd/src/flownode.rs
- src/datanode/src/heartbeat.rs
- src/frontend/src/instance.rs
Additional comments not posted (7)
src/meta-client/src/lib.rs (2)
64-69: EnumMetaClientTypelooks good.The enum
MetaClientTypeis well-defined and follows Rust's best practices.
73-117: Ensure proper error handling and logging increate_meta_client.The function is well-structured, but it would be beneficial to verify that all potential errors are properly handled and logged.
src/cmd/src/error.rs (1)
343-348: Error variantMetaClientInitlooks good.The error variant
MetaClientInitis well-defined and follows Rust's best practices.tests-integration/src/cluster.rs (1)
321-324: Ensure consistent usage ofMetaClientRef.The update to use
MetaClientReflooks correct. Verify that theMetaClientRefusage is consistent across the codebase.Verification successful
Consistent usage of
MetaClientRefverified.The
MetaClientRefis used consistently across the codebase in the following locations:
src/datanode/src/heartbeat.rssrc/datanode/src/datanode.rssrc/meta-client/src/lib.rsThe type alias
MetaClientRefis defined asArc<client::MetaClient>, ensuring a consistent reference type across the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent usage of `MetaClientRef`. # Test: Search for `MetaClientRef` usage. rg --type rust 'MetaClientRef'Length of output: 591
src/datanode/src/datanode.rs (3)
37-37: Import ofMetaClientReflooks good.The import is necessary for the subsequent changes that utilize
MetaClientRefinstead ofMetaClient.
158-158: Update ofmeta_clientfield to useMetaClientReflooks good.This change aligns with the new approach of using
MetaClientRefand is necessary for the new initialization logic.
174-174: Update ofwith_meta_clientmethod to useMetaClientReflooks good.This change is consistent with the previous updates and necessary for the correct initialization of
MetaClientRef.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/meta-client/examples/lock.rs (6 hunks)
- src/meta-client/src/client.rs (1 hunks)
Additional comments not posted (7)
src/meta-client/examples/lock.rs (5)
Line range hint
53-75: LGTM!The function
run_normalcorrectly uses theMetaClientReftype formeta_client.
Line range hint
76-91: LGTM!The function
run_multi_threadcorrectly uses theMetaClientReftype formeta_client.
Line range hint
92-107: LGTM!The function
run_multi_thread_with_one_timeoutcorrectly uses theMetaClientReftype formeta_client.
Line range hint
108-132: LGTM!The function
run_with_timeoutcorrectly uses theMetaClientReftype formeta_client.
44-44: Ensure correct usage ofMetaClientRef.The
meta_clientis now anArc<MetaClientRef>. Verify that all usages ofmeta_clientcorrectly handle theArctype.Verification successful
Ensure correct usage of
MetaClientRef.The
meta_clientis correctly handled asArc<MetaClientRef>in the relevant files.
src/meta-client/examples/lock.rs: Themeta_clientis wrapped inArcand passed asMetaClientRefto various functions.src/meta-client/src/lib.rs: Themeta_clientis returned asArc<MetaClientRef>from thecreate_meta_clientfunction.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify correct usage of `meta_client` as `Arc<MetaClientRef>`. # Test: Search for `meta_client` usage. Expect: Correct handling of `Arc<MetaClientRef>`. rg --type rust $'meta_client'Length of output: 18626
Script:
#!/bin/bash # Extract the relevant code snippets from the files of interest # Extract code from src/meta-client/examples/lock.rs echo "Code from src/meta-client/examples/lock.rs:" rg --context 5 'meta_client' src/meta-client/examples/lock.rs # Extract code from src/meta-client/src/lib.rs echo -e "\nCode from src/meta-client/src/lib.rs:" rg --context 5 'meta_client' src/meta-client/src/lib.rsLength of output: 5009
src/meta-client/src/client.rs (2)
216-216: LGTM!The
MetaClientstruct has been updated to remove theClonetrait, which is consistent with the changes to useMetaClientRef.
216-216: Ensure correct handling ofMetaClientRefin all methods.Verify that all methods in the
MetaClientimplementations correctly handle theMetaClientReftype.
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?
As the title and @coderabbitai said.
Checklist
Summary by CodeRabbit
Refactor
new_metasrv_clienttonew_meta_clientacross several modules.MetaClientRefinstead ofMetaClient.MetaClientTypeenum andcreate_meta_clientfunction to support different meta client configurations.New Features
MetaClientIniterror variant for enhanced error handling.Bug Fixes
MetaClientIniterror variant.