[ISSUE #1681]🚀Implement ChangeInvisibleTimeProcessor logic🍻

[mxsm]

Which Issue(s) This PR Fixes(Closes)

Fixes #1681

Brief Description

How Did You Test This Change?

Summary by CodeRabbit

• New Features

  • Introduced a new ChangeInvisibleTimeProcessor for enhanced message processing capabilities.
  • Added support for handling requests to change message visibility time.

• Bug Fixes

  • Improved error handling for request processing scenarios, ensuring more robust responses.

• Documentation

  • Updated method signatures and added new methods to improve clarity on functionality.

[coderabbitai]

Walkthrough

The changes introduced in this pull request involve the integration of the ChangeInvisibleTimeProcessor into the BrokerRequestProcessor within the RocketMQ broker. This includes modifying the initialization of the change_invisible_time_processor field to utilize a new instance of ChangeInvisibleTimeProcessor, which is now generic and capable of handling various message store types. Additionally, the request processing logic has been extended to accommodate a new request code related to changing message visibility, enhancing the broker's processing capabilities.

Changes

File Path	Change Summary
rocketmq-broker/src/broker_runtime.rs	Added import for ChangeInvisibleTimeProcessor. Updated BrokerRequestProcessor to initialize change_invisible_time_processor with a specific instance of ChangeInvisibleTimeProcessor.
rocketmq-broker/src/processor.rs	Changed type of change_invisible_time_processor to ArcMut<ChangeInvisibleTimeProcessor<MS>>. Added new match arm in process_request for handling RequestCode::ChangeMessageInvisibleTime.
rocketmq-broker/src/processor/change_invisible_time_processor.rs	Made ChangeInvisibleTimeProcessor struct generic. Added a constructor and updated process_request method. Introduced process_request_inner with detailed error handling and several new helper methods.

Assessment against linked issues

Objective	Addressed	Explanation
Implement ChangeInvisibleTimeProcessor logic (#1681)	✅

Possibly related PRs

• [ISSUE #1411]🔥BrokerRequestProcessor add QueryAssignmentProcessor logic handle🚨 #1412: Enhances BrokerRequestProcessor by adding logic to handle QueryAssignmentProcessor, related to the changes made in this PR.
• [ISSUE #1680]♻️Refactor ChangeInvisibleTimeProcessor handle logic🍻 #1683: Refactors ChangeInvisibleTimeProcessor, directly related to the functionality introduced in this PR.
• [ISSUE #1666]🚀Implement MQClientAPIImpl#changeInvisibleTimeAsync method logic🔥 #1669: Implements change_invisible_time_async in MQClientAPIImpl, relevant to handling invisible time changes.
• [ISSUE #1670]🚀Implement DefaultMQPushConsumerImpl#change_pop_invisible_time_async method logic🔥 #1672: Implements change_pop_invisible_time_async in DefaultMQPushConsumerImpl, related to message visibility handling.
• [ISSUE #1687]♻️Refactor PopMessageProcessor method handle error #1688: Refactors PopMessageProcessor to improve error handling, potentially interacting with the new processing logic.

Suggested labels

feature, enhancement, auto merge, ready to review, waiting-review, AI review first

Suggested reviewers

• SpaceXCN
• TeslaRustor

Poem

In the code where rabbits hop,
ChangeInvisibleTimeProcessor won’t stop.
With new logic and paths to explore,
Message visibility opens the door!
Hops of progress, leaps of cheer,
In the broker's heart, we hold dear! 🐇✨

---

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

• X
• Mastodon
• Reddit
• LinkedIn

🪧 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>, please review it.
  • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @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 using 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.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

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.

[rocketmq-rust-bot]

🔊@mxsm 🚀Thanks for your contribution 🎉. CodeRabbit(AI) will review your code first 🔥

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 177 lines in your changes missing coverage. Please review.

Project coverage is 27.91%. Comparing base (c8d1670) to head (67babab).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...r/src/processor/change_invisible_time_processor.rs	0.00%	164 Missing ⚠️
rocketmq-broker/src/broker_runtime.rs	0.00%	8 Missing ⚠️
rocketmq-broker/src/processor.rs	0.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1690      +/-   ##
==========================================
- Coverage   27.98%   27.91%   -0.08%     
==========================================
  Files         471      471              
  Lines       63311    63479     +168     
==========================================
  Hits        17718    17718              
- Misses      45593    45761     +168     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[coderabbitai]

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (6)

rocketmq-broker/src/processor/change_invisible_time_processor.rs (3)

44-51: Add documentation comments for struct fields

The ChangeInvisibleTimeProcessor<MS> struct now contains several fields crucial for broker operations. Adding documentation comments (///) for each field will enhance code readability and maintainability.

---

88-220: Improve error handling and logging in process_request_inner

In the process_request_inner method, consider the following improvements:

• Validate request_header fields: After decoding, ensure that all required fields in request_header are present and valid.
• Handle potential parsing errors: Methods like ExtraInfoUtil::split, ExtraInfoUtil::get_revive_qid, and ExtraInfoUtil::get_broker_name can fail. Ensure you handle Err cases appropriately, possibly returning meaningful error responses.
• Consistent logging levels: Use appropriate logging levels (info, warn, error) to reflect the severity of the events.
• Detailed error messages: Provide more context in error logs to aid in troubleshooting.

---

222-246: Implement or document unimplemented methods

The methods ack_origin, append_check_point, and process_change_invisible_time_for_order are currently unimplemented:

• If these methods are essential for the processor's functionality, consider implementing them to prevent runtime errors.
• If they are placeholders for future work, clearly document the expected behavior and consider adding TODO comments.

Would you like assistance in implementing these methods or creating GitHub issues to track their development?

rocketmq-broker/src/processor.rs (2)

69-69: Update documentation to reflect generic type change

The field change_invisible_time_processor now includes a generic type <MS>. Ensure that any documentation or comments associated with the BrokerRequestProcessor struct are updated to reflect this change.

---

171-177: Ensure robust handling for ChangeMessageInvisibleTime requests

The new match arm for RequestCode::ChangeMessageInvisibleTime delegates processing to change_invisible_time_processor. Verify that:

• All possible error cases within process_request are appropriately handled.
• There is adequate logging for request processing to facilitate debugging.
• The request_code parameter is correctly used within the processor.

rocketmq-broker/src/broker_runtime.rs (1)

69-69: Confirm the correctness of the new import

The ChangeInvisibleTimeProcessor import has been added. Ensure that:

• The import path is correct and follows project conventions.
• There are no conflicts with other imports or duplicate imports.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between c8d1670 and 67babab.

📒 Files selected for processing (3)

• rocketmq-broker/src/broker_runtime.rs (2 hunks)
• rocketmq-broker/src/processor.rs (2 hunks)
• rocketmq-broker/src/processor/change_invisible_time_processor.rs (1 hunks)

🔇 Additional comments (1)

rocketmq-broker/src/broker_runtime.rs (1)

536-543: Verify initialization of change_invisible_time_processor

The change_invisible_time_processor is initialized with several critical components. Confirm that:

• All dependencies (broker_config, topic_config_manager, message_store, etc.) are correctly passed and properly cloned or referenced.
• The initialization aligns with the expected lifecycle and ownership models in Rust, avoiding unnecessary cloning or potential race conditions.
• The ArcMut wrapper is appropriate for the intended usage of shared mutable state.