-
Notifications
You must be signed in to change notification settings - Fork 2
Support Lazy Connect #104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Support Lazy Connect #104
Conversation
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
14b0978
to
9eef6a5
Compare
|
||
throw new ConnectionException("Failed creating a client"); | ||
return client; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is going on here? Async sort (see Request.GenericCommands.SortAsync
) in the C# is implemented such that it must know the server version. (I presume that this is to maintain compatibility with StackExchange.Redis
, since none of the other Valkey Glide client have this behaviour; instead, they have separate sort and sort read-only methods).

The server version is retrieved with the INFO command, so it cannot be performed during initialization, because this would prematurely trigger the connection with the server if lazy connection is enabled. I got around this by making the _serverVersion
attribute lazily initialized: it is only queried when it is first needed.
Signed-off-by: currantw <[email protected]>
87773ca
to
d279437
Compare
There was a problem hiding this comment.
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 adds lazy connection support to the Valkey Glide C# client, allowing clients to defer network connections until the first command is executed instead of establishing connections during initialization.
Key changes:
- Added
LazyConnect
configuration option to client builders and configuration classes - Modified server version determination to be lazy rather than happening during client initialization
- Added comprehensive integration tests to verify lazy vs eager connection behavior
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tests/Valkey.Glide.IntegrationTests/TestUtils.cs | New utility class for counting client connections in tests |
tests/Valkey.Glide.IntegrationTests/StandaloneClientTests.cs | Added lazy and eager connection tests for standalone client |
tests/Valkey.Glide.IntegrationTests/ClusterClientTests.cs | Added lazy and eager connection tests for cluster client |
sources/Valkey.Glide/Internals/FFI.structs.cs | Added LazyConnect field to FFI connection configuration |
sources/Valkey.Glide/GlideClusterClient.cs | Refactored server version initialization to be lazy |
sources/Valkey.Glide/GlideClient.cs | Refactored server version initialization to be lazy |
sources/Valkey.Glide/ConnectionConfiguration.cs | Added LazyConnect property and methods to configuration classes |
sources/Valkey.Glide/BaseClient.cs | Modified client creation flow and extracted common server version parsing |
sources/Valkey.Glide/BaseClient.GenericCommands.cs | Updated to use lazy server version retrieval |
rust/src/ffi.rs | Updated Rust FFI to handle lazy connect configuration |
DEVELOPER.md | Fixed typo in filename reference |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
[Fact] | ||
public async Task LazyConnect() | ||
{ | ||
using var referenceClient = TestConfiguration.DefaultClusterClient(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests may be flaky using the same server as the other tests. In other clients a dedicated server is spun up during this test, so it is isolated and don't affect the client count
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added support for dedicated servers by extracting existing logic from TestConfiguration
to a new ServerUtils
test class with methods StartClusterServer
, StartStandaloneServer
, and StopServer
. Updated eager and lazy connect tests to use these. Thanks for the comment! Hopefully this should address it.
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
…dedicated server. Signed-off-by: currantw <[email protected]>
1d63c4d
to
fa101c4
Compare
Summary
Adds support for lazy connection to the Valkey Glide C# client.
Description
This pull request add support for lazy connection to the Valkey Glide C# client. When lazy connect is enabled, the client is initialized without establishing network connections to the Valkey servers; instead, connections are only created when the first command is executed.
See Lazy Connect - Valkey Glide Wiki for more details, including how lazy connect works and important considerations.
Changes
BaseClient
and subclasses to lazily determine the server version. See description here of why this is necessary to enabled lazy connect.LazyConnect
attribute toConnectionConfiguration
,StandaloneClientConfiguration
,ClusterClientConfiguration
, andClientConfigurationBuilder
.lazyConnect
attribute toConnectionConfig
internal FFI struct.EagerConnect
andLazyConnect
tests toClusterClientTests
andStandaloneClientTests
, as well as a test utility methods for retrieving the number of server connections from a client.Related Issues