Skip to content
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: make flow distributed work&tests #4256

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

discord9
Copy link
Contributor

@discord9 discord9 commented Jul 3, 2024

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

preq: #4254

What's changed and what's your intention?

make flownode dist start work

  • add FrontendClient for flownode to connect to frontend for writing back results
  • add sqlness tests for dist flownode
  • modify sqlness runner to start flownode

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.

Summary by CodeRabbit

  • New Features

    • Added comprehensive configuration settings for Flownode, including modes, gRPC options, heartbeat intervals, logging, and tracing.
    • Introduced a frontend client for database operations in the flow module.
    • Implemented compression encoding for gRPC communication in FlownodeServer.
  • Enhancements

    • Updated error handling to support new TonicTransport variant.
    • Improved distributed mode configuration with new settings and defaults.
  • Documentation

    • Reorganized configuration documentation, adding a new section for Flownode settings.
  • Tests

    • Enhanced test environment to support Flownode server process.

Copy link
Contributor

coderabbitai bot commented Jul 3, 2024

Walkthrough

This update introduces significant enhancements and configurations for the flownode settings in both distributed and standalone modes. Key modifications include changes in the configuration files, updates to error handling, addition of gRPC server settings, frontend client integration, and refinements in metadata cache handling. These changes provide greater flexibility, improved error reporting, and enhanced performance in distributed environments.

Changes

File(s) Summary
config/flownode.example.toml, config/config...md Introduced and updated configurations for flownode, including mode selection and various settings
src/cmd/Cargo.toml, src/flow/Cargo.toml Added tonic.workspace = true and client.workspace = true
src/cmd/src/error.rs Added TonicTransport error handling to Error enum
src/cmd/src/flownode.rs Added gRPC server configuration fields to StartCommand struct
src/flow/src/adapter.rs Added frontend_addr to FlownodeOptions and modified set_frontend_invoker
src/flow/src/fe_client.rs Introduced FrontendClient struct and implemented methods for RPC
src/flow/src/lib.rs Added new module fe_client and exposed FrontendClient
src/flow/src/server.rs Added compression encoding support for gRPC flowserver
src/meta-srv/src/... Updated error messages to NoEnoughAvailableNode and included select_target parameter
tests/cases/.../cluster_info.result Added FLOWNODE entry in the CLUSTER_INFO query result set
tests/runner/src/env.rs Added flownode_process management to Env struct
config/config-docs-template.md, config/config.md Reorganized configurations and added documentation for Flownode

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Flownode
    participant gRPC Server
    participant FrontendClient
    
    User->>Flownode: Start Flownode
    Flownode->>gRPC Server: Setup connection
    gRPC Server->>FrontendClient: Initialize FrontendClient
    FrontendClient-->>gRPC Server: FrontendClient Ready
    gRPC Server-->>Flownode: gRPC Server Ready
    Flownode-->>User: Flownode Started Successfully
Loading

Poem

In the world of nodes so grand,
Flownodes rise with a gentle hand.
gRPC whispers, errors fall,
Distributed flows, we stand tall.
Log, trace, and cache we mend,
Here’s to seamless flows, my friend!
🌐🐰✨


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the docs-not-required This change does not impact docs. label Jul 3, 2024
@discord9 discord9 force-pushed the flow_fe_client branch 2 times, most recently from 05f5fa8 to 60f7b29 Compare July 4, 2024 11:08
@discord9 discord9 marked this pull request as ready for review July 4, 2024 12:33
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 4ec247f and 275863c.

Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
Files selected for processing (18)
  • config/flownode.example.toml (1 hunks)
  • src/cmd/Cargo.toml (1 hunks)
  • src/cmd/src/error.rs (2 hunks)
  • src/cmd/src/flownode.rs (5 hunks)
  • src/flow/Cargo.toml (1 hunks)
  • src/flow/src/adapter.rs (3 hunks)
  • src/flow/src/fe_client.rs (1 hunks)
  • src/flow/src/lib.rs (2 hunks)
  • src/flow/src/server.rs (2 hunks)
  • src/meta-srv/src/error.rs (3 hunks)
  • src/meta-srv/src/handler/collect_stats_handler.rs (1 hunks)
  • src/meta-srv/src/metasrv.rs (2 hunks)
  • src/meta-srv/src/selector/common.rs (3 hunks)
  • src/meta-srv/src/selector/round_robin.rs (2 hunks)
  • src/meta-srv/src/table_meta_alloc.rs (2 hunks)
  • src/operator/src/insert.rs (1 hunks)
  • tests/cases/distributed/information_schema/cluster_info.result (2 hunks)
  • tests/runner/src/env.rs (9 hunks)
Files skipped from review due to trivial changes (5)
  • src/cmd/Cargo.toml
  • src/flow/Cargo.toml
  • src/meta-srv/src/handler/collect_stats_handler.rs
  • src/meta-srv/src/selector/round_robin.rs
  • src/operator/src/insert.rs
Additional comments not posted (56)
src/flow/src/lib.rs (2)

30-30: Confirm module addition.

The fe_client module is correctly added.


40-40: Confirm re-export of FrontendClient.

The FrontendClient is correctly re-exported.

config/flownode.example.toml (12)

1-2: Confirm addition of mode setting.

The mode setting is correctly added.


4-6: Confirm addition of node identifier setting.

The node_id setting is correctly added.


8-9: Confirm addition of frontend address setting.

The frontend_addr setting is correctly added.


11-23: Confirm addition of gRPC server options.

The gRPC server options are correctly added.


26-41: Confirm addition of metasrv client options and timeout settings.

The metasrv client options and timeout settings are correctly added.


43-44: Confirm addition of TCP_NODELAY setting.

The tcp_nodelay setting is correctly added.


46-53: Confirm addition of metadata cache configuration.

The metadata cache configuration is correctly added.


55-61: Confirm addition of heartbeat options.

The heartbeat options are correctly added.


63-70: Confirm addition of logging options.

The logging options are correctly added.


72-77: Confirm addition of OTLP tracing options.

The OTLP tracing options are correctly added.


79-87: Confirm addition of logging tracing sample ratio.

The logging tracing sample ratio is correctly added.


88-92: Confirm addition of tracing options.

The tracing options are correctly added.

src/meta-srv/src/table_meta_alloc.rs (2)

25-25: Confirm addition of imports.

The imports for SelectTarget and SelectorRef are correctly added.


67-70: Confirm use of SelectTarget::Datanode in error handling.

The SelectTarget::Datanode is correctly used in error handling.

src/flow/src/fe_client.rs (9)

1-13: Confirm license header.

The license header is correctly added.


15-15: Confirm module documentation.

The module documentation is correctly added.


17-29: Confirm necessary imports.

The necessary imports are correctly added.


32-35: Confirm FrontendClient struct definition.

The FrontendClient struct is correctly defined.


37-43: Confirm FrontendClient constructor.

The FrontendClient constructor is correctly implemented.


45-60: Confirm to_rpc_request function.

The to_rpc_request function is correctly implemented.


62-65: Confirm from_rpc_error function.

The from_rpc_error function is correctly implemented.


67-76: Confirm resp_to_output function.

The resp_to_output function is correctly implemented.


78-108: Confirm implementation of FrontendInvoker for FrontendClient.

The implementation of FrontendInvoker for FrontendClient is correctly done.

tests/cases/distributed/information_schema/cluster_info.result (2)

28-28: Ensure Consistency of Output Format.

The added line includes multiple peer types with their respective addresses and details. Verify that the format matches the expected output and that all necessary fields are included.

Ensure that the output format is consistent with other similar outputs and that all necessary fields are correctly populated.


58-58: Ensure Consistency of Output Format.

The added line includes multiple peer types with their respective addresses and details. Verify that the format matches the expected output and that all necessary fields are included.

Ensure that the output format is consistent with other similar outputs and that all necessary fields are correctly populated.

src/meta-srv/src/selector/common.rs (3)

23-23: Import Statement Added.

The SelectTarget import is added. Ensure that this import is necessary and used correctly within the file.

The import statement is necessary for the modified error handling.


38-41: Updated Error Handling: NoEnoughAvailableNodeSnafu.

The error handling now uses NoEnoughAvailableNodeSnafu with SelectTarget::Datanode. Ensure that this change is consistent with the rest of the codebase.

The updated error handling is consistent with the new naming convention and usage of SelectTarget.


57-60: Updated Error Handling: NoEnoughAvailableNodeSnafu.

The error handling now uses NoEnoughAvailableNodeSnafu with SelectTarget::Datanode. Ensure that this change is consistent with the rest of the codebase.

The updated error handling is consistent with the new naming convention and usage of SelectTarget.

src/flow/src/server.rs (1)

124-127: Added Compression Handling.

The FlownodeServer now handles compressed data using gzip and zstd encoding. Ensure that the compression handling is correctly implemented and consistent with the rest of the code.

The compression handling is correctly implemented and consistent with the rest of the code.

src/cmd/src/flownode.rs (7)

34-34: Import Statement Added.

The Endpoint import is added. Ensure that this import is necessary and used correctly within the file.

The import statement is necessary for the new frontend connection setup.


39-39: Import Statement Added.

The TonicTransportSnafu import is added. Ensure that this import is necessary and used correctly within the file.

The import statement is necessary for the new error handling.


123-142: Added Fields to StartCommand Struct.

The StartCommand struct now includes additional fields for node_id, rpc_addr, rpc_hostname, metasrv_addrs, frontend_addr, config_file, and env_prefix. Ensure that these fields are necessary and used correctly within the struct.

The added fields are necessary for the new functionality and are used correctly within the struct.


189-191: Set Frontend Address in merge_with_cli_options.

The merge_with_cli_options method now sets the frontend_addr field. Ensure that this change is necessary and correctly implemented.

The change is necessary for setting the frontend address and is correctly implemented.


231-232: Ensure Frontend Address is Provided.

The build method now ensures that the frontend_addr is provided. Ensure that this change is necessary and correctly implemented.

The change is necessary for ensuring the frontend address is provided and is correctly implemented.


235-237: Optional Cluster ID Handling.

The build method now handles an optional cluster_id. Ensure that this change is correctly implemented.

The change is correctly implemented for handling an optional cluster_id.


319-334: Set Up Frontend Connection.

The build method now sets up a connection to the frontend server using Tonic transport. Ensure that this change is correctly implemented.

The change is correctly implemented for setting up the frontend connection.

src/cmd/src/error.rs (2)

350-357: Addition of a new error variant TonicTransport.

The new error variant TonicTransport is added to handle errors related to Tonic transport. This variant includes a location, the Tonic transport error, and an optional message. The implementation is correct and follows Rust's error handling best practices.


417-417: Handling of TonicTransport in ErrorExt implementation.

The new error variant TonicTransport is correctly mapped to the StatusCode::Internal status code. This ensures that Tonic transport errors are appropriately categorized as internal server errors.

src/meta-srv/src/metasrv.rs (1)

252-259: Implementation of the Display trait for SelectTarget.

The Display trait is correctly implemented for the SelectTarget enum. This allows for user-friendly string representations of the enum variants, which is useful for logging and debugging.

tests/runner/src/env.rs (9)

118-118: Addition of flownode_process field to GreptimeDB struct.

The flownode_process field is added to manage the flownode process. This change is consistent with the other process management fields and helps in managing the flownode process.


145-146: Starting the flownode process in distributed mode.

The flownode process is correctly started in the start_distributed method. This ensures that the flownode process is included in the distributed setup.


156-156: Addition of flownode_process field to GreptimeDB struct.

The flownode_process field is added to manage the flownode process in the start_distributed method. This change is consistent with the other process management fields and helps in managing the flownode process.


173-173: Addition of flownode_process field to GreptimeDB struct.

The flownode_process field is added to manage the flownode process in the connect_db method. This change is consistent with the other process management fields and helps in managing the flownode process.


200-201: Log file naming for flownode process.

The log file for the flownode process is correctly named and managed in the start_server method. This ensures that logs for the flownode process are properly handled.


221-221: Starting the flownode process.

The arguments for starting the flownode process are correctly set up in the start_server method. This ensures that the flownode process is started with the correct configuration.


318-332: Addition of flownode_start_args method.

The flownode_start_args method is correctly implemented to generate the arguments needed to start the flownode process. This method ensures that the flownode process is started with the correct configuration.


448-448: Addition of flownode_process field to GreptimeDB struct.

The flownode_process field is added to manage the flownode process in the GreptimeDB struct. This change is consistent with the other process management fields and helps in managing the flownode process.


544-547: Stopping the flownode process.

The flownode process is correctly stopped in the stop method. This ensures that the flownode process is properly managed and terminated.

src/flow/src/adapter.rs (3)

87-87: Addition of frontend_addr field to FlownodeOptions struct.

The frontend_addr field is added to the FlownodeOptions struct to specify the address of the frontend. This change is consistent with the other configuration options and helps in setting up the flownode.


101-101: Initialization of frontend_addr field in FlownodeOptions struct.

The frontend_addr field is correctly initialized to None in the Default implementation of the FlownodeOptions struct. This ensures that the field is properly handled when the struct is created with default values.


138-138: Modification of set_frontend_invoker method signature.

The set_frontend_invoker method signature is modified to accept a boxed FrontendInvoker. This change is consistent with the usage of the FrontendInvoker in the FlowWorkerManager struct and ensures that the invoker is properly handled.

src/meta-srv/src/error.rs (3)

29-29: Import Approved: SelectTarget

The import of SelectTarget is necessary for the new error variant NoEnoughAvailableNode.


179-189: Addition Approved: NoEnoughAvailableNode Error Variant

The NoEnoughAvailableNode error variant is correctly defined with relevant fields.


901-901: Integration Approved: NoEnoughAvailableNode in status_code Method

The NoEnoughAvailableNode error variant is appropriately mapped to StatusCode::Internal.

Copy link

codecov bot commented Jul 4, 2024

Codecov Report

Attention: Patch coverage is 4.30108% with 89 lines in your changes missing coverage. Please review.

Project coverage is 84.94%. Comparing base (b1219fa) to head (8b3134d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4256      +/-   ##
==========================================
- Coverage   85.25%   84.94%   -0.31%     
==========================================
  Files        1058     1059       +1     
  Lines      186971   187054      +83     
==========================================
- Hits       159404   158898     -506     
- Misses      27567    28156     +589     

src/cmd/src/flownode.rs Show resolved Hide resolved
src/flow/src/fe_client.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 and nitpick comments (4)
config/config-docs-template.md (1)

29-31: Fix trailing newline issue.

Ensure the file ends with a single newline character to adhere to Markdownlint guidelines.

- {{ toml2docs "./flownode.example.toml"}}
+ {{ toml2docs "./flownode.example.toml"}}
+ 
Tools
Markdownlint

31-31: null
Files should end with a single newline character

(MD047, single-trailing-newline)

config/config.md (3)

441-478: Fix grammatical issues and trailing blank lines.

  1. Grammatical Fix: The word ‘frontend’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
  2. Complete Sentence: Ensure the log level description forms a complete sentence.
  3. Trailing Blank Lines: Remove multiple consecutive blank lines to adhere to Markdownlint guidelines.
- | `frontend_addr` | String | `http://127.0.0.1:4001` | Frontend grpc address. Used by flownode to write result back to frontend. |
+ | `frontend_addr` | String | `http://127.0.0.1:4001` | Frontend gRPC address. Used by flownode to write results back to the frontend. |

- | `logging.level` | String | `None` | The log level. Can be `info`/`debug`/`warn`/`error`. |
+ | `logging.level` | String | `None` | The log level, which can be `info`, `debug`, `warn`, or `error`. |

- 
- 
Tools
LanguageTool

[grammar] ~449-~449: The word ‘frontend’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: .... Used by flownode to write result back to frontend. | | grpc | -- | -- | The gRPC server...

(VB_TO_NN_DT)


[style] ~471-~471: To form a complete sentence, be sure to include a subject.
Context: ...vel| String |None| The log level. Can beinfo/debug/warn/error. | | ...

(MISSING_IT_THERE)

Markdownlint

442-442: Expected: 1; Actual: 2
Multiple consecutive blank lines

(MD012, no-multiple-blanks)


[!TIP]
Codebase Verification

Update default HTTP address for distributed mode.

The default HTTP address for the Frontend in distributed mode has not been consistently updated across the documentation. Please update the following occurrences of the old address 127.0.0.1:4000 to the new address:

  • config/config.md:
    • Line containing | http.addr | String | 127.0.0.1:4000 | The address to bind the HTTP server. |
    • Lines containing | export_metrics.remote_write.url | String | "" | The url the metrics send to. The url example can be: http://127.0.0.1:4000/v1/prometheus/write?db=information_schema.
Analysis chain

Line range hint 163-173:
Update default HTTP address for distributed mode.

The default HTTP address for the Frontend in distributed mode has been updated. Ensure that this change is consistent with other parts of the documentation.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the updated HTTP address for the Frontend is consistent across the documentation.

# Test: Search for the old default HTTP address. Expect: No occurrences of the old address.
rg --type markdown $'127.0.0.1:4000'

Length of output: 907


[!TIP]
Codebase Verification

Inconsistent default HTTP address for Datanode in distributed mode

The documentation contains multiple different default HTTP addresses for the Datanode (127.0.0.1:4000, 127.0.0.1:6000, and 127.0.0.1:5000). Please review and ensure that the default HTTP address for the Datanode in distributed mode is consistent across all occurrences.

  • config/config.md:
    • http.addr | String | 127.0.0.1:4000
    • http.addr | String | 127.0.0.1:6000
    • http.addr | String | 127.0.0.1:5000
Analysis chain

Line range hint 310-339:
Update default HTTP address for Datanode in distributed mode.

The default HTTP address for the Datanode in distributed mode has been updated. Ensure that this change is consistent with other parts of the documentation.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the updated HTTP address for the Datanode is consistent across the documentation.

# Test: Search for the old default HTTP address. Expect: No occurrences of the old address.
rg --type markdown $'127.0.0.1:4000'

Length of output: 907


Script:

#!/bin/bash
# Description: Verify if the new default HTTP address for the Datanode is mentioned in the documentation.

# Test: Search for the new default HTTP address pattern. Expect: Occurrences of the new address.
rg --type markdown '127.0.0.1:'

Length of output: 2399


Script:

#!/bin/bash
# Description: Verify if the default HTTP address for the Datanode in distributed mode is mentioned correctly.

# Test: Search for occurrences of `http.addr` in the context of Datanode in distributed mode.
rg --type markdown 'http.addr'

Length of output: 296

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 275863c and 27da89b.

Files selected for processing (4)
  • config/config-docs-template.md (2 hunks)
  • config/config.md (6 hunks)
  • config/flownode.example.toml (1 hunks)
  • src/flow/src/fe_client.rs (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • config/flownode.example.toml
  • src/flow/src/fe_client.rs
Additional context used
Markdownlint
config/config-docs-template.md

31-31: null
Files should end with a single newline character

(MD047, single-trailing-newline)

config/config.md

442-442: Expected: 1; Actual: 2
Multiple consecutive blank lines

(MD012, no-multiple-blanks)

LanguageTool
config/config.md

[grammar] ~449-~449: The word ‘frontend’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: .... Used by flownode to write result back to frontend. | | grpc | -- | -- | The gRPC server...

(VB_TO_NN_DT)


[style] ~471-~471: To form a complete sentence, be sure to include a subject.
Context: ...vel| String |None| The log level. Can beinfo/debug/warn/error. | | ...

(MISSING_IT_THERE)

Additional comments not posted (2)
config/config-docs-template.md (1)

3-9: Ensure correct logical placement of the new section.

The new Flownode section is logically placed under the Distributed Mode configurations. The structure looks good.

config/config.md (1)

3-9: Ensure correct logical placement of the new section.

The new Flownode section is logically placed under the Distributed Mode configurations. The structure looks good.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Outside diff range and nitpick comments (1)
config/config.md (1)

439-440: Remove extra blank line.

There are multiple consecutive blank lines. Remove the extra blank line for cleaner formatting.

- 
Tools
Markdownlint

440-440: Expected: 1; Actual: 2
Multiple consecutive blank lines

(MD012, no-multiple-blanks)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 27da89b and 2c91c80.

Files selected for processing (1)
  • config/config.md (2 hunks)
Additional context used
LanguageTool
config/config.md

[grammar] ~447-~447: The word ‘frontend’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: .... Used by flownode to write result back to frontend. | | grpc | -- | -- | The gRPC server...

(VB_TO_NN_DT)


[style] ~469-~469: To form a complete sentence, be sure to include a subject.
Context: ...vel| String |None| The log level. Can beinfo/debug/warn/error. | | ...

(MISSING_IT_THERE)

Markdownlint
config/config.md

440-440: Expected: 1; Actual: 2
Multiple consecutive blank lines

(MD012, no-multiple-blanks)

Additional comments not posted (2)
config/config.md (2)

441-476: New section Flownode configuration looks good!

The new configuration options for Flownode are well-documented and comprehensive.

Tools
LanguageTool

[grammar] ~447-~447: The word ‘frontend’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: .... Used by flownode to write result back to frontend. | | grpc | -- | -- | The gRPC server...

(VB_TO_NN_DT)


[style] ~469-~469: To form a complete sentence, be sure to include a subject.
Context: ...vel| String |None| The log level. Can beinfo/debug/warn/error. | | ...

(MISSING_IT_THERE)


3-9: Table of contents update looks good!

The table of contents has been correctly updated to include the new Flownode section.

config/config.md Show resolved Hide resolved
config/config.md Show resolved Hide resolved
feat: Frontend Client

feat: set frontend invoker for flownode

feat: set frontend invoker for flownode

chore: test script

WIP: test flow distributed

feat: hard coded demo

docs: flownode example toml

feat: add flownode support in runner

docs: comments for node

chore: after rebase

docs: add a todo

tests: move flow tests to common

fix: flownode sqlness dist test

chore: per review

docs: make

fix: make doc
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Outside diff range and nitpick comments (1)
config/config-docs-template.md (1)

3-9: Add an entry for Flownode in the table of contents.

The addition of the Flownode section in the distributed mode should be reflected in the table of contents for completeness.

-  - [Distributed Mode](#distributed-mode)
-    - [Frontend](#frontend)
-    - [Metasrv](#metasrv)
-    - [Datanode](#datanode)
+  - [Distributed Mode](#distributed-mode)
+    - [Frontend](#frontend)
+    - [Metasrv](#metasrv)
+    - [Datanode](#datanode)
+    - [Flownode](#flownode)
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2c91c80 and 8b3134d.

Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
Files selected for processing (20)
  • config/config-docs-template.md (2 hunks)
  • config/config.md (2 hunks)
  • config/flownode.example.toml (1 hunks)
  • src/cmd/Cargo.toml (1 hunks)
  • src/cmd/src/error.rs (2 hunks)
  • src/cmd/src/flownode.rs (5 hunks)
  • src/flow/Cargo.toml (1 hunks)
  • src/flow/src/adapter.rs (3 hunks)
  • src/flow/src/fe_client.rs (1 hunks)
  • src/flow/src/lib.rs (2 hunks)
  • src/flow/src/server.rs (2 hunks)
  • src/meta-srv/src/error.rs (3 hunks)
  • src/meta-srv/src/handler/collect_stats_handler.rs (1 hunks)
  • src/meta-srv/src/metasrv.rs (2 hunks)
  • src/meta-srv/src/selector/common.rs (3 hunks)
  • src/meta-srv/src/selector/round_robin.rs (2 hunks)
  • src/meta-srv/src/table_meta_alloc.rs (2 hunks)
  • src/operator/src/insert.rs (1 hunks)
  • tests/cases/distributed/information_schema/cluster_info.result (2 hunks)
  • tests/runner/src/env.rs (9 hunks)
Files skipped from review as they are similar to previous changes (17)
  • config/flownode.example.toml
  • src/cmd/Cargo.toml
  • src/cmd/src/error.rs
  • src/cmd/src/flownode.rs
  • src/flow/Cargo.toml
  • src/flow/src/adapter.rs
  • src/flow/src/fe_client.rs
  • src/flow/src/server.rs
  • src/meta-srv/src/error.rs
  • src/meta-srv/src/handler/collect_stats_handler.rs
  • src/meta-srv/src/metasrv.rs
  • src/meta-srv/src/selector/common.rs
  • src/meta-srv/src/selector/round_robin.rs
  • src/meta-srv/src/table_meta_alloc.rs
  • src/operator/src/insert.rs
  • tests/cases/distributed/information_schema/cluster_info.result
  • tests/runner/src/env.rs
Additional context used
Markdownlint
config/config-docs-template.md

31-31: null
Files should end with a single newline character

(MD047, single-trailing-newline)

config/config.md

440-440: Expected: 1; Actual: 2
Multiple consecutive blank lines

(MD012, no-multiple-blanks)

LanguageTool
config/config.md

[grammar] ~447-~447: The word ‘frontend’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: .... Used by flownode to write result back to frontend. | | grpc | -- | -- | The gRPC server...

(VB_TO_NN_DT)


[style] ~469-~469: To form a complete sentence, be sure to include a subject.
Context: ...vel| String |None| The log level. Can beinfo/debug/warn/error. | | ...

(MISSING_IT_THERE)

Additional comments not posted (4)
src/flow/src/lib.rs (2)

30-30: LGTM!

The addition of the fe_client module is consistent with the rest of the file.


40-40: LGTM!

The export of FrontendClient is consistent with the rest of the file.

config/config.md (2)

441-447: Fix grammatical issue.

The word 'frontend' is not correct in this context. Use 'front end' instead.

- Used by flownode to write result back to frontend.
+ Used by flownode to write result back to the front end.
Tools
LanguageTool

[grammar] ~447-~447: The word ‘frontend’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: .... Used by flownode to write result back to frontend. | | grpc | -- | -- | The gRPC server...

(VB_TO_NN_DT)


469-469: Incomplete sentence.

To form a complete sentence, be sure to include a subject.

- The log level. Can be `info`/`debug`/`warn`/`error`.
+ The log level. It can be `info`/`debug`/`warn`/`error`.
Tools
LanguageTool

[style] ~469-~469: To form a complete sentence, be sure to include a subject.
Context: ...vel| String |None| The log level. Can beinfo/debug/warn/error. | | ...

(MISSING_IT_THERE)

config/config-docs-template.md Show resolved Hide resolved
config/config.md Show resolved Hide resolved
@zyy17 zyy17 added this pull request to the merge queue Jul 5, 2024
Merged via the queue into GreptimeTeam:main with commit 09fff24 Jul 5, 2024
62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-not-required This change does not impact docs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants