Implement Cluster Scan #113

currantw · 2025-10-22T00:48:57Z

Implement Cluster Scan

Overview

Adds ScanAsync methods to the standalone and cluster clients.

Key Changes

Added ScanAsync to IGenericClusterCommands and implemented it in GlideClusterClient, which calls new BaseClient.ClusterScanCommand method.
Added ScanAsync to IGenericCommands (standalone client) and implemented it in GlideClusterClient, which calls the existing Glide SCAN command. Provides consistent scan interface for standalone and cluster clients.
Cluster scan is implemented with ClusterScanCursor, while both standalone and cluster scanning use ScanOptions, which supports match pattern, count, and type arguments. Refactor existing Request.GenericCommands.ScanAsync and KeysAsync method to use ScanOptions.
Added RequestClusterScanFfi and RemoveClusterScanCursorFfi methods to "FFI.methods.rs", as well as Add request_cluster_scan and remove_cluster_scan_cursor to "rust/src/lib.rs" that call into the existing core methods (implementation based on equivalent methods here).

Testing Coverage

Added new ScanTests for both standalone and cluster ScanAsync methods.
Updated CommandTests for change to internal Request.ScanAsync interface.

Breaking Changes

None.

Signed-off-by: currantw <[email protected]>

…n `Request.GenericCommands`. Signed-off-by: currantw <[email protected]>

Signed-off-by: currantw <[email protected]>

…y bank") Signed-off-by: currantw <[email protected]>

Signed-off-by: currantw <[email protected]>

… methods Signed-off-by: currantw <[email protected]>

Signed-off-by: currantw <[email protected]>

…r_scan_cursor` in `lib.rs`. Signed-off-by: currantw <[email protected]>

…scan_cursor`. Signed-off-by: currantw <[email protected]>

…tion Signed-off-by: currantw <[email protected]>

Signed-off-by: currantw <[email protected]>

currantw · 2025-10-22T23:18:05Z

.gitignore

 .vs
 .vscode
 .kiro/
+.amazonq/


Unrelated addition. Supports Amazon Q memory bank.

currantw · 2025-10-22T23:21:42Z

rust/src/lib.rs

+/// * `args` and `arg_lengths` must be valid arrays of length `arg_count`
+/// * `args` format: [b"MATCH", pattern, b"COUNT", count, b"TYPE", type] (all optional)
+#[unsafe(no_mangle)]
+pub unsafe extern "C-unwind" fn request_cluster_scan(


Don't have any experience with Rust. Implementation modelled on existing cluster scan methods here. Any feedback appreciated!

currantw · 2025-10-22T23:23:22Z

rust/src/lib.rs

+    let scan_state_cursor = if cursor_id == "0" {
+        redis::ScanStateRC::new()
+    } else {
+        match glide_core::cluster_scan_container::get_cluster_scan_cursor(cursor_id.clone()) {
+            Ok(existing_cursor) => existing_cursor,
+            Err(_error) => {
+                unsafe {
+                    (core.failure_callback)(
+                        callback_index,
+                        format!("Invalid cursor ID: {}", cursor_id).as_ptr() as *const c_char,
+                        RequestErrorType::Unspecified,
+                    );
+                }
+                return;
+            }
+        }
+    };


Updated to handle the case (raise an error) where an invalid cursor is provided, as described in this pull request.

currantw · 2025-10-22T23:25:15Z

sources/Valkey.Glide/BaseClient.cs

+    #region protected fields
+    protected Version? _serverVersion; // cached server version
+    protected static readonly Version DefaultServerVersion = new(8, 0, 0);
+    #endregion protected fields


Not new. Just moved from "private fields" region below (since these obviously are not private...)

currantw · 2025-10-22T23:53:03Z

sources/Valkey.Glide/Internals/Request.GenericCommands.cs

        => Simple<bool>(RequestType.Move, [key.ToGlideString(), database.ToGlideString()]);

-    public static Cmd<object[], (long, ValkeyKey[])> ScanAsync(long cursor, ValkeyValue pattern = default, long pageSize = 0)
+    public static Cmd<object[], (string, ValkeyKey[])> ScanAsync(string cursor, ScanOptions? options = null)


Updated to use ScanOptions and for consistency with the cluster interface.

We can only change this API because the earlier C# release was a public preview right?

currantw · 2025-10-23T04:55:08Z

sources/Valkey.Glide/BaseClient.cs


    protected abstract Task<Version> GetServerVersionAsync();

+    protected async Task<(string cursor, ValkeyKey[] keys)> ClusterScanCommand(string cursor, string[] args)


I would have liked to put this in GlideClusterClient, since it is actually only applicable to the cluster client (obviously). But that would require changing the access modifier for _messageContainer, which I'm not sure is advisable? @jbrinkman / @alexr-bq any thoughts?

Would it be possible to give _messageContainer an internal access level? That way arbitrary user code that extends BaseClient doesn't have access to this method.

Signed-off-by: currantw <[email protected]>

currantw · 2025-10-23T05:08:19Z

tests/Valkey.Glide.UnitTests/CommandTests.cs

+            () => Assert.Equal(["SCAN", "0", "TYPE", "string"], Request.ScanAsync("0", new ScanOptions { Type = ValkeyType.String }).GetArgs()),
+            () => Assert.Equal(["SCAN", "5", "TYPE", "list"], Request.ScanAsync("5", new ScanOptions { Type = ValkeyType.List }).GetArgs()),
+            () => Assert.Equal(["SCAN", "0", "TYPE", "set"], Request.ScanAsync("0", new ScanOptions { Type = ValkeyType.Set }).GetArgs()),
+            () => Assert.Equal(["SCAN", "0", "TYPE", "zset"], Request.ScanAsync("0", new ScanOptions { Type = ValkeyType.SortedSet }).GetArgs()),
+            () => Assert.Equal(["SCAN", "0", "TYPE", "hash"], Request.ScanAsync("0", new ScanOptions { Type = ValkeyType.Hash }).GetArgs()),
+            () => Assert.Equal(["SCAN", "0", "TYPE", "stream"], Request.ScanAsync("0", new ScanOptions { Type = ValkeyType.Stream }).GetArgs()),


Added additional tests here for TYPE, which wasn't supported before.

currantw · 2025-10-23T05:11:17Z

sources/Valkey.Glide/Commands/IGenericCommands.cs

+    /// </code>
+    /// </example>
+    /// <seealso href="https://valkey.io/commands/scan/">SCAN command</seealso>
+    Task<(string cursor, ValkeyKey[] keys)> ScanAsync(string cursor, ScanOptions? options = null);


Standalone ScanAsync added for consistency with other clients. There is still the existing KeysAsync, which matches the existing StackExchange.Redis interface, but that does not provide support for the filtering keys by type.

Signed-off-by: currantw <[email protected]>

Copilot

Pull Request Overview

This PR implements cluster and standalone scan functionality for the Valkey GLIDE client. The changes introduce a consistent ScanAsync API across both client types while adding support for filtering by pattern, type, and count options.

Key Changes:

Added ScanAsync methods to IGenericCommands (standalone) and IGenericClusterCommands (cluster) interfaces with implementations in respective clients
Introduced ScanOptions class supporting match pattern, count, and type filtering, and ClusterScanCursor for cluster scan state management
Refactored internal Request.ScanAsync to use ScanOptions instead of individual parameters, updating cursor handling from long to string

Reviewed Changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/Valkey.Glide.UnitTests/CommandTests.cs	Updated unit tests to use new `ScanAsync` signature with `ScanOptions` and string cursors
tests/Valkey.Glide.IntegrationTests/ScanTests.cs	Added comprehensive integration tests for scan functionality with filtering options
sources/Valkey.Glide/abstract_APITypes/ClusterScanCursor.cs	New cursor abstraction for cluster scan operations
sources/Valkey.Glide/Internals/Request.GenericCommands.cs	Refactored scan command builder to accept `ScanOptions` and return string cursors
sources/Valkey.Glide/Internals/FFI.methods.cs	Added FFI bindings for cluster scan operations
sources/Valkey.Glide/GlideClusterClient.cs	Implemented `ScanAsync` for cluster client using new cluster scan command
sources/Valkey.Glide/GlideClient.cs	Implemented `ScanAsync` for standalone client and updated `KeysAsync` to use new scan interface
sources/Valkey.Glide/Commands/Options/ScanOptions.cs	New options class for scan configuration
sources/Valkey.Glide/Commands/IGenericCommands.cs	Added `ScanAsync` method signature for standalone client
sources/Valkey.Glide/Commands/IGenericClusterCommands.cs	Added `ScanAsync` method signature for cluster client
sources/Valkey.Glide/Commands/Constants/Constants.cs	Added TYPE keyword constant
sources/Valkey.Glide/BaseClient.cs	Added `ClusterScanCommand` implementation and relocated server version fields
rust/src/lib.rs	Implemented Rust FFI functions for cluster scan operations

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

sources/Valkey.Glide/GlideClient.cs

rust/src/lib.rs

Signed-off-by: currantw <[email protected]>

jduo · 2025-10-24T20:36:55Z

sources/Valkey.Glide/GlideClient.cs

        do
        {
-            (long nextCursor, ValkeyKey[] keys) = await Command(Request.ScanAsync(currentCursor, pattern, pageSize));
+            var options = new ScanOptions();


Nit: let's allocate this outside the loop and reuse

currantw self-assigned this Oct 22, 2025

currantw force-pushed the currantw/cluster-scan branch from cf4bbf1 to ca014f5 Compare October 22, 2025 00:57

currantw added 28 commits October 21, 2025 18:05

Add new ScanOptions class

b1ef63a

Signed-off-by: currantw <[email protected]>

FIx 'ScanOptions' linter error

69bab7f

Signed-off-by: currantw <[email protected]>

Add new ClusterScanCursor class

ea46eff

Signed-off-by: currantw <[email protected]>

Update ClusterScanCursor to not use IDisposable

d78ab52

Signed-off-by: currantw <[email protected]>

Add ScanAsync interface methods.

1444ee8

Signed-off-by: currantw <[email protected]>

Add stub ScanAsync implementations

80fe43b

Signed-off-by: currantw <[email protected]>

Update interface methods to return ValkeyKey instead of GlideString.

ee1ac58

Signed-off-by: currantw <[email protected]>

Revert previous change - string[] is more appropriate

e12d3f8

Signed-off-by: currantw <[email protected]>

Update Request.GenericCommands.ScanAsync to use ScanOptions

776179b

Signed-off-by: currantw <[email protected]>

Small simplification of GlideClient.KeysAsync

911feac

Signed-off-by: currantw <[email protected]>

Implement GlideClient.ScanAsync

b0c6af4

Signed-off-by: currantw <[email protected]>

Implement ClusterScanAsync and update ScanAsync for consistency i…

9768da2

…n `Request.GenericCommands`. Signed-off-by: currantw <[email protected]>

Implement GlideClusterClient.ScanAsync

a62127d

Signed-off-by: currantw <[email protected]>

Revert some changes. Cluster scan requires special FFI handling.

c960bfb

Signed-off-by: currantw <[email protected]>

Update .gitignore to ignore .amazonq/ directory (used to store "memor…

f1102d6

…y bank") Signed-off-by: currantw <[email protected]>

Unrelated cleanup: move _serverVersion into protected fields region

7a0e279

Signed-off-by: currantw <[email protected]>

Expose "request_cluster_scan" and "remove_cluster_scan_cursor" as FFI…

5c88eb7

… methods Signed-off-by: currantw <[email protected]>

Cleanup ClusterScanCursor and implement IDisposable interface.

88978ce

Signed-off-by: currantw <[email protected]>

Implement ClusterScanCommand, revert IDisposable for ClusterScanCursor.

b444c3b

Signed-off-by: currantw <[email protected]>

Fix linting errors

46b1a72

Signed-off-by: currantw <[email protected]>

Update scan methods to return ValkeyKey[] instead of string[]

f42b710

Signed-off-by: currantw <[email protected]>

Additional ScanOptions unit tests

37aaa06

Signed-off-by: currantw <[email protected]>

Make ClusterScanCursor's `CursorId immutable

aa1d412

Signed-off-by: currantw <[email protected]>

Add ClusterScanCursor unit tests

6942ba3

Signed-off-by: currantw <[email protected]>

Update SCAN command converter unit tests

d2c925c

Signed-off-by: currantw <[email protected]>

Add cluster scan integration tests

674a2fa

Signed-off-by: currantw <[email protected]>

Add standalone scan integration tests

0549b91

Signed-off-by: currantw <[email protected]>

Add ArgumentNullException test for cluster scan cursor

fde65d3

Signed-off-by: currantw <[email protected]>

currantw added 5 commits October 21, 2025 18:09

Combine cluster and standalone scan test files

dd7c52c

Signed-off-by: currantw <[email protected]>

Fix failing tests, linting, and formatting.

b821b09

Signed-off-by: currantw <[email protected]>

Add stub implementation for request_cluster_scan and `remove_cluste…

87d8060

…r_scan_cursor` in `lib.rs`. Signed-off-by: currantw <[email protected]>

Initial implementation of request_cluster_scan and `remove_cluster_…

b44f959

…scan_cursor`. Signed-off-by: currantw <[email protected]>

Update rust implementation, make more consistent with core implementa…

05d8fb9

…tion Signed-off-by: currantw <[email protected]>

currantw force-pushed the currantw/cluster-scan branch from ca014f5 to 05d8fb9 Compare October 22, 2025 01:09

Fix bug in the case of an existing cursor ID, update tests.

eb2e54c

Signed-off-by: currantw <[email protected]>

currantw commented Oct 22, 2025

View reviewed changes

.gitignore

.vs

.vscode

.kiro/

.amazonq/

Copy link

Collaborator Author

currantw Oct 22, 2025

Choose a reason for hiding this comment

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

Unrelated addition. Supports Amazon Q memory bank.

currantw commented Oct 22, 2025

View reviewed changes

currantw commented Oct 23, 2025

View reviewed changes

Minor cleanup.

0818667

Signed-off-by: currantw <[email protected]>

currantw commented Oct 23, 2025

View reviewed changes

Fix failing test

bf576ee

Signed-off-by: currantw <[email protected]>

currantw requested review from alexr-bq and Copilot October 23, 2025 17:23

Copilot AI reviewed Oct 23, 2025

View reviewed changes

sources/Valkey.Glide/GlideClient.cs Outdated Show resolved Hide resolved

rust/src/lib.rs Outdated Show resolved Hide resolved

currantw requested a review from jbrinkman October 23, 2025 17:23

currantw marked this pull request as ready for review October 23, 2025 17:47

currantw requested a review from a team as a code owner October 23, 2025 17:47

Addressed GitHub Copilot review comments

6e81c0e

Signed-off-by: currantw <[email protected]>

yipin-chen requested a review from jduo October 24, 2025 20:04

jduo reviewed Oct 24, 2025

View reviewed changes


		protected abstract Task<Version> GetServerVersionAsync();

		protected async Task<(string cursor, ValkeyKey[] keys)> ClusterScanCommand(string cursor, string[] args)

Uh oh!

Implement Cluster Scan #113

Are you sure you want to change the base?

Implement Cluster Scan #113

Uh oh!

Conversation

currantw commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Implement Cluster Scan

Overview

Key Changes

Testing Coverage

Breaking Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

currantw Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

currantw commented Oct 22, 2025 •

edited

Loading

currantw Oct 22, 2025 •

edited

Loading