Skip to content
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

Redis Cache Rate Limiting to InMemory FallBack Not Working #241

Closed
manisha201301 opened this issue Jul 28, 2021 · 2 comments
Closed

Redis Cache Rate Limiting to InMemory FallBack Not Working #241

manisha201301 opened this issue Jul 28, 2021 · 2 comments

Comments

@manisha201301
Copy link

Hello everyone.,

I have been trying to implement Redis Cache rate limiting which falls back to In-Memory rate limiting. I am trying to implement IRateLimitCounterStore (using the entire class ) just as the Solution2 mentioned here: https://github.com/stefanprodan/AspNetCoreRateLimit/wiki/Using-Redis-as-a-distributed-counter-store

Somehow, it is not even hitting the GetAsync or SetAsync methods, even though the class is injected and passed down the service configuration.

Any ideas on how to implement it? Or what could be going wrong?

I am implementing it like this
Startup.cs

public void Configure(...) 
{
     app.UseCustomRateLimiting();
}

public void CofigureServices() 
{
    services.Configure<ClientRateLimitOptions>(Configuration.GetSection("ClientRateLimiting"));
    services.AddSingleton<IConnectionMultiplexer>(provider => 
    ConnectionMultiplexer.Connect(Configuration[redisConnectionKey]));
    services.AddRedisRateLimiting();

    services.AddSingleton<IRateLimitCounterStore, RedisRateLimitCounterStore>();
    services.AddSingleton<IRateLimitConfiguration, CustomRateLimitConfiguration>();
}

public static class RateLimitingMiddlewareExtensions
{
    public static IApplicationBuilder UseCustomRateLimiting(this IApplicationBuilder builder)
    {
        return builder.UseMiddleware<CutomRateLimitingLogging>();
      }
}

This is my Custom class to override LogBlockedRequest

    public class CutomRateLimitingLogging : ClientRateLimitMiddleware
    {
        private readonly IMultiLogger Logger;

        public CutomRateLimitingLogging(RequestDelegate next, IProcessingStrategy processingStrategy, IOptions<ClientRateLimitOptions> options, IRateLimitCounterStore counterStore, IClientPolicyStore policyStore, IRateLimitConfiguration config, ILogger<ClientRateLimitMiddleware> logger, IMultiLogger loggerOut)
            : base(next, processingStrategy, options, counterStore, policyStore, config, logger)
        {
            Logger = loggerOut;
        }

        protected override void LogBlockedRequest(HttpContext httpContext, ClientRequestIdentity identity, RateLimitCounter counter, RateLimitRule rule)
        {
            Logger.LogWarning(...);
        }
    }
@kmcoulson
Copy link

I'm hitting the same issue, did you get anywhere with it?

@BalintBanyasz
Copy link

This works fine for me:

// needed to load configuration from appsettings.json
services.AddOptions();

// needed to store rate limit counters and ip rules
services.AddMemoryCache();

//load general configuration from appsettings.json
services.Configure<ClientRateLimitOptions>(configuration.GetSection("ClientRateLimiting"));

//load client rules from appsettings.json
services.Configure<ClientRateLimitPolicies>(configuration.GetSection("ClientRateLimitPolicies"));

// inject counter and rules stores
var redisOptions = ConfigurationOptions.Parse(configuration["ConnectionStrings:Redis"]);
//services.AddSingleton<IConnectionMultiplexer>(provider => ConnectionMultiplexer.Connect(redisOptions));

//services.AddRedisRateLimiting();

services.AddSingleton(Options.Create(new RedisOptions(redisOptions.ToString())));
services.AddSingleton<IClientPolicyStore, MemoryCacheClientPolicyStore>();
services.AddSingleton<IRateLimitCounterStore, RedisRateLimitCounterStore>();
services.AddSingleton<IProcessingStrategy, AsyncKeyLockProcessingStrategy>();

// configuration (resolvers, counter key builders)
services.AddSingleton<IRateLimitConfiguration, RateLimitConfiguration>();

RedisOptions.cs

public class RedisOptions
{
    public string ConnectionString { get; }

    public bool Enabled { get; }

    public RedisOptions(string connectionString, bool enabled = true)
    {
        ConnectionString = connectionString;
        Enabled = enabled;
    }
}

I've noticed a significant performance gain after removing the Redis IsConnected check from TryRedisCommandAsync (when Redis is not available):

private async Task<T> TryRedisCommandAsync<T>(Func<Task<T>> command, Func<Task<T>> fallbackCommand)
        {
            if (_redisOptions?.Enabled == true) // && _redis?.IsConnected == true)

The only downside is that there is a concurrency issue with the default AsyncKeyLockProcessingStrategy implementation. The ideal solution would be a custom processing strategy that tries to use RedisProcessingStrategy and falls back to AsyncKeyLockProcessingStrategy.

It would be nice if there was an officially supported option for Redis with In-Memory fallback (or at least a separate package) and it could be simply added via AddRedisRateLimitingWithInMemoryFallback().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants