Conversation
There was a problem hiding this comment.
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. |
We tried to use same func for backward and forward, but the code is too big and doesn't really simpler that two function
There was a problem hiding this comment.
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
25 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.