Repository navigation
perf(cache): write a set's value before its event, and an opt-in background L2 write - #266
Merged
Merged
Conversation
cosmin-staicu
requested review from
alinahornet,
cosminvlad,
litheon,
lucianaparaschivei and
razvalex
as code owners
October 21, 2026 11:02
|
🔎 Maintainer heads-up: automated triage flagged this PR as potentially material, so it may need a signed CLA in addition to the DCO sign-off. Strong signals
Other signals
This is advisory only — the bot does not decide. Please judge against the CLA criteria (material, product-critical, patent-sensitive, corporate contributor, broad commercial use). Note that thresholds can be gamed by splitting PRs, so use your judgement.
|
github-actions
Bot
added
the
needs-cla-review
cosmin-staicu
added
cla-not-required
cosmin-staicu
force-pushed
the
perf/background-write
branch
5 times, most recently
from
October 22, 2026 04:09
972d033 to
ec1666d
Compare
…ground L2 write A single-key set published its event before writing the value, so a peer that woke on the event and read L2 in between kept the old value for its whole local lifetime. Single-key, hash and rehydrate writes now write first and publish second; a batch set publishes every key the inner tier holds. CachePolicy.BackgroundDistributedWrite runs the L2 write of a single-key set and of a GetOrAdd fill, and a set's event after it, off the caller's path, on both the single-key and the hash cache. The value stays local at the disconnected cap until its write lands. Writes of one key reach L2 in issue order: awaited writes, removes, TryAdd and refresh wait for the key's background writes, and a background write waits for every earlier write of its key. A local write that a newer one overtook, and a fill from an L2 read begun before a write of the key or while one is in flight, leave the newer local value in place. MaxBackgroundWrites bounds them, falling back to an awaited write; Dispose and DisposeAsync drain them once, for BackgroundWriteDrainTimeout (infinite waits for all), and count the ones cancelled. Batches, fills under the distributed lock and the IDistributedCache adapter stay awaited. The notify filter of a forced reconnect now keys on the connector's current multiplexer, so a reconnect during a subscribe neither keeps a retired connection's failures nor ignores the new one's. Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>
cosmin-staicu
force-pushed
the
perf/background-write
branch
from
October 22, 2026 06:14
ec1666d to
eb72d20
Compare
|
alinahornet
approved these changes
Oct 22, 2026
cosmin-staicu
added a commit
that referenced
this pull request
Oct 23, 2026
…h cache fixes (#267) ## RemoveManyAsync and SetManyAsync On `ICache`, `ICache<T>`, `IHashCache` and `IHashCache<T>`, served by `ISpanKeyCache*` and `ISpanKeyHashCache*`, which gain these members and `RemoveFieldsAsync` with default bodies, so an implementation built against 3.0.0 still loads. Any other implementation, a mock included, falls back to chunked single-key calls. They take an `IEnumerable<CacheKey>` and return the keys that failed; an empty list means all succeeded. - **Any slots.** One command per key, so a cluster never answers `CROSSSLOT`. - **Chunks.** `InMemoryRedisCacheOptions.BatchChunkSize` (500) keys per chunk, `BatchConcurrentChunks` (2) in flight. A chunk's writes or removes start together, then that chunk's events, so a chunk costs about two round trips and memory stays with the chunk. A failure never stops the rest. The caller's cancellation throws: no command is sent after it, in the chunk or a later one, and what ran stays done. - **Order per key.** Remove drops the local entry, unlinks, then announces, even if the unlink failed (as `RemoveAsync` does). Set writes, then announces; a failed write gets no event. Both take their place in the key's write order from #266, so a background write of the key is never overtaken, and every local write is tracked so an earlier read cannot put an old value back. - **A key named twice.** Within a chunk the key ends as its writes in order would leave it: the last write wins, and hash merges combine in order. Across chunks, which run two at a time, the order is not set. - **Topic-wide event.** Above the opt-in `BatchTopicInvalidationThreshold`, one `TopicInvalidated` event replaces the per-key events. It drops the local tier of every cache sharing the topic, since events name no cache. A node on an older version ignores it, so enable it only once every node runs this version. The size is known through `Cache<T>` and its key strategy too. - **Background writes.** The calls stay awaited under `BackgroundDistributedWrite` and wait only for keys with a pending background write. The notify doorbell now keeps one `PUBLISH` in flight and coalesces the rings after it into one more. One `PUBLISH` per event overflowed a subscriber's `client-output-buffer-limit pubsub` at 100k events, and Redis closed the subscription. A `PUBLISH` stalled on Redis holds the rings behind it for at most one second. `RemoveAsync(CacheKey[])` and the batch `SetAsync` are unchanged: the batch set is one atomic transaction, which the new path would give up. ## Hash cache - **`HashReplace` keeps the local hash whole.** A `HashReplace` write left only the written fields in the local tier, so the writing node lost the others until the entry expired. It now merges into the held local hash, as one locked step so two concurrent merges keep both sets of fields; with none held, it stores nothing and the next read fetches the whole hash. - **`GetCacheEntryAsync` in one round trip.** An `EXISTS` before the read is gone, as the docs already said. - **`RemoveFieldsAsync`** removes single fields (`HDEL`). Peers drop the whole local entry through an ordinary remove event, so older nodes need no new format. The event goes out whenever the delete was sent, since some chunks may have landed even if it failed. - **Large hashes.** A write of more than `RedisCacheOptions.HashFieldBatchSize` (1000) fields goes in bounded `HSET` chunks. A `KeyReplace` write stays atomic and keeps its TTL through a same-slot staging key and `RENAME`; a hash with no expiry is renamed and persisted in one transaction, so it never lands with the staging key's lifetime; a `HashReplace` merge is not atomic across chunks (documented). Field reads are split too, and a whole-hash read pages by `HSCAN` above ten batches on Redis 7+. ## Measured (local Docker) | | N=1 | N=10 | N=225 | |---|---|---|---| | `RemoveManyAsync`, single node | 0.9 ms | 1.1 ms | 10.2 ms | | `RemoveManyAsync`, cluster | 0.9 ms | 1.35 ms | 10.8 ms | | `SetManyAsync`, cluster | 0.9 ms | 1.3 ms | 11.3 ms | | `RemoveAsync` in a loop | 0.9 ms | 8.9 ms | 217 ms | - 100k keys: about 23–30k keys/s for both calls on single node and cluster, no failures. - A 1M-field hash on a single node: writes at 122k fields/s; the longest server command in `SLOWLOG` was about 3.5 ms. ## Tests - Unit: chunk boundaries, a failing middle chunk, cancellation between and within chunks, the in-flight bound, pending background writes, the topic-wide switch and its failure, the size through `Cache<T>`'s key strategy, a key named twice, an older node's decode, the fallback without the capability; the `HashReplace` merge, two merges at once; `RemoveFieldsAsync`; large hashes; the doorbell coalescing and its one-second bound. - Redis integration on single node and cluster: 130 keys across slots, the topic-wide drop on a peer, hash set and remove, about two round trips for 300 keys under delayed writes, 20k keys in chunks, the merge, `RemoveFieldsAsync`, 5000-field hashes, a chunked replace of a hash with no expiry. - Three cluster-only integration failures also happen on `main` against a cluster; they are standalone-only tests. ## Follow-up - A chunked hash write starts every chunk at once, each a copy of its fields: a 1M-field hash at `HashFieldBatchSize` 1000 puts about 1000 commands on the wire before any is awaited and doubles its payload in memory. A window of chunks in flight, as `BatchConcurrentChunks` gives the batch calls, would bound both; it needs its own benchmark, so it is left for a separate change. Signed-off-by: Cosmin Staicu <cosmin.staicu@uipath.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two changes to the write path of
MultilayerCacheandMultilayerHashCache.Write before publishing (default)
A single-key set published its event (
XADD) before writing the value. A peer that woke on the event, dropped its local entry and read L2 in that window cached the old value again, for its whole local lifetime. Single-key sets, hash sets and the rehydrate writes now write first and publish second, as the batch set already did. A batch set now publishes every key the inner tier holds, even when only the local write failed.Opt-in background L2 write
CachePolicy.BackgroundDistributedWritetakes the L2 write of a single-keySet*and of aGetOrAdd*fill off the caller's path, together with the set's event after it, on both caches. Awaited round trips: a set goes from 2 to 0, a fill's write from 1 to 0.What it keeps:
LocalExpirationDisconnected) until its L2 write lands, then gets the normal local lifetime. The promotion skips an entry a newer write, a remove or an invalidation replaced.cache.background_write.failed.InMemoryRedisCacheOptions.MaxBackgroundWrites(default 1024) bounds the writes in flight; past it a write runs awaited instead of being dropped.DisposeandDisposeAsyncdrain it forBackgroundWriteDrainTimeout(default 5 s).What stays awaited: removes, refresh, batch operations, a fill under the distributed lock (the lock is released when the run ends, so a peer must find the value), and the
IDistributedCacheadapter, whose buffer writes use the caller's memory. The adapter forces the flag off for any policy it is given.What a caller gives up with the flag on: after
SetAsyncreturns, other nodes can still read the old value until the write lands (about one round trip), and atrueresult means the local write succeeded, not the L2 one.Tests