fix: key value service resub before subscribe slot removed was failed

[giangndm]

This PR fixes the key-value service in the following cases:

• SUBscribing to a key
• UNSUBscribing from the key
• re-SUBscribing to the key before the subscribe slot is removed (after UNSUB_ACK after a few seconds)

The result is that the SUB message will not be sent. This bug occurs with both single key values and hash-map key-values.

Summary by CodeRabbit

• Refactor
  • Optimized internal operations to ensure efficiency and reduced unnecessary processing.
• New Features
  • Enhanced logging for key operations, improving visibility for system monitoring.
  • Subscription functionality now indicates success more clearly, aiding in better management of subscriptions.
• Tests
  • Added comprehensive tests for subscription and unsubscription functionalities to ensure reliability.

[coderabbitai]

Walkthrough

The recent updates focus on optimizing the key-value service's efficiency and logging practices. Notably, request ID generation and logging levels have been refined across various operations to ensure better resource management and more informative logs. Subscription handling has been enhanced with clearer logging and validation of existing subscriptions. Additionally, the introduction of new test cases aims to solidify the reliability of subscription features.

Changes

Files	Change Summary
.../key_value/src/behavior/hashmap_local.rs, .../key_value/src/behavior/simple_local.rs	- Optimized req_id generation - Elevated log level to info for "set" - Improved del method efficiency - Enhanced subscription handling with logs - Refined unsubscribe method - Added tests for subscription features
.../key_value/src/behavior/hashmap_remote.rs, .../key_value/src/behavior/simple_remote.rs	- Adjusted logging to info level for set, del, sub, and unsub events
.../key_value/src/storage/hashmap.rs, .../key_value/src/storage/simple.rs	- Modified subscribe method to indicate success or pre-existing subscription
.../pub_sub/src/relay.rs	- Updated logging to info level for source addition and removal events

Poem

🐇✨
In the realm of keys and values, changes abound,
Where logs now sing a clearer sound.
Subscriptions checked with care,
A rabbit's touch, light as air. 🌟

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>.
  • Generate unit-tests for this file.
• 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 tests 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 from git and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @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.
• The JSON schema for the configuration file is available here.
• 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/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between ba15687 and d130996.

Files selected for processing (7)

• packages/services/key_value/src/behavior/hashmap_local.rs (9 hunks)
• packages/services/key_value/src/behavior/hashmap_remote.rs (2 hunks)
• packages/services/key_value/src/behavior/simple_local.rs (6 hunks)
• packages/services/key_value/src/behavior/simple_remote.rs (2 hunks)
• packages/services/key_value/src/storage/hashmap.rs (3 hunks)
• packages/services/key_value/src/storage/simple.rs (3 hunks)
• packages/services/pub_sub/src/relay.rs (2 hunks)

Files skipped from review due to trivial changes (1)

• packages/services/key_value/src/behavior/simple_remote.rs

Additional comments: 21

packages/services/pub_sub/src/relay.rs (2)

• 106-106: The logging level change from debug to info in the on_source_added method enhances observability, which aligns with the PR objectives to improve logging for better maintainability and observability.
• 115-115: The logging level change from debug to info in the on_source_removed method is consistent with the changes made in on_source_added and supports the PR's goal of enhancing logging for improved service monitoring.

packages/services/key_value/src/behavior/hashmap_remote.rs (4)

• 48-58: Changing the logging level to info for successful set operations improves the observability of key events, aligning with the PR's objectives to enhance logging across the service.
• 78-87: Logging successful del operations at the info level is a positive change for monitoring key deletions, supporting the PR's goal of improved logging for better service observability.
• 92-94: Elevating the logging level for successful sub operations to info enhances the visibility of subscription events, which is in line with the PR's objectives to improve logging.
• 99-101: Adjusting the logging level to info for successful unsub operations aids in better monitoring of unsubscription events, aligning with the PR's aim to enhance logging for improved observability.

packages/services/key_value/src/storage/simple.rs (1)

• 156-168: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [159-193]

The modification of the subscribe method to return a boolean indicating the success or failure of subscription attempts is a significant improvement. It directly addresses the bug related to handling rapid subscribe-unsubscribe-re-subscribe sequences by allowing the caller to understand whether a subscription was successfully added or if it was already present. This change, along with the addition of the now_ms parameter for handling expiration, enhances the method's functionality and reliability, aligning with the PR's objectives to correct subscription handling issues.

packages/services/key_value/src/storage/hashmap.rs (1)

• 183-195: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [186-222]

The implementation of the subscribe method has been updated to return a boolean indicating the success of the subscription operation. This change aligns with the PR objectives to handle rapid subscribe-unsubscribe-re-subscribe sequences more effectively. However, it's crucial to ensure that all callers of this method properly handle the boolean response to avoid logical errors in subscription management.

packages/services/key_value/src/behavior/simple_local.rs (4)

• 301-301: The log level for the "set" operation has been elevated from debug to info. This change enhances the observability of key actions, aligning with the PR objectives to improve logging across the system. Ensure that the increased verbosity does not overwhelm the log storage or make it difficult to filter important information.
• 334-337: The del method now checks for the existence of the key before generating a req_id and logging the deletion attempt. This optimization prevents unnecessary operations and aligns with the PR's goal to enhance efficiency. Ensure that all deletion scenarios are adequately tested to confirm that no edge cases have been introduced.
• 348-385: The subscribe method has been refactored to handle existing subscriptions more accurately, including logging for subscription events. This change addresses the bug related to rapid subscribe-unsubscribe-re-subscribe sequences by ensuring that re-subscriptions are correctly managed. It's important to verify that the new logging does not introduce any performance issues or excessive verbosity in the logs.
• 396-397: The unsubscribe method now logs the unsubscription event and generates req_id only when necessary. This adjustment improves the method's efficiency and logging detail, supporting the PR's objectives. Confirm that unsubscription logic works as expected in all scenarios, especially in edge cases involving rapid subscription changes.

packages/services/key_value/src/behavior/hashmap_local.rs (9)

• 139-139: Logging added for syncing set keys is a good practice for observability.
• 165-165: Logging added for syncing subscription keys enhances observability and debugging capabilities.
• 269-269: Elevating the logging level to info for on_key_set events improves the visibility of key set operations, aligning with the PR objectives to enhance logging.
• 283-283: Elevating the logging level to info for on_key_del events is consistent with the PR's goal to improve logging and observability.
• 306-306: Logging the set operation at the info level is a positive change for better observability of key-value operations.
• 326-326: Logging the get operation at the info level aligns with the PR's objectives to enhance logging and observability.
• 344-344: Logging the del operation at the info level is consistent with the PR's goal to improve logging and observability.
• 388-388: Logging the subscribe operation at the info level enhances the visibility of subscription actions, which is beneficial for monitoring and debugging.
• 404-404: Logging the unsubscribe operation at the info level is a good practice for better observability of subscription management.