Skip to content

RSCAN command implementation - #35

Merged
iwanbk merged 5 commits into
mainfrom
rscan-cmd
May 6, 2025
Merged

RSCAN command implementation#35
iwanbk merged 5 commits into
mainfrom
rscan-cmd

Conversation

@iwanbk

@iwanbk iwanbk commented Apr 30, 2025

Copy link
Copy Markdown
Contributor

No description provided.

@iwanbk
iwanbk requested a review from Copilot April 30, 2025 14:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request implements the new RSCAN command by updating the metadata tree scanning API to support bidirectional scanning. Key changes include:

  • Updating the iter_kv API to include an optional ScanDirection parameter.
  • Renaming the scanning parameter in Namespace from start_after to start_key.
  • Adjusting test cases and multiple store implementations to align with the new API signature.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
s3cas/src/cas/fs.rs Updated iter_kv call to include an extra None parameter.
respd/src/storage.rs Updated iter_kv invocation with the new parameter.
respd/src/namespace.rs Renamed parameter and added a ScanDirection option in scan.
respd/src/cmd.rs Updated scan method invocation to pass the new parameter.
metastore/src/traits.rs Modified iter_kv signature and added ScanDirection documentation.
metastore/src/stores/test_utils.rs Adjusted iter_kv calls in test utilities to the new API signature.
metastore/src/stores/fjall_notx.rs Updated iter_kv implementation to support directional scanning.
metastore/src/stores/fjall.rs Updated iter_kv implementation with new parameter handling.
metastore/src/meta_store.rs Updated iter_kv call in bucket listing to include the new parameter.

iwanbk added 4 commits May 6, 2025 11:47
We tried to use same func for backward and forward,
but the code is too big and doesn't really simpler that two function
@iwanbk
iwanbk requested a review from Copilot May 6, 2025 07:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements the RSCAN command to scan keys in backward order. Key changes include adding integration tests for RSCAN, extending the namespace API with a scan_backward method, updating command parsing and handling to support RSCAN, and documenting the new command.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
respd/tests/integration_test.rs Added integration tests covering various RSCAN scenarios
respd/src/namespace.rs Updated scan API; added scan_backward for reverse scanning
respd/src/cmd.rs Introduced RSCAN command and its handler
respd/README.md Documented the new RSCAN command
metastore/src/traits.rs Added iter_kv_backward to trait definition
metastore/src/stores/fjall_notx.rs Implemented iter_kv_backward for backward scanning in FjallTreeNotx
metastore/src/stores/fjall.rs Implemented iter_kv_backward for backward scanning in FjallTree
Comments suppressed due to low confidence (2)

respd/src/cmd.rs:938

  • [nitpick] For consistency with the scan_backward API, consider renaming the 'start_after' variable to 'start_key'.
let start_after = cursor.map(|c| c.into_bytes());

metastore/src/stores/fjall.rs:288

  • [nitpick] Consider using a more explicit unbounded range construct (if supported) or adding a clarifying comment to explain the use of '..Vec::new()' for backward scanning.
None => ..Vec::new(), // Start from the end

@iwanbk
iwanbk merged commit fd03601 into main May 6, 2025
@iwanbk
iwanbk deleted the rscan-cmd branch May 6, 2025 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants