Skip to content

Implement output-cache store using redis #48450

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

Merged
merged 31 commits into from
Jun 21, 2023
Merged

Implement output-cache store using redis #48450

merged 31 commits into from
Jun 21, 2023

Conversation

mgravell
Copy link
Member

@mgravell mgravell commented May 26, 2023

Implement output-cache store using redis

Output-cache is currently implemented via an in-process mechanism; here we propose a new redis-based output-cache back-end implementation

Note that CI is disabled; we will address that separately

Design overview

Packaging: included in Microsoft.Extensions.Caching.StackExchangeRedis OOB package, in builds > net7.0 (because IOutputCacheStore is net7)

Redis compliance

The implementation targets down-level redis, but will also detect and use redis 6.2 features when available (specifically: ZADD GT)

The implementation is cluster-aware and does not attempt multi-key operations due to cross-slot concerns; there are no benefits of "cluster hash-tags" in this scenario.

API changes

- new `services.AddStackExchangeRedisOutputCache(...)` registration method - new DIM additions to `IOutputCacheStore` to remove some array usage scenarios - new `IOutputCacheBufferWriterStore : IOutputCacheStore` (naming TBD) to optionally allow better "get" mechanism for out-of-process scenarios (buffer lifetimes, etc)

edit above revised due to API review:

  • new services.AddStackExchangeRedisOutputCache(...) registration method
  • new IOutputCacheBufferStore : IOutputCacheStore to optionally allow better fetch and store mechanism for out-of-process scenarios (buffer lifetimes, etc)

Feature details

The core of this feature works via redis "string" payloads, with redis handling expiration etc as usual. We prepend (efficiently) a key-prefix including the InstanceName (logical partitioning) and a "__MSOCV_" prefix, so an output-cache key "foo" in the instance name "bar" will be stored in redis at key "bar__MSOCV_foo".

Tag-based eviction complicates things, as redis does not have an inbuilt mechanism for this, so we maintain some additional data structures:

  • {InstanceName}__MSOCT is a sorted-set of all known tags with the score the maximum logical expiration time (unix milliseconds) of all keys associated with that tag
  • {InstanceName}__MSOCT_{sometag} is a sorted-set for tag "sometag" which contains the output-cache keys (not including any of the prefixing) associated with that tag, with the score the logical expiration time (unix milliseconds)

This means that to evict all output-cache entries by tag, we simply enumerate {InstanceName}__MSOCT_{sometag}, removing all the associated keys (and removing them from the sorted set)

Because the primary output-cache payload expiration is handled by redis itself, but sorted-sets do not support per-entry expiration, we will (after some time) be left with unnecessary keys in the multiple {InstanceName}__MSOCT_{sometag} sorted-sets, so to mitigate this we also have a periodic GC operation that scans {InstanceName}__MSOCT; for every element, it does a score-based cull of {InstanceName}__MSOCT_{sometag} (ZREMRANGEBYSCORE) to remove all keys that have expired, and similarly a score-based cull of {InstanceName}__MSOCT to remove tags that no longer have any in-date values.

To prevent multiple GC runs happing concurrently, the existence of key {InstanceName}__MSOCTGC is used as a marker to indicate an in-progress GC run; no other GC runs will succeed while the key exists.

@ghost ghost added the area-runtime label May 26, 2023
@Misiu
Copy link

Misiu commented May 30, 2023

Take a look at https://github.com/thepirat000/CachingFramework.Redis as it supports tags. I use it in multiple projects and tagging works great. Not sure if this is the best approach when it comes to performance, but it works fine for my use cases.
Linking it as a reference.

@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares and removed area-runtime labels Jun 2, 2023
@mgravell mgravell marked this pull request as ready for review June 16, 2023 14:55
@mgravell mgravell enabled auto-merge (squash) June 20, 2023 16:06
@mgravell mgravell merged commit c7d58cd into main Jun 21, 2023
@mgravell mgravell deleted the marc/ocredis branch June 21, 2023 21:52
@ghost ghost added this to the 8.0-preview6 milestone Jun 21, 2023
@adityamandaleeka
Copy link
Member

@mgravell Please make sure you take a look at @sebastienros's comments. This got auto-merged upon sign-off (which is fine since we want to get it in for preview 6 and none of the comments look like showstoppers).

Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

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

LGTM for an initial look!

Trying to clone the branch locally to run the skipped tests. Do we have an issue tracking the work to light up Redis in CI? I know you've been chasing that thread for a bit but it would be helpful to have a place to track the latest developments.

/// </summary>
/// <param name="optionsAccessor">The configuration options.</param>
/// <param name="logger">The logger.</param>
internal RedisOutputCacheStore(IOptions<RedisCacheOptions> optionsAccessor, ILogger logger)
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 intentional that we don't support configuring a logger in the API? Do we plan on doing this eventually?

Copy link
Member Author

Choose a reason for hiding this comment

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

what would you propose here? ILogger<IOutputCacheStore> instead of ILogger?

mgravell added a commit that referenced this pull request Jun 28, 2023
@adityamandaleeka adityamandaleeka added the blog-candidate Consider mentioning this in the release blog post label Jun 28, 2023
@ghost
Copy link

ghost commented Jun 28, 2023

@mgravell, this change will be considered for inclusion in the blog post for the release it'll ship in. Nice work!

Please ensure that the original comment in this thread contains a clear explanation of what the change does, why it's important (what problem does it solve?), and, if relevant, include things like code samples and/or performance numbers.

This content may not be exactly what goes into the blog post, but it will help the team putting together the announcement.

Thanks!

mgravell added a commit that referenced this pull request Jul 21, 2023
mgravell added a commit that referenced this pull request Jul 24, 2023
mgravell added a commit that referenced this pull request Jul 27, 2023
- unify OutputCacheEntry and FormatterEntry
- leased buffers for headers, tags, etc (dispose on way out)
- use ReadOnlySequence<byte> instead of List<byte[]> with recyclable segments
- avoid copying the payload data once fectched
- serialization tweak: use common headers (not yet listed)

finish porting tests; all good

migrate writes to IBufferWriter<byte>, using recyclable array buffer writer (similar to MemoryStream, but: faster)

use pooled buffers when buffering output-cache payloads RROSS should be able to recycle buffers as needed

implement header name/value lookup buffer

add bench project

note memory overhead in bench

full bench suite

add benchmark result

- don't store tags in the cache payload
- don't store segment details in the cache payload

whitespace

include test that uses body-writer rather than stream

tidy up benchmarks

remove pipe impl; we don't need it (proven in bench)

add a few more known headers; don't store request-id

don't write empty segments

fix buffer cleanup paths

revert changes to shared SegmentWriteStream

update numbers

use fixed size stackalloc

Update src/Middleware/OutputCaching/src/OutputCacheEntryFormatter.cs

Co-authored-by: Günther Foidl <[email protected]>

- remove global SkipLocalsInit
- add accessibility on all new members
- in length checks, use - over + to avoid overflow

nits

relocate perf to microbenchmarks

use standard project structure for micro-benchmarks

use "actual" instead of "bytes" to avoid a "pop"/"ld" in the "release" case

avoid a memcopy by writing directly to the buffer bytes (and increasing DRY)

Update src/Middleware/OutputCaching/src/OutputCacheEntry.cs

Co-authored-by: Brennan <[email protected]>

Update src/Middleware/OutputCaching/test/OutputCacheEntryFormatterTests.cs

Co-authored-by: Brennan <[email protected]>

fix PR nits from #48450

fix sln (dead project)

Update src/Middleware/OutputCaching/src/OutputCacheEntry.cs

Co-authored-by: Brennan <[email protected]>

DRY nit

moar nits

use leased buffer for tags when calling SetAsync

Update src/Middleware/OutputCaching/src/OutputCacheEntryFormatter.cs

Co-authored-by: Brennan <[email protected]>

use [LoggerMessage] for logging

merge submodule delete

bump

optimize Output Cache; no API changes yet - all internal:
- unify OutputCacheEntry and FormatterEntry
- leased buffers for headers, tags, etc (dispose on way out)
- use ReadOnlySequence<byte> instead of List<byte[]> with recyclable segments
- avoid copying the payload data once fectched
- serialization tweak: use common headers (not yet listed)

sln fix

dammit
mgravell added a commit that referenced this pull request Aug 3, 2023
* optimize Output Cache; no API changes yet - all internal:
- unify OutputCacheEntry and FormatterEntry
- leased buffers for headers, tags, etc (dispose on way out)
- use ReadOnlySequence<byte> instead of List<byte[]> with recyclable segments
- avoid copying the payload data once fectched
- serialization tweak: use common headers (not yet listed)

finish porting tests; all good

migrate writes to IBufferWriter<byte>, using recyclable array buffer writer (similar to MemoryStream, but: faster)

use pooled buffers when buffering output-cache payloads RROSS should be able to recycle buffers as needed

implement header name/value lookup buffer

add bench project

note memory overhead in bench

full bench suite

add benchmark result

- don't store tags in the cache payload
- don't store segment details in the cache payload

whitespace

include test that uses body-writer rather than stream

tidy up benchmarks

remove pipe impl; we don't need it (proven in bench)

add a few more known headers; don't store request-id

don't write empty segments

fix buffer cleanup paths

revert changes to shared SegmentWriteStream

update numbers

use fixed size stackalloc

Update src/Middleware/OutputCaching/src/OutputCacheEntryFormatter.cs

Co-authored-by: Günther Foidl <[email protected]>

- remove global SkipLocalsInit
- add accessibility on all new members
- in length checks, use - over + to avoid overflow

nits

relocate perf to microbenchmarks

use standard project structure for micro-benchmarks

use "actual" instead of "bytes" to avoid a "pop"/"ld" in the "release" case

avoid a memcopy by writing directly to the buffer bytes (and increasing DRY)

Update src/Middleware/OutputCaching/src/OutputCacheEntry.cs

Co-authored-by: Brennan <[email protected]>

Update src/Middleware/OutputCaching/test/OutputCacheEntryFormatterTests.cs

Co-authored-by: Brennan <[email protected]>

fix PR nits from #48450

fix sln (dead project)

Update src/Middleware/OutputCaching/src/OutputCacheEntry.cs

Co-authored-by: Brennan <[email protected]>

DRY nit

moar nits

use leased buffer for tags when calling SetAsync

Update src/Middleware/OutputCaching/src/OutputCacheEntryFormatter.cs

Co-authored-by: Brennan <[email protected]>

use [LoggerMessage] for logging

merge submodule delete

bump

optimize Output Cache; no API changes yet - all internal:
- unify OutputCacheEntry and FormatterEntry
- leased buffers for headers, tags, etc (dispose on way out)
- use ReadOnlySequence<byte> instead of List<byte[]> with recyclable segments
- avoid copying the payload data once fectched
- serialization tweak: use common headers (not yet listed)

sln fix

dammit

* use `_field` naming in `FormatterBinaryReader.cs`

* move log methods to dedicated file

* simplify cached response cleanup

* add comment re buffer size
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares blog-candidate Consider mentioning this in the release blog post feature-output-caching
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants