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

HybridCache: LocalCacheExpiration is not relative to Expiration #5824

Open
zmj opened this issue Jan 29, 2025 · 1 comment
Open

HybridCache: LocalCacheExpiration is not relative to Expiration #5824

zmj opened this issue Jan 29, 2025 · 1 comment
Labels
bug This issue describes a behavior which is not expected - a bug. untriaged

Comments

@zmj
Copy link

zmj commented Jan 29, 2025

Description

The docstring for HybridCacheEntryOptions.LocalCacheExpiration says:

When retrieving a cached value from an external cache store, this value will be used to calculate the local
cache expiration, not exceeding the remaining overall cache lifetime.

That doesn't seem to match the behavior in DefaultHybridCache.SetL1: https://github.com/dotnet/extensions/blob/main/src/Libraries/Microsoft.Extensions.Caching.Hybrid/Internal/DefaultHybridCache.L2.cs#L108 I don't see anything here that respects the overall lifetime.

My scenario is that the value I'm caching has a constant valid lifetime, which I set as HybridCacheEntryOptions.Expiration. LocalCacheExpiration is a lower value, to free up memory that we suspect won't be used based on usage patterns. However, I see some errors due to usage of an expired value out of GetOrCreateAsync.

I suspect what's happening is:

  1. First GetOrCreateAsync is an L1 cache miss, and L2 hit. Could be any of: this instance never having the value in L1, or the value expiring due to the lower LocalCacheExpiration, or memory pressure. The retrieved value's true relative expiration (createdAt + options.Expiration - Now) is less than LocalCacheExpiration. L1 cache TTL is set to LocalCacheExpiration.
  2. Second GetOrCreateAsync is an L1 cache hit, but it's in the time window after the value has expired. Using the value fails.

I could work around this by including my value's expiration in my cache data and invalidating appropriately, but I'm unclear whether that's intended usage. The docstring for LocalCacheExpiration suggests otherwise - that the L2 serialized payload would include the creation time, and the L1 TTL would be set relative to it.

Reproduction Steps

can provide if description is unclear

Expected behavior

HybridCacheEntryOptions.Expiration is an upper bound on L1 TTL

Actual behavior

L1 TTL is only equal to HybridCacheEntryOptions.LocalCacheExpiration

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

@zmj zmj added bug This issue describes a behavior which is not expected - a bug. untriaged labels Jan 29, 2025
@zmj
Copy link
Author

zmj commented Feb 7, 2025

I created a repro for this. Version="9.1.0-preview.1.25064.3"

using Microsoft.Extensions.Caching.Distributed;
using Microsoft.Extensions.Caching.Hybrid;
using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Internal;
using Microsoft.Extensions.Options;
using Microsoft.Extensions.Time.Testing;

var fakeTime = new FakeTimeProvider();

var builder = Host.CreateEmptyApplicationBuilder(settings: null);
builder.Services.AddSingleton<TimeProvider>(fakeTime);
builder.Services.AddMemoryCache(options => options.Clock = new SystemClock(fakeTime));
builder.Services.AddSingleton<IDistributedCache, FakeDistributedCache>();
builder.Services.AddHybridCache(options => options.DefaultEntryOptions = new()
{
    Expiration = .99 * Value.ExpiresIn, // as long as possible minus clock skew
    LocalCacheExpiration = .5 * Value.ExpiresIn, // lower value in case it's not needed again
});
var app = builder.Build();
var cache = app.Services.GetRequiredService<HybridCache>();

const string cacheKey = "key";
// setup: the value is stored in L2 only
_ = await cache.GetOrCreateAsync(cacheKey, state: fakeTime, (state, _) => Value.Create(tp: state), new HybridCacheEntryOptions { Flags = HybridCacheEntryFlags.DisableLocalCacheWrite });
// time passes: the value is close to expiration but not expired
fakeTime.Advance(.8 * Value.ExpiresIn);
// get and use the value, adding it to L1
var v1 = await cache.GetOrCreateAsync(cacheKey, state: fakeTime, (state, _) => Value.Create(tp: state));
v1.Use(fakeTime); // this is ok
// more time passes: the value is now logically expired, and expired from L2, but still present in L1
fakeTime.Advance(.4 * Value.ExpiresIn);
// get and use the value again: should miss cache and get underlying data, but instead is an L1 hit
var v2 = await cache.GetOrCreateAsync(cacheKey, state: fakeTime, (state, _) => Value.Create(tp: state));
v2.Use(fakeTime); // bang

record Value(DateTimeOffset ExpiresAt)
{
    public void Use(TimeProvider tp)
    {
        if (tp.GetUtcNow() > ExpiresAt)
        {
            throw new Exception("attempted to use expired value");
        }
    }

    public static TimeSpan ExpiresIn { get; } = TimeSpan.FromMinutes(60);

    public static ValueTask<Value> Create(TimeProvider tp) => new(new Value(ExpiresAt: tp.GetUtcNow() + ExpiresIn));
}

// wrapper to set _backendCache
class FakeDistributedCache(TimeProvider tp) : MemoryDistributedCache(Options.Create(new MemoryDistributedCacheOptions
{
    Clock = new SystemClock(tp),
}));

class SystemClock(TimeProvider tp) : ISystemClock
{
    public DateTimeOffset UtcNow => tp.GetUtcNow();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes a behavior which is not expected - a bug. untriaged
Projects
None yet
Development

No branches or pull requests

1 participant