Skip to content
2 changes: 1 addition & 1 deletion DEVELOPER.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ cargo fmt --all -- --check

6. Test framework and style

The CSharp Valkey-Glide client uses xUnit v3 for testing code. The test code styles are defined in `.editorcofing` (see `dotnet_diagnostic.xUnit..` rules). The xUnit rules are enforced by the [xUnit analyzers](https://github.com/xunit/xunit.analyzers) referenced in the main xunit.v3 NuGet package. If you choose to use xunit.v3.core instead, you can reference xunit.analyzers explicitly. For additional info, please, refer to https://xunit.net and https://github.com/xunit/xunit
The CSharp Valkey-Glide client uses xUnit v3 for testing code. The test code styles are defined in `.editorconfig` (see `dotnet_diagnostic.xUnit..` rules). The xUnit rules are enforced by the [xUnit analyzers](https://github.com/xunit/xunit.analyzers) referenced in the main xunit.v3 NuGet package. If you choose to use xunit.v3.core instead, you can reference xunit.analyzers explicitly. For additional info, please, refer to <https://xunit.net> and <https://github.com/xunit/xunit>

## Community and Feedback

Expand Down
3 changes: 2 additions & 1 deletion rust/src/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ pub struct ConnectionConfig {
pub protocol: redis::ProtocolVersion,
/// zero pointer is valid, means no client name is given (`None`)
pub client_name: *const c_char,
pub lazy_connect: bool,
/*
TODO below
pub periodic_checks: Option<PeriodicCheck>,
Expand Down Expand Up @@ -147,11 +148,11 @@ pub(crate) unsafe fn create_connection_request(
} else {
None
},
lazy_connect: config.lazy_connect,
// TODO below
periodic_checks: None,
pubsub_subscriptions: None,
inflight_requests_limit: None,
lazy_connect: false,
}
}

Expand Down
2 changes: 1 addition & 1 deletion sources/Valkey.Glide/BaseClient.GenericCommands.cs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public async Task<bool> KeyCopyAsync(ValkeyKey sourceKey, ValkeyKey destinationK
public async Task<ValkeyValue[]> SortAsync(ValkeyKey key, long skip = 0, long take = -1, Order order = Order.Ascending, SortType sortType = SortType.Numeric, ValkeyValue by = default, ValkeyValue[]? get = null, CommandFlags flags = CommandFlags.None)
{
Utils.Requires<NotImplementedException>(flags == CommandFlags.None, "Command flags are not supported by GLIDE");
return await Command(Request.SortAsync(key, skip, take, order, sortType, by, get, _serverVersion));
return await Command(Request.SortAsync(key, skip, take, order, sortType, by, get, await GetServerVersionAsync()));
}

public async Task<long> SortAndStoreAsync(ValkeyKey destination, ValkeyKey key, long skip = 0, long take = -1, Order order = Order.Ascending, SortType sortType = SortType.Numeric, ValkeyValue by = default, ValkeyValue[]? get = null, CommandFlags flags = CommandFlags.None)
Expand Down
16 changes: 10 additions & 6 deletions sources/Valkey.Glide/BaseClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,12 @@ protected static async Task<T> CreateClient<T>(BaseClientConfiguration config, F
CreateClientFfi(request.ToPtr(), successCallbackPointer, failureCallbackPointer);
client._clientPointer = await message; // This will throw an error thru failure callback if any

if (client._clientPointer != IntPtr.Zero)
if (client._clientPointer == IntPtr.Zero)
{
// Initialize server version after successful connection
await client.InitializeServerVersionAsync();
return client;
throw new ConnectionException("Failed creating a client");
}

throw new ConnectionException("Failed creating a client");
return client;
}
Copy link
Collaborator Author

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).

image

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.


protected BaseClient()
Expand Down Expand Up @@ -127,6 +125,11 @@ internal virtual async Task<T> Command<R, T>(Cmd<R, T> command, Route? route = n

// All memory allocated is auto-freed by `using` operator
}
protected Version? ParseServerVersion(string response)
{
var versionMatch = System.Text.RegularExpressions.Regex.Match(response, @"(?:valkey_version|redis_version):([\d\.]+)");
return versionMatch.Success ? new(versionMatch.Groups[1].Value) : null;
}
#endregion protected methods

#region private methods
Expand All @@ -145,7 +148,7 @@ private void FailureCallback(ulong index, IntPtr strPtr, RequestErrorType errTyp

internal void SetInfo(string info) => _clientInfo = info;

protected abstract Task InitializeServerVersionAsync();
protected abstract Task<Version> GetServerVersionAsync();

[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
private delegate void SuccessAction(ulong index, IntPtr ptr);
Expand All @@ -169,6 +172,7 @@ private void FailureCallback(ulong index, IntPtr strPtr, RequestErrorType errTyp
private readonly object _lock = new();
private string _clientInfo = ""; // used to distinguish and identify clients during tests
protected Version? _serverVersion; // cached server version
protected static readonly Version DefaultServerVersion = new(8, 0, 0);

#endregion private fields
}
33 changes: 29 additions & 4 deletions sources/Valkey.Glide/ConnectionConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ internal record ConnectionConfig
public uint DatabaseId;
public Protocol? Protocol;
public string? ClientName;
public bool LazyConnect;

internal FFI.ConnectionConfig ToFfi() =>
new(Addresses, TlsMode, ClusterMode, (uint?)RequestTimeout?.TotalMilliseconds, (uint?)ConnectionTimeout?.TotalMilliseconds, ReadFrom, RetryStrategy, AuthenticationInfo, DatabaseId, Protocol, ClientName);
new(Addresses, TlsMode, ClusterMode, (uint?)RequestTimeout?.TotalMilliseconds, (uint?)ConnectionTimeout?.TotalMilliseconds, ReadFrom, RetryStrategy, AuthenticationInfo, DatabaseId, Protocol, ClientName, LazyConnect);
}

/// <summary>
Expand Down Expand Up @@ -171,7 +172,7 @@ public abstract class BaseClientConfiguration
/// <summary>
/// Configuration for a standalone client. <br />
/// Use <see cref="StandaloneClientConfigurationBuilder" /> or
/// <see cref="StandaloneClientConfiguration(List{ValueTuple{string?, ushort?}}, bool?, TimeSpan?, TimeSpan?, ReadFrom?, RetryStrategy?, string?, string?, uint?, Protocol?, string?)" /> to create an instance.
/// <see cref="StandaloneClientConfiguration(List{ValueTuple{string?, ushort?}}, bool?, TimeSpan?, TimeSpan?, ReadFrom?, RetryStrategy?, string?, string?, uint?, Protocol?, string?, bool)" /> to create an instance.
/// </summary>
public sealed class StandaloneClientConfiguration : BaseClientConfiguration
{
Expand All @@ -190,6 +191,7 @@ internal StandaloneClientConfiguration() { }
/// <param name="databaseId"><inheritdoc cref="ClientConfigurationBuilder{T}.DataBaseId" path="/summary" /></param>
/// <param name="protocol"><inheritdoc cref="ClientConfigurationBuilder{T}.ProtocolVersion" path="/summary" /></param>
/// <param name="clientName"><inheritdoc cref="ClientConfigurationBuilder{T}.ClientName" path="/summary" /></param>
/// <param name="lazyConnect"><inheritdoc cref="ClientConfigurationBuilder{T}.LazyConnect" path="/summary" /></param>
public StandaloneClientConfiguration(
List<(string? host, ushort? port)> addresses,
bool? useTls = null,
Expand All @@ -201,7 +203,8 @@ public StandaloneClientConfiguration(
string? password = null,
uint? databaseId = null,
Protocol? protocol = null,
string? clientName = null
string? clientName = null,
bool lazyConnect = false
)
{
StandaloneClientConfigurationBuilder builder = new();
Expand All @@ -215,6 +218,7 @@ public StandaloneClientConfiguration(
_ = databaseId.HasValue ? builder.DataBaseId = databaseId.Value : new();
_ = protocol.HasValue ? builder.ProtocolVersion = protocol.Value : new();
_ = clientName is not null ? builder.ClientName = clientName : "";
builder.LazyConnect = lazyConnect;
Request = builder.Build().Request;
}
}
Expand All @@ -239,6 +243,7 @@ internal ClusterClientConfiguration() { }
/// <param name="databaseId"><inheritdoc cref="ClientConfigurationBuilder{T}.DataBaseId" path="/summary" /></param>
/// <param name="protocol"><inheritdoc cref="ClientConfigurationBuilder{T}.ProtocolVersion" path="/summary" /></param>
/// <param name="clientName"><inheritdoc cref="ClientConfigurationBuilder{T}.ClientName" path="/summary" /></param>
/// <param name="lazyConnect"><inheritdoc cref="ClientConfigurationBuilder{T}.LazyConnect" path="/summary" /></param>
public ClusterClientConfiguration(
List<(string? host, ushort? port)> addresses,
bool? useTls = null,
Expand All @@ -250,7 +255,8 @@ public ClusterClientConfiguration(
string? password = null,
uint? databaseId = null,
Protocol? protocol = null,
string? clientName = null
string? clientName = null,
bool lazyConnect = false
)
{
ClusterClientConfigurationBuilder builder = new();
Expand All @@ -264,6 +270,7 @@ public ClusterClientConfiguration(
_ = databaseId.HasValue ? builder.DataBaseId = databaseId.Value : new();
_ = protocol.HasValue ? builder.ProtocolVersion = protocol.Value : new();
_ = clientName is not null ? builder.ClientName = clientName : "";
builder.LazyConnect = lazyConnect;
Request = builder.Build().Request;
}
}
Expand Down Expand Up @@ -534,6 +541,24 @@ public T WithDataBaseId(uint dataBaseId)
return (T)this;
}
#endregion
#region Lazy Connect
/// <summary>
/// Configure whether to defer connections until the first command is executed.<br />
/// If not explicitly set, a default value of <c>false</c> will be used.
/// </summary>
public bool LazyConnect
{
get => Config.LazyConnect;
set => Config.LazyConnect = value;
}

/// <inheritdoc cref="LazyConnect" />
public T WithLazyConnect(bool lazyConnect)
{
LazyConnect = lazyConnect;
return (T)this;
}
#endregion

internal ConnectionConfig Build() => Config;
}
Expand Down
22 changes: 11 additions & 11 deletions sources/Valkey.Glide/GlideClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -196,21 +196,21 @@ public async IAsyncEnumerable<ValkeyKey> KeysAsync(int database = -1, ValkeyValu
} while (currentCursor != 0);
}

protected override async Task InitializeServerVersionAsync()
protected override async Task<Version> GetServerVersionAsync()
{
try
if (_serverVersion == null)
{
var infoResponse = await Command(Request.Info([InfoOptions.Section.SERVER]));
var versionMatch = System.Text.RegularExpressions.Regex.Match(infoResponse, @"(?:valkey_version|redis_version):([\d\.]+)");
if (versionMatch.Success)
try
{
_serverVersion = new Version(versionMatch.Groups[1].Value);
var infoResponse = await Command(Request.Info([InfoOptions.Section.SERVER]));
_serverVersion = ParseServerVersion(infoResponse) ?? DefaultServerVersion;
}
catch
{
_serverVersion = DefaultServerVersion;
}
}
catch
{
// If we can't get version, assume newer version (use SORT_RO)
_serverVersion = new Version(8, 0, 0);
}

return _serverVersion;
}
}
22 changes: 11 additions & 11 deletions sources/Valkey.Glide/GlideClusterClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -301,21 +301,21 @@ public async Task<string> SelectAsync(long index, CommandFlags flags = CommandFl
return await Command(Request.Select(index), Route.Random);
}

protected override async Task InitializeServerVersionAsync()
protected override async Task<Version> GetServerVersionAsync()
{
try
if (_serverVersion == null)
{
var infoResponse = await Command(Request.Info([InfoOptions.Section.SERVER]).ToClusterValue(true), Route.Random);
var versionMatch = System.Text.RegularExpressions.Regex.Match(infoResponse.SingleValue, @"(?:valkey_version|redis_version):([\d\.]+)");
if (versionMatch.Success)
try
{
_serverVersion = new Version(versionMatch.Groups[1].Value);
var infoResponse = await Command(Request.Info([InfoOptions.Section.SERVER]).ToClusterValue(true), Route.Random);
_serverVersion = ParseServerVersion(infoResponse.SingleValue) ?? DefaultServerVersion;
}
catch
{
_serverVersion = DefaultServerVersion;
}
}
catch
{
// If we can't get version, assume newer version (use SORT_RO)
_serverVersion = new Version(8, 0, 0);
}

return _serverVersion;
}
}
6 changes: 5 additions & 1 deletion sources/Valkey.Glide/Internals/FFI.structs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ public ConnectionConfig(
AuthenticationInfo? authenticationInfo,
uint databaseId,
ConnectionConfiguration.Protocol? protocol,
string? clientName)
string? clientName,
bool lazyConnect = false)
{
_addresses = addresses;
_request = new()
Expand All @@ -235,6 +236,7 @@ public ConnectionConfig(
HasProtocol = protocol.HasValue,
Protocol = protocol ?? default,
ClientName = clientName,
LazyConnect = lazyConnect,
};
}

Expand Down Expand Up @@ -770,6 +772,8 @@ private struct ConnectionRequest
public ConnectionConfiguration.Protocol Protocol;
[MarshalAs(UnmanagedType.LPStr)]
public string? ClientName;
[MarshalAs(UnmanagedType.U1)]
public bool LazyConnect;
// TODO more config params, see ffi.rs
}

Expand Down
67 changes: 67 additions & 0 deletions tests/Valkey.Glide.IntegrationTests/ClusterClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Valkey.Glide.Pipeline;

using static Valkey.Glide.Commands.Options.InfoOptions;
using static Valkey.Glide.ConnectionConfiguration;
using static Valkey.Glide.Errors;
using static Valkey.Glide.Route;

Expand Down Expand Up @@ -585,4 +586,70 @@ public async Task TestKeyCopyAsync(GlideClusterClient client)
// Verify the source key still exists in the current database
Assert.True(await client.KeyExistsAsync(sourceKey));
}

[Fact]
public async Task LazyConnect()
{
string serverName = $"test_{Guid.NewGuid():N}";

try
{
// Create dedicated server.
var addresses = ServerUtils.StartClusterServer(serverName);
var configBuilder = new ClusterClientConfigurationBuilder()
.WithAddress(addresses[0].host, addresses[0].port);
var eagerConfig = configBuilder.WithLazyConnect(false).Build();
var lazyConfig = configBuilder.WithLazyConnect(true).Build();

// Create reference client.
using var referenceClient = await GlideClusterClient.CreateClient(eagerConfig);
var initialCount = await TestUtils.GetConnectionCount(referenceClient);

// Create lazy client (does not connect immediately).
using var lazyClient = await GlideClusterClient.CreateClient(lazyConfig);
var connectCount = await TestUtils.GetConnectionCount(referenceClient);

Assert.Equal(initialCount, connectCount);

// First command establishes connection.
await lazyClient.PingAsync();
var commandCount = await TestUtils.GetConnectionCount(referenceClient);

Assert.True(connectCount < commandCount);
}
finally
{
ServerUtils.StopServer(serverName);
}
}

[Fact]
public async Task EagerConnect()
{
string serverName = $"test_{Guid.NewGuid():N}";

try
{
// Create dedicated server.
var addresses = ServerUtils.StartClusterServer(serverName);
var eagerConfig = new ClusterClientConfigurationBuilder()
.WithAddress(addresses[0].host, addresses[0].port)
.WithLazyConnect(false)
.Build();

// Create reference client.
using var referenceClient = await GlideClusterClient.CreateClient(eagerConfig);
var initialCount = await TestUtils.GetConnectionCount(referenceClient);

// Create eager client (connects immediately).
using var eagerClient = await GlideClusterClient.CreateClient(eagerConfig);
var connectCount = await TestUtils.GetConnectionCount(referenceClient);

Assert.True(initialCount < connectCount);
}
finally
{
ServerUtils.StopServer(serverName);
}
}
}
Loading
Loading