Skip to content

Conversation

@claudiamurialdo
Copy link
Collaborator

Replaces the default RedisCache behavior (https://github.com/dotnet/aspnetcore/blob/main/src/Caching/StackExchangeRedis/src/RedisCache.cs) , which refreshes the session TTL on every access to the web session, with a custom implementation that renews it only when near expiry (below 20% of the idle timeout).
This reduces redundant EXPIRE calls and Redis roundtrips while maintaining consistent session expiration behavior.

Issue:206131

claudiamurialdo added 3 commits November 3, 2025 10:09
…ns in Redis and improve session handling performance

(cherry picked from commit 2f29b17)

# Conflicts:
#	dotnet/src/dotnetcore/GxNetCoreStartup/SessionHelper.cs
(cherry picked from commit 04bd601)

# Conflicts:
#	dotnet/src/dotnetcore/GxNetCoreStartup/SessionHelper.cs
(cherry picked from commit aebd49a)
…ns in Redis and improve session handling performance
@genexusbot
Copy link
Collaborator

Cherry pick to beta success

fedeazzato
fedeazzato previously approved these changes Nov 4, 2025
Copy link
Member

@fedeazzato fedeazzato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm struggling to see how the tradeoff between the increase in TTL invocations results in performance gains due to the reduction of EXIPRE invocations, but I trust your findings.

Comment on lines 96 to 100
var ttl = await _db.KeyTimeToLiveAsync(redisKey);

if (ShouldRefreshKey(ttl))
{
_ = _db.KeyExpireAsync(redisKey, _idleTimeout);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really more performant to check for the TTL (invoking a Redis operation) before expiring the key? Isn't the Redis endpoint hit anyway?

Shouldn't the invocation to KeyExpireAsync be awaited?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking the TTL is cheaper than refreshing on every GetAsync because TTL is a read-only operation that only retrieves metadata, whereas KeyExpire (Refresh) modifies Redis internal state. By reading TTL first, it only refreshes when necessary, reducing write operations, CPU load, and latency on the Redis server.

Added the missing await.
Good catch. I added await to _db.KeyExpireAsync to ensure the TTL refresh completes reliably. The operation was fire-and-forget, which could lead to the session not being refreshed under high load or transient Redis.

Comment on lines 78 to 83
var ttl = _db.KeyTimeToLive(redisKey);

if (ShouldRefreshKey(ttl))
{
_db.KeyExpire(redisKey, _idleTimeout);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. How checking for TTL before invoking expire results in performance gains, if both must reach a Redis?

- Introduce IdleTimeoutRefreshRatio constant (0.30) for configurable refresh threshold
- Ensure session TTL is refreshed correctly on writes (Set/SetAsync)
@genexusbot
Copy link
Collaborator

Cherry pick to beta success

Copy link
Member

@fedeazzato fedeazzato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! 💯

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

Successfully merging this pull request may close these issues.

4 participants