Improve the DI extensions

[LeaFrock]

For the default DI of .NET, the official doc recommends: Keep DI factories fast and synchronous .

But the DI extension here has a related problem when using PoolSize:

services.AddSingleton((provider) => provider
            .GetRequiredService<IRedisClientFactory>()
            .GetDefaultRedisClient()
            .GetDefaultDatabase());

As the IRedisDatabase is a singleton, the initialization will be executed in a single thread with lock.

When creating RedisClientFactory, RedisConnectionPoolManager needs to be created, whose constructor contains a method called EmitConnections:

    public RedisConnectionPoolManager(RedisConfiguration redisConfiguration, ILogger<RedisConnectionPoolManager>? logger = null)
    {
        this.redisConfiguration = redisConfiguration ?? throw new ArgumentNullException(nameof(redisConfiguration));
        this.logger = logger ?? NullLogger<RedisConnectionPoolManager>.Instance;

        lock (@lock)
        {
            connections = new IStateAwareConnection[redisConfiguration.PoolSize];
            EmitConnections();
        }
    }

    private void EmitConnections()
    {
        for (var i = 0; i < redisConfiguration.PoolSize; i++)
        {
            var multiplexer = ConnectionMultiplexer.Connect(redisConfiguration.ConfigurationOptions);

            if (redisConfiguration.ProfilingSessionProvider != null)
                multiplexer.RegisterProfiler(redisConfiguration.ProfilingSessionProvider);

            connections[i] = redisConfiguration.StateAwareConnectionFactory(multiplexer, logger);
        }
    }

Notice the for code lines, it takes an obvious amount of time to finish.

Suppose the following scenario:

1. the web API runs
2. a lot of controller actions depending on IRedisDatabase are called
3. one of them enter the lock of the initialization of IRedisDatabase; others have to wait
4. during the meantime (maybe above 1000ms), all related requests are blocked, which mean their threads are waiting too
5. the requests are growing, and the app has to create many new threads and allocate more (virtual) memory
6. the app becomes not stable, and even crashed

Recently I've met this kind of crash problem on IIS, while ASP.NET Core is in-process hosted with a 32-bit work process mode, which offers only 4G virtual memory. The app throws an OutOfMemoryException finally while starting, as long as the concurrency in the meanwhile is high enough.

[LeaFrock]

At least I think this point should be specifically documented.

Currently I suggest the following codes before app.Run to prevent the problem:

// ...
var app = builder.Build();
var redisDb = app.Services.GetRequiredService<IRedisDatabase>();
redisDb.Ping();
// ...
app.Run();

[imperugo]

Hi @LeaFrock

I've partially documented that part here /~https://github.com/imperugo/StackExchange.Redis.Extensions/blob/master/doc/configuration/connection-pool.md when I'm saying

Depending on the size of the pool, the client, on startup, opens a connection to the server. This means that the higher the pool size the longer the application startup time and the number of connections used.

Probably a goog idea cold be to open all the connection in parallels (right now is made one by one by EmitConnections method).

Moreover the connection array could be frozen in case of .NET 8

Let me test it

[imperugo]

Another idea could be to create and extension method alled "WaitRedisConnectionsAndRun" or a sort of bootstrap tasks that wait redis

[imperugo]

here a possible solution (tested) with parallel runs

private void EmitConnections()
    {
            Parallel.For(0, redisConfiguration.PoolSize, index =>
            {
                var multiplexer = ConnectionMultiplexer.Connect(redisConfiguration.ConfigurationOptions);

                if (redisConfiguration.ProfilingSessionProvider != null)
                    multiplexer.RegisterProfiler(redisConfiguration.ProfilingSessionProvider);

                connections[index] =  redisConfiguration.StateAwareConnectionFactory(multiplexer, logger);
            });
    }

Let me know what u think

[LeaFrock]

This only alleviates the problem but does not ultimately solve it.

It would be best if we can initialize the connections among the extension method immediately, which means all actions of connecting occur before app.Run rather than after.

I‘ve make a commit in my fork. The commit is indeed a break change but brings lower risk of usage. Just give a reference and I hope you think about it at your free time.

Your reply is welcomed.

[imperugo]

Hi @LeaFrock
I checked your commit but the breaking change seems important.

I'm going to release a new version that is using the parallelization of the pool inizialization in few minutes.
Please test it and let me know if is better.

Thanks

[imperugo]

Honestly looks weird,

What configuration are you using?

[LeaFrock]

What configuration are you using?

  "Redis": {
    "Name": "xxxx",
    "Password": "xxxxxx",
    "AllowAdmin": true,
    "Ssl": false,
    "ConnectTimeout": 5000,
    "ConnectRetry": 2,
    "Database": 0,
    "Hosts": [
      {
        "Host": "xxxxx",
        "Port": "6379"
      }
    ],
    "MaxValueLength": 102400,
    "PoolSize": 5,
    "KeyPrefix": "xxxxxx"
  }

Our project is ASP.NET Core 8. It was deployed on IIS and hosted with a 32-bits work process mode.

This out-of-virtual-memory problem is really disgusting. Now I change the mode to 64-bits and it works fine.

[imperugo]

Honestly I've no idea how to fix it.

Probably a way could be to verify if the app is running on 32 bit or not and switch from parallel to old approach

[LeaFrock]

verify if the app is running on 32 bit or not

As long as the connections are created after app.Run, the problem is always on.

I've no idea how to fix it

Yep, it seems impossible to fix without break changes.

The ideal solution is making all connections occur during the DI initialization step. After verifying the OS environment, choose to run in parallel if possible, or a simple for to 32-bits (at least it works if connections are created before app.Run).

[LeaFrock]

It seems that no one else has encountered the same situation as me, and I have already changed to 64-bits.

Let's leave this issue at that until we meet more similar reports or have chance to make some break changes.