Skip to content

Commit faa4109

Browse files
mgravellmgravell
committed
PR nits
1 parent 62f9fd2 commit faa4109

10 files changed

Lines changed: 69 additions & 43 deletions

File tree

‎src/StackExchange.Redis/ExtensionMethods.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,8 @@ public static async ValueTask<bool> TryAcquireGcraAsync(
354354
CommandFlags flags = CommandFlags.None,
355355
CancellationToken cancellationToken = default)
356356
{
357+
cancellationToken.ThrowIfCancellationRequested();
358+
357359
var startTime = DateTime.UtcNow;
358360
var allowMilliseconds = allow.TotalMilliseconds;
359361

‎src/StackExchange.Redis/Gcra.GcraMessage.cs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
namespace StackExchange.Redis;
2+
3+
internal partial class RedisDatabase
4+
{
5+
internal sealed class GcraMessage(
6+
int database,
7+
CommandFlags flags,
8+
RedisKey key,
9+
int maxBurst,
10+
int requestsPerPeriod,
11+
double periodSeconds,
12+
int count) : Message(database, flags, RedisCommand.GCRA)
13+
{
14+
protected override void WriteImpl(in MessageWriter writer)
15+
{
16+
// GCRA key max_burst requests_per_period period [NUM_REQUESTS count]
17+
writer.WriteHeader(Command, ArgCount);
18+
writer.WriteBulkString(key);
19+
writer.WriteBulkString(maxBurst);
20+
writer.WriteBulkString(requestsPerPeriod);
21+
writer.WriteBulkString(periodSeconds);
22+
23+
if (count != 1)
24+
{
25+
writer.WriteBulkString("NUM_REQUESTS"u8);
26+
writer.WriteBulkString(count);
27+
}
28+
}
29+
30+
public override int ArgCount
31+
{
32+
get
33+
{
34+
int argCount = 4; // key, max_burst, requests_per_period, period
35+
if (count != 1) argCount += 2; // NUM_REQUESTS, count
36+
return argCount;
37+
}
38+
}
39+
}
40+
}

src/StackExchange.Redis/GcraRateLimitResult.cs renamed to src/StackExchange.Redis/Gcra.GcraRateLimitResult.cs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,8 @@ public readonly partial struct GcraRateLimitResult
3232
public int FullBurstAfterSeconds { get; }
3333

3434
/// <summary>
35-
/// Creates a new <see cref="GcraRateLimitResult"/>.
35+
/// Initializes a new instance of the <see cref="GcraRateLimitResult"/> struct.
3636
/// </summary>
37-
/// <param name="limited">Whether the request was rate limited.</param>
38-
/// <param name="maxRequests">The maximum number of requests allowed.</param>
39-
/// <param name="availableRequests">The number of requests available immediately.</param>
40-
/// <param name="retryAfterSeconds">The number of seconds after which to retry (in seconds). -1 if not limited.</param>
41-
/// <param name="fullBurstAfterSeconds">The number of seconds after which a full burst will be allowed (in seconds).</param>
4237
public GcraRateLimitResult(bool limited, int maxRequests, int availableRequests, int retryAfterSeconds, int fullBurstAfterSeconds)
4338
{
4439
Limited = limited;

src/StackExchange.Redis/GcraRateLimitResult.ResultProcessor.cs renamed to src/StackExchange.Redis/Gcra.ResultProcessor.cs

File renamed without changes.

‎src/StackExchange.Redis/GcraRateLimitResult.Message.cs‎

Lines changed: 0 additions & 17 deletions
This file was deleted.

‎src/StackExchange.Redis/HotKeys.ResultProcessor.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,11 @@ private HotKeysResult(ref RespReader reader)
5656
if (!reader.TryMoveNext()) break;
5757

5858
long i64;
59+
bool b;
5960
switch (field)
6061
{
61-
case HotKeysField.TrackingActive:
62-
TrackingActive = reader.ReadBoolean();
62+
case HotKeysField.TrackingActive when reader.TryReadBoolean(out b):
63+
TrackingActive = b;
6364
break;
6465
case HotKeysField.SampleRatio when reader.TryReadInt64(out i64):
6566
SampleRatio = i64;

‎src/StackExchange.Redis/RedisDatabase.Strings.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,13 @@ private Message GetStringDeleteMessage(in RedisKey key, in ValueCondition when,
4949

5050
public GcraRateLimitResult StringGcraRateLimit(RedisKey key, int maxBurst, int requestsPerPeriod, double periodSeconds = 1.0, int count = 1, CommandFlags flags = CommandFlags.None)
5151
{
52-
var msg = GetStringGcraRateLimitMessage(key, maxBurst, requestsPerPeriod, periodSeconds, count, flags);
52+
var msg = new GcraMessage(Database, flags, key, maxBurst, requestsPerPeriod, periodSeconds, count);
5353
return ExecuteSync(msg, ResultProcessor.GcraRateLimit);
5454
}
5555

5656
public Task<GcraRateLimitResult> StringGcraRateLimitAsync(RedisKey key, int maxBurst, int requestsPerPeriod, double periodSeconds = 1.0, int count = 1, CommandFlags flags = CommandFlags.None)
5757
{
58-
var msg = GetStringGcraRateLimitMessage(key, maxBurst, requestsPerPeriod, periodSeconds, count, flags);
58+
var msg = new GcraMessage(Database, flags, key, maxBurst, requestsPerPeriod, periodSeconds, count);
5959
return ExecuteAsync(msg, ResultProcessor.GcraRateLimit);
6060
}
6161

‎tests/StackExchange.Redis.Tests/GcraTestServer.cs‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,13 @@ protected virtual TypedRedisValue Gcra(RedisClient client, in RedisRequest reque
6767
};
6868

6969
// Return the configured result as a 5-element array
70-
var elements = new[]
71-
{
72-
TypedRedisValue.Integer(_expectedResult.Limited ? 1 : 0),
73-
TypedRedisValue.Integer(_expectedResult.MaxRequests),
74-
TypedRedisValue.Integer(_expectedResult.AvailableRequests),
75-
TypedRedisValue.Integer(_expectedResult.RetryAfterSeconds),
76-
TypedRedisValue.Integer(_expectedResult.FullBurstAfterSeconds),
77-
};
78-
return TypedRedisValue.MultiBulk(elements, RespPrefix.Array);
70+
var result = TypedRedisValue.Rent(5, out var span, RespPrefix.Array);
71+
span[0] = TypedRedisValue.Integer(_expectedResult.Limited ? 1 : 0);
72+
span[1] = TypedRedisValue.Integer(_expectedResult.MaxRequests);
73+
span[2] = TypedRedisValue.Integer(_expectedResult.AvailableRequests);
74+
span[3] = TypedRedisValue.Integer(_expectedResult.RetryAfterSeconds);
75+
span[4] = TypedRedisValue.Integer(_expectedResult.FullBurstAfterSeconds);
76+
return result;
7977
}
8078

8179
/// <summary>

‎tests/StackExchange.Redis.Tests/RoundTripUnitTests/GcraRateLimitRoundTrip.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public async Task GcraRateLimit_DefaultCount_RoundTrip(
1515
string requestResp,
1616
string responseResp)
1717
{
18-
var msg = Message.Create(0, CommandFlags.None, RedisCommand.GCRA, (RedisKey)key, maxBurst, requestsPerPeriod, periodSeconds);
18+
var msg = new RedisDatabase.GcraMessage(0, CommandFlags.None, key, maxBurst, requestsPerPeriod, periodSeconds, 1);
1919
var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.GcraRateLimit, requestResp, responseResp);
2020

2121
Assert.False(result.Limited);
@@ -36,7 +36,7 @@ public async Task GcraRateLimit_WithCount_RoundTrip(
3636
string requestResp,
3737
string responseResp)
3838
{
39-
var msg = Message.Create(0, CommandFlags.None, RedisCommand.GCRA, (RedisKey)key, maxBurst, requestsPerPeriod, periodSeconds, RedisLiterals.NUM_REQUESTS, count);
39+
var msg = new RedisDatabase.GcraMessage(0, CommandFlags.None, key, maxBurst, requestsPerPeriod, periodSeconds, count);
4040
var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.GcraRateLimit, requestResp, responseResp);
4141

4242
Assert.True(result.Limited);
@@ -56,7 +56,7 @@ public async Task GcraRateLimit_CustomPeriod_RoundTrip(
5656
string requestResp,
5757
string responseResp)
5858
{
59-
var msg = Message.Create(0, CommandFlags.None, RedisCommand.GCRA, (RedisKey)key, maxBurst, requestsPerPeriod, periodSeconds);
59+
var msg = new RedisDatabase.GcraMessage(0, CommandFlags.None, key, maxBurst, requestsPerPeriod, periodSeconds, 1);
6060
var result = await TestConnection.ExecuteAsync(msg, ResultProcessor.GcraRateLimit, requestResp, responseResp);
6161

6262
Assert.False(result.Limited);

‎toys/StackExchange.Redis.Server/TypedRedisValue.cs‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,25 @@ namespace StackExchange.Redis
1010
/// </summary>
1111
public readonly struct TypedRedisValue
1212
{
13-
// note: if this ever becomes exposed on the public API, it should be made so that it clears;
14-
// can't trust external callers to clear the space, and using recycle without that is dangerous
15-
internal static TypedRedisValue Rent(int count, out Span<TypedRedisValue> span, RespPrefix type)
13+
/// <summary>
14+
/// Rents an array from the pool and returns a <see cref="TypedRedisValue"/> that wraps it.
15+
/// The returned span is cleared to ensure safe usage.
16+
/// </summary>
17+
/// <param name="count">The number of elements to rent.</param>
18+
/// <param name="span">The span that can be used to populate the array.</param>
19+
/// <param name="type">The RESP type of the array.</param>
20+
/// <returns>A <see cref="TypedRedisValue"/> that wraps the rented array.</returns>
21+
public static TypedRedisValue Rent(int count, out Span<TypedRedisValue> span, RespPrefix type)
1622
{
1723
if (count == 0)
1824
{
1925
span = default;
2026
return EmptyArray(type);
2127
}
2228

23-
var arr = ArrayPool<TypedRedisValue>.Shared.Rent(count); // new TypedRedisValue[count];
29+
var arr = ArrayPool<TypedRedisValue>.Shared.Rent(count);
2430
span = new Span<TypedRedisValue>(arr, 0, count);
31+
span.Clear(); // Clear the span to ensure safe usage by external callers
2532
return new TypedRedisValue(arr, count, type);
2633
}
2734

0 commit comments

Comments
 (0)