fix: stream status incorrect #4547
Conversation
WalkthroughThe changes in this pull request involve renaming and modifying functions related to loading local node configurations and stream statistics management across various files. The Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Tip OpenAI O1 model for chat
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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/config/src/cluster.rs (1)
30-43: Consider using a unique identifier for theidfield and handling different node states.
- Making the
load_local_nodefunction public is appropriate if it needs to be accessed from other modules.- Initializing the
Nodestruct with explicit fields based on the configuration provides a more detailed and specific initialization, reflecting the current configuration settings.However, consider the following suggestions:
- The
idfield is hardcoded to1. If multiple nodes are expected, consider using a unique identifier for each node.- The
statusfield is set toNodeStatus::Online, which assumes the node is always online when loaded. Consider handling cases where the node might be offline or in a different state.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/common/infra/cluster/mod.rs (2 hunks)
- src/config/src/cluster.rs (1 hunks)
- src/infra/src/file_list/mysql.rs (3 hunks)
- src/infra/src/file_list/postgres.rs (3 hunks)
- src/infra/src/file_list/sqlite.rs (3 hunks)
Files skipped from review due to trivial changes (1)
- src/common/infra/cluster/mod.rs
Additional context used
Path-based instructions (4)
src/config/src/cluster.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/infra/src/file_list/postgres.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/infra/src/file_list/sqlite.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/infra/src/file_list/mysql.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 (10)
src/infra/src/file_list/postgres.rs (4)
485-487: LGTM!The code correctly identifies new stream keys that are not present in
old_statsand adds them tonew_streamsfor further processing.
488-488: LGTM!The code correctly adds the current stream key and item to
update_streamsfor updating the stream statistics later.
520-522: LGTM!The changes to the SQL update statement are correct. Incrementally updating the statistics by adding the new values to the existing ones allows for cumulative statistics tracking.
569-574: LGTM!The new SQL query is correct and enhances data integrity. It ensures that the
max_tsfield is always greater than or equal tomin_ts, preventing inconsistencies in the stream statistics.src/infra/src/file_list/sqlite.rs (3)
490-493: LGTM!The changes simplify the process of updating stream statistics by directly checking the
old_statsmap and adding new streams to a separate vector. This approach improves efficiency and aligns with the AI-generated summary.
528-529: Excellent improvement!The modifications to the SQL update statement ensure that the statistics for existing streams are incrementally updated by adding the new values to the existing ones. This approach maintains the cumulative nature of the statistics and prevents data loss, enhancing the accuracy of the stream statistics. Well done!
570-575: Good catch!The introduction of the new SQL query to set
max_tstomin_tswhenmax_tsis less thanmin_tsis a valuable addition. This ensures data integrity and maintains the consistency and accuracy of the stream statistics. It's a thoughtful improvement that aligns with the AI-generated summary.src/infra/src/file_list/mysql.rs (3)
480-483: LGTM!The simplified logic for identifying new streams looks good. It directly checks if a stream exists in
old_statsand adds it tonew_streamsif it doesn't, which is more efficient than the previous implementation.
Line range hint
517-524: Incremental updates andmax_tsconsistency check are good optimizations.The modifications to the SQL update statement for
stream_statslook good:
- Incrementally updating the
file_num,records,original_size, andcompressed_sizefields enhances efficiency by avoiding redundant data retrieval and manipulation.- Updating the
max_tsfield to reflect the minimum timestamp when it is less than the current minimum ensures data consistency.
565-570: Consistency check formax_tsandmin_tsis a good addition.The new SQL query to update the
max_tsfield to the value ofmin_tswhenmax_tsis less thanmin_tsis a good addition. It ensures data consistency by maintaining the invariant thatmax_tsshould always be greater than or equal tomin_tsin thestream_statstable.
fixed #3935
If your steam stats is incorrect, please upgrade the this version and reset the stream stats once.
Summary by CodeRabbit
New Features
load_local_nodefunction to provide detailed initialization of Node instances based on configuration settings.Bug Fixes
Refactor