diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..896d508 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,162 @@ +# Changelog + +All notable changes to HyperCache are recorded here. The format follows +[Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and the project +adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [Unreleased] + +## [2.0.0] — 2026-05-04 + +A modernization release. The headline themes: + +- Eviction is now sharded by default for concurrency-friendly throughput. +- The distributed-memory backend (`DistMemory`) gained body limits, TLS, + bearer-token auth, lifecycle-context cancellation, and surfaced + listener errors. +- A typed wrapper (`Typed[T, V]`) is available for compile-time + type-safe access without the caller-side type assertions of the + untyped API. +- The legacy `pkg/cache` v1 store and the `longbridgeapp/assert` test + dependency are gone. + +The full course-correction plan (Phase 0 baseline → Phase 6 file split, +plus Phase 5a–5e DistMemory hardening) is in commit history. The two +RFCs that informed the design decisions live under [docs/rfcs/](docs/rfcs/). + +### Breaking changes + +- **`pkg/cache` v1 removed.** All callers must use `pkg/cache/v2`. +- **`longbridgeapp/assert` test dependency removed.** Tests now use + `stretchr/testify/require`. Internal test code only — no impact on + library consumers, but downstream contributors authoring tests + against this codebase must use `require`. +- **`sentinel.ErrMgmtHTTPShutdownTimeout` removed.** + `ManagementHTTPServer.Shutdown` now calls `app.ShutdownWithContext` + and returns the underlying ctx error directly. Callers comparing + against the removed sentinel must switch to `errors.Is(err, + context.DeadlineExceeded)` or equivalent. +- **Sharded eviction is default-on (32 shards).** Items no longer + evict in strict global LRU/LFU order — the algorithm operates + independently within each shard. Total capacity is honored within + ±32 (one slot of slack per shard). Use `WithEvictionShardCount(1)` + to restore strict-global ordering at the cost of single-mutex + contention. +- **`hypercache.go` decomposed into 6 files** (`hypercache.go`, + `hypercache_io.go`, `hypercache_eviction.go`, + `hypercache_expiration.go`, `hypercache_dist.go`, + `hypercache_construct.go`). No public API change; third-party + patches against line numbers in the prior single-file layout will + not apply. +- **`ManagementHTTPServer` constructor order fix.** + `WithMgmtReadTimeout` and `WithMgmtWriteTimeout` previously mutated + struct fields *after* `fiber.New` had locked in the defaults — the + options were silent no-ops. Construction order is now correct, so + any code relying on the silent no-op (e.g., setting absurd values + knowing they would be ignored) will see those values take effect. + +### Performance + +Measurements on Apple M4 Pro, `go test -bench`, `count=5`, benchstat. +Full release snapshot captured in [bench-v2.0.0.txt](bench-v2.0.0.txt). + +- **Per-shard atomic `Count`.** `BenchmarkConcurrentMap_Count`: + 53 → ~10 ns/op. `_CountParallel`: 1181 → ~13 ns/op. Eliminates the + lock-storm that previously serialized on a single mutex during + eviction-loop count checks. +- **Sharded eviction algorithm** (`pkg/eviction/sharded.go`). + Replaces the global eviction-algorithm mutex with 32 per-shard + mutexes routed by the same hash `ConcurrentMap` uses, so a key's + data shard and eviction shard align (cache-locality on Set). +- **`iter.Seq2` migration** replacing channel-based `IterBuffered`. + `BenchmarkConcurrentMap_All` (renamed from `_IterBuffered`): + 757µs → 26.5µs/op (-96.51%). Bytes/op: 1.73 MiB → 0 B/op. + Allocs/op: 230 → 0. Eliminated 32 goroutines + 32 channels per + iteration. +- **xxhash consolidation** (`pkg/cache/v2/hash.go`). Replaced inlined + FNV-1a with `xxhash.Sum64String` folded to 32 bits. + `BenchmarkConcurrentMap_GetShard`: 10.07 → 3.46 ns/op (-65.63%). +- **Sharded item-aware eviction was tried and rejected** per + [RFC 0001](docs/rfcs/0001-backend-owned-eviction.md). The + hypothesis (duplicate-map overhead is the bottleneck) was + falsified — sharded contention dominates. Code removed; lessons + preserved in the RFC for future contributors. + +### Features + +- **`hypercache.Typed[T, V]` wrapper** for compile-time type-safe + cache access. Wraps an existing `HyperCache[T]`; multiple `Typed` + views can share one underlying cache over disjoint keyspaces. + Includes `Set`, `Get`, `GetTyped` (explicit `ErrTypeMismatch`), + `GetWithInfo`, `GetOrSet`, `GetMultiple`, `Remove`, `Clear`. See + [hypercache_typed.go](hypercache_typed.go) and + [RFC 0002 Phase 1](docs/rfcs/0002-generic-item-typing.md). Phase 2 + (deep `Item[V]` generics) is v3 territory, conditional on adoption + signal. +- **`WithDistHTTPLimits(DistHTTPLimits)` option** for the dist + transport: server `BodyLimit` / `ReadTimeout` / `WriteTimeout` / + `IdleTimeout` / `Concurrency`, plus client `ResponseLimit` / + `ClientTimeout`. Defaults: 16 MiB request/response body cap, 5 s + read/write/client timeout, 60 s idle, fiber's 256 KiB concurrency + cap. Partial overrides honored — zero fields inherit defaults. +- **`WithDistHTTPAuth(DistHTTPAuth)` option** for bearer-token auth on + `/internal/*` and `/health` (`Token` for the common case; + `ServerVerify`/`ClientSign` hooks for JWT, mTLS-derived identity, + HMAC, etc.). Constant-time token compare on the server side. The + auto-created HTTP client signs every outgoing request with the + same token. Mismatched-token peers are rejected with HTTP 401 + (`sentinel.ErrUnauthorized`). +- **TLS support** via `DistHTTPLimits.TLSConfig`. The server wraps + its listener with `tls.NewListener`; the auto-created HTTP client + attaches the same `*tls.Config` to its `Transport.TLSClientConfig` + with ALPN forced to `http/1.1` (fiber/fasthttp doesn't speak h2). + Same `*tls.Config` configures both sides — operators applying it + consistently across the cluster get encrypted intra-cluster + traffic out of the box. Plaintext peers handshake-fail. +- **Dist server lifecycle context** — `DistMemory.LifecycleContext()` + exposes a context derived from the constructor's that is canceled + on `Stop()`. Replaces the prior pattern where handlers captured + the constructor's `context.Background()` and never observed + cancellation. In-flight handlers and replica forwards see `Done()` + the moment `Stop` is called. +- **`LastServeError()` accessor** on both `distHTTPServer` and + `ManagementHTTPServer`. Replaces the prior `_ = serveErr` pattern + that silently swallowed listener-loop crashes — operators can now + surface the failure to logs/alerts. +- **`Stop()` goroutine-leak fix.** Both `distHTTPServer.stop` and + `ManagementHTTPServer.Shutdown` now call + `app.ShutdownWithContext(ctx)` directly instead of wrapping + `app.Shutdown()` in a goroutine and racing it against ctx done + (which leaked the goroutine when ctx fired first). +- **New sentinels:** `sentinel.ErrTypeMismatch`, + `sentinel.ErrUnauthorized`. + +### Internal + +Worth surfacing for contributors: + +- **v2 module layout** is the file split listed under "Breaking + changes" above — readability win, no API change. +- **Test helpers** introduced under `tests/`: + `tests/dist_cluster_helper.go::SetupInProcessCluster[RF]`, + `tests/merkle_node_helper.go`, + `pkg/backend/dist_memory_test_helpers.go::EnableHTTPForTest` + (build tag `test`). +- **Lint discipline:** 35 `nolint` directives total across the repo, + each with a one-line justification. golangci-lint v2.12.1 runs + clean with `--build-tags test`. + +### Removed + +- `pkg/cache` v1 (see "Breaking changes"). +- `longbridgeapp/assert` test dependency (see "Breaking changes"). +- `sentinel.ErrMgmtHTTPShutdownTimeout` (see "Breaking changes"). +- Experimental `WithItemAwareEviction` option / `IAlgorithmItemAware` + interface / `LRUItemAware` / `ShardedItemAware` types — landed + briefly during the RFC 0001 spike, then torn out per the RFC's + own discipline when the perf gate failed. The + [RFC document](docs/rfcs/0001-backend-owned-eviction.md) preserves + the measurement and the lessons. + +[Unreleased]: https://github.com/hyp3rd/hypercache/compare/v2.0.0...HEAD +[2.0.0]: https://github.com/hyp3rd/hypercache/releases/tag/v2.0.0 diff --git a/README.md b/README.md index 47be40c..56e4899 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,12 @@ Available algorithm names you can pass to `WithEvictionAlgorithm`: Note: ARC is experimental and isn’t included in the default registry. If you choose to use it, register it manually or enable it explicitly in your build. +#### Sharded eviction (default since v2.0.0) + +The configured algorithm is wrapped by a 32-shard router (`pkg/eviction/sharded.go`) that uses the same key hash as `ConcurrentMap` — so a key's data shard and eviction shard line up. This eliminates the global mutex contention single-instance algorithms (LRU/LFU/Clock/CAWOLFU) suffer from. Total capacity is honored within ±32 (one slot of slack per shard), and items evict per-shard rather than in strict global LRU/LFU order. + +Use `WithEvictionShardCount(1)` to disable sharding when you need strict-global ordering at the cost of single-mutex contention. Pass any other positive power of two to tune (e.g. `WithEvictionShardCount(64)`). + ## API `NewInMemoryWithDefaults(ctx, capacity)` is the quickest way to start: @@ -177,6 +183,7 @@ if err != nil { | `WithExpirationTriggerBuffer` | Buffer size for coalesced expiration trigger channel. | | `WithExpirationTriggerDebounce` | Drop rapid-fire triggers within a window. | | `WithEvictionAlgorithm` | Select eviction algorithm (lru, lfu, clock, cawolfu, arc*). | +| `WithEvictionShardCount` | Number of eviction-algorithm shards (default 32; 1 disables sharding). | | `WithMaxEvictionCount` | Cap number of items evicted per cycle. | | `WithMaxCacheSize` | Max cumulative serialized item size (bytes). | | `WithStatsCollector` | Choose stats collector implementation. | @@ -191,9 +198,33 @@ if err != nil { | `WithDistSeeds` | (DistMemory) Static seed addresses to pre-populate membership. | | `WithDistTombstoneTTL` | (DistMemory) Retain delete tombstones for this duration before compaction (<=0 = infinite). | | `WithDistTombstoneSweep` | (DistMemory) Interval to run tombstone compaction (<=0 disables). | +| `WithDistHTTPLimits` | (DistMemory) Body / response / timeout / concurrency caps for the dist HTTP server + auto-client. | +| `WithDistHTTPAuth` | (DistMemory) Bearer-token auth (`Token`) plus optional `ServerVerify` / `ClientSign` hooks. | *ARC is experimental (not registered by default). +### Type-safe access (`Typed[V]`) + +The untyped `HyperCache.Get` returns `(any, bool)`, so callers must +type-assert at every call site. `hypercache.NewTyped[T, V]` wraps an +existing `*HyperCache[T]` to provide a compile-time-typed surface +without changing the underlying storage: + +```go +hc, _ := hypercache.NewInMemoryWithDefaults(ctx, 10_000) +sessions := hypercache.NewTyped[backend.InMemory, *Session](hc) + +_ = sessions.Set(ctx, "u:42", &Session{UserID: "u-42"}, time.Hour) +s, ok := sessions.Get(ctx, "u:42") // s is *Session — no type assert +``` + +Multiple `Typed[V1]`, `Typed[V2]` views can share one underlying +cache over disjoint keyspaces. Wrong-type reads return `(zero, false)` +by default (fail-soft); use `GetTyped` for an explicit +`sentinel.ErrTypeMismatch`. See +[docs/rfcs/0002-generic-item-typing.md](docs/rfcs/0002-generic-item-typing.md) +for the design and the v3 deep-generics roadmap. + ### Redis / Redis Cluster notes When using Redis or Redis Cluster, item size accounting uses the configured serializer (e.g. msgpack) to align in-memory and remote representations. Provide the serializer via backend options (`WithSerializer` / `WithClusterSerializer`). @@ -222,16 +253,49 @@ Current capabilities (implemented): - Lightweight gossip snapshot exchange (in-process only). - Rebalancing (primary change & lost ownership migrations) with batching and concurrency throttling metrics. - Latency histograms for Get/Set/Remove. +- HTTP transport hardening: bounded request/response bodies, idle-connection timeout, concurrency cap, bearer-token auth, TLS / mTLS via `*tls.Config`, lifecycle-context cancellation on `Stop`, and surfaced listener errors. See "Transport hardening" below. Limitations / not yet implemented: -- Replica-only ownership diff migrations. - Full gossip-based dynamic membership & indirect probing. - Advanced versioning (HLC / vector clocks). - Tracing spans for distributed operations. -- Security (TLS/mTLS, auth) & compression. +- Compression on the wire. - Persistence / durability (out of scope presently). +#### Transport hardening (since v2.0.0) + +The dist HTTP server and the auto-created HTTP client share a single configuration surface — apply the same option to every node in the cluster. + +```go +// 1) Limits: body caps, timeouts, concurrency, optional TLS. +limits := backend.DistHTTPLimits{ + BodyLimit: 16 * 1024 * 1024, // server inbound cap + ResponseLimit: 16 * 1024 * 1024, // client inbound cap + IdleTimeout: 60 * time.Second, + TLSConfig: tlsConfig, // non-nil enables HTTPS on both sides +} + +// 2) Auth: a shared bearer token covers most clusters; ServerVerify / +// ClientSign hooks are escape hatches for JWT, mTLS-derived +// identity, HMAC, etc. +auth := backend.DistHTTPAuth{Token: "shared-cluster-secret"} + +bi, _ := backend.NewDistMemory(ctx, + backend.WithDistNode("nodeA", "127.0.0.1:7001"), + backend.WithDistReplication(3), + backend.WithDistHTTPLimits(limits), + backend.WithDistHTTPAuth(auth), +) +``` + +Operational helpers: + +- `DistMemory.LifecycleContext()` — context derived from the constructor's; canceled on `Stop()`. In-flight handlers and replica forwards observe `Done()`. +- `LastServeError()` on both `distHTTPServer` and `ManagementHTTPServer` — replaces the prior silent-swallow of listener-loop crashes. + +Defaults if `WithDistHTTPLimits` is not supplied: 16 MiB body cap, 5 s read/write/client timeouts, 60 s idle, fiber's 256 KiB concurrency cap. Auth is disabled by default. + #### Rebalancing & Ownership Migration (Experimental Phase 3) The DistMemory backend includes an experimental periodic rebalancer that: @@ -283,7 +347,7 @@ Test helpers `AddPeer` and `RemovePeer` simulate join / leave events that trigge | Advanced versioning (HLC/vector) | Planned | | Client SDK (direct routing) | Planned | | Tracing spans | Planned | -| Security (TLS/auth) | Planned | +| Security (TLS/auth) | Done (since v2.0.0; see "Transport hardening") | | Compression | Planned | | Persistence | Out of scope (current phase) | | Chaos / fault injection | Planned | diff --git a/cspell.config.yaml b/cspell.config.yaml index 5520855..12dbb77 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -35,10 +35,13 @@ dictionaries: [] words: - acks - ALPN + - assertable - autosync - backpressure + - baselining - benchmarkdist - benchmem + - benchstat - benchtime - bitnami - bodyclose @@ -54,6 +57,7 @@ words: - cmap - Cmder - codacy + - codemod - containedctx - contextcheck - cpuprofile @@ -85,6 +89,7 @@ words: - Fprintln - freqs - funlen + - geomean - gerr - gitversion - GITVERSION @@ -95,6 +100,7 @@ words: - goconst - gofiber - GOFILES + - gofmt - gofumpt - goimports - golangci @@ -145,6 +151,7 @@ words: - NOVENDOR - paralleltest - Pipeliner + - pluggability - popd - Prealloc - protoc diff --git a/docs/rfcs/0001-backend-owned-eviction.md b/docs/rfcs/0001-backend-owned-eviction.md new file mode 100644 index 0000000..7370430 --- /dev/null +++ b/docs/rfcs/0001-backend-owned-eviction.md @@ -0,0 +1,311 @@ +# RFC 0001 — Backend-owned eviction order + +- **Status**: Closed — REJECTED +- **Target**: n/a (spike measured, hypothesis falsified, code removed) +- **Owners**: TBD +- **Related code**: [pkg/eviction/eviction.go](../../pkg/eviction/eviction.go), [pkg/cache/v2/item.go](../../pkg/cache/v2/item.go), [hypercache_io.go](../../hypercache_io.go) + +## Summary + +Eliminate the duplicate hash + lock + map insert that every `HyperCache.Set` / `Remove` / `Clear` performs by hosting the eviction algorithm's per-key state directly inside `cache.Item`. A side-effect closes a long-standing semantic gap: `Get` currently does not bump LRU recency, so "LRU" today is actually "least-recently-written". + +## Background + +`HyperCache.Set` performs two independent updates to two independent data structures keyed by the same string: + +```go +// hypercache_io.go (current) +err = hyperCache.backend.Set(ctx, item) // 1. hash key, lock data shard, insert into shard map +hyperCache.evictionAlgorithm.Set(key, item.Value) // 2. hash key, lock algo, insert into algo map + linked-list +``` + +Each call: + +1. Hashes the key (xxhash-sum64 → 32-bit fold; ~3.6 ns each per [bench-step3.txt](../../bench-step3.txt)). +1. Locks a shard (RWMutex; contention dominates under load). +1. Inserts into a `map[string]X` (allocates a hash bucket). +1. For LRU/LFU: rotates a doubly-linked list node. + +Step 1 is repeated on the same key — wasted work. Steps 2–3 happen on disjoint shard maps even though they cover the same logical key — also wasted work. The only step that is actually unique to the eviction layer is step 4 (the linked-list rotation). + +`Remove` has the same pattern. `Clear` lists every key, calls `algorithm.Delete(key)` per item, then clears the backend — N hashes, N algo locks for what should be one teardown. + +`Get` does *not* call `algorithm.Get(key)` today (the eviction algorithm is unaware of reads). LRU is therefore "Least Recently Written", which violates user expectations and is documented nowhere. + +### Measured cost + +From `bench-step3.txt` and earlier baselines (M4 Pro, `-race` off): + +| Operation | Hash | Lock+Map | Total ns/op | +| --------- | ---- | -------- | ----------- | +| `ConcurrentMap_GetShard` (xxhash → shard idx) | 3.6 | — | 3.6 | +| `Set` (single shard, no eviction) | — | — | ~80 | +| `Set` (with `algorithm.Set`) | — | — | ~711 (`HyperCache_SetParallel`) | + +The eviction-side overhead is roughly **8×** the storage-side cost on the hot path. Most of that is the algorithm's own mutex (LRU's `sync.RWMutex` serializes Set + moveToFront), partially mitigated by [pkg/eviction/sharded.go](../../pkg/eviction/sharded.go) but not eliminated. + +## Goals + +1. **Halve the per-op work on `Set` and `Remove`** by removing the duplicate hash + map insert. Linked-list rotation (LRU) or counter increment (LFU) remains. +1. **Make `Get` actually update recency.** Once eviction state lives inside `Item`, the existing `touchBackend` Touch path can take a `*Item` and update the algorithm pointer in O(1). +1. **Preserve algorithm pluggability** (LRU, LFU, Clock, ARC, CAWOLFU). No collapsing into one frozen impl. +1. **Preserve backend pluggability.** Redis / RedisCluster delegate eviction to the engine; the new path must be a no-op for them. + +## Non-goals + +- Replacing the eviction algorithm interface with a different abstraction (queue, generations, etc.). +- Lock-free linked lists. The linked-list rotation under a per-shard lock is a well-understood pattern; lock-free LRU is out of scope. +- Touching the distributed eviction story (DistMemory shedding via rebalancer is a separate concern; see [docs/distributed.md](../distributed.md) §Rebalancing). + +## Constraints + +- **Item is pooled** ([pkg/cache/v2/item.go:34-58](../../pkg/cache/v2/item.go#L34-L58)). Adding fields costs zero per-Set allocations as long as we reset them in `Put`. +- **Item ships across the wire** in DistMemory replication ([pkg/backend/dist_http_types.go](../../pkg/backend/dist_http_types.go)). Eviction-state pointers MUST NOT be JSON-serialized — they are local-only. +- **Sharded eviction** ([pkg/eviction/sharded.go](../../pkg/eviction/sharded.go)) currently shards algorithm state on the same hash as ConcurrentMap; if we move state into Item, we still need per-shard lists (the *list head/tail* belongs to the shard, not the Item). + +## Options + +### Option A — Embed eviction state in `cache.Item` + +Add to `Item`: + +```go +type Item struct { + Key string + Value any + // ... existing fields ... + + // evictPrev/evictNext are pointers in the per-shard LRU list. They + // are owned by the eviction algorithm; backends MUST NOT serialize + // or copy them across shards. Touch under the shard's eviction lock. + evictPrev *Item `json:"-" cbor:"-"` + evictNext *Item `json:"-" cbor:"-"` + + // accessFreq supports LFU/CAWOLFU without a separate map. + accessFreq atomic.Uint64 + + // clockBit supports the Clock algorithm. + clockBit atomic.Bool +} +``` + +The `IAlgorithm` interface gains a `*Item`-aware sibling: + +```go +type IAlgorithmItemAware interface { + SetItem(it *Item) + TouchItem(it *Item) + DeleteItem(it *Item) + Evict() (*Item, bool) +} +``` + +Algorithms operating on Item pointers maintain only a per-shard head/tail pointer pair (LRU) or a freq histogram (LFU). The `map[string]*lruCacheItem` inside today's LRU disappears — Item itself IS the list node. + +`HyperCache.Set` becomes: + +```go +err = hyperCache.backend.Set(ctx, item) // 1 hash, 1 lock, 1 insert +hyperCache.evictionAlgorithm.SetItem(item) // 0 hashes, 1 lock, 0 inserts (just list rotate) +``` + +The two locks are still distinct (data shard lock vs eviction shard lock) — *unless* we co-locate them, which is Option A2 below. + +#### Option A2 (extension) — Co-locate locks + +Make `ConcurrentMapShard` own both the storage map and the LRU head/tail: + +```go +type ConcurrentMapShard struct { + sync.RWMutex + items map[string]*Item + count atomic.Int64 + + // Eviction state (algorithm-specific; one of these is non-nil). + lruHead, lruTail *Item // for LRU + lfuFreqs []*Item // for LFU (frequency buckets) + clockHand *Item // for Clock + // ... +} +``` + +Set holds the shard write lock once, updates the items map AND rotates the LRU list under the same lock. **One hash, one lock, atomic in shard scope.** + +This crosses the abstraction boundary between `pkg/cache/v2` and `pkg/eviction` more aggressively than A. The eviction package becomes a set of helper functions operating on `ConcurrentMapShard` instead of self-contained types. + +### Option B — Composite eviction-aware backend per algorithm + +Eliminate `IAlgorithm` entirely. Each algorithm gets its own backend type: + +```go +type LRUInMemory struct { /* storage + LRU baked in */ } +type LFUInMemory struct { /* storage + LFU baked in */ } +``` + +**Pros**: simplest mental model, zero abstraction cost. + +**Cons**: explodes the backend matrix (5 algorithms × 4 backends = 20 types). Doesn't model real-world need (users rarely switch algorithms; they pick one and stick). + +### Option C — Status quo + share-pre-computed-hash + +Compute the shard index once in `HyperCache.Set`, pass it to both `backend.Set` and `algorithm.Set`. Saves one xxhash call (~3.6 ns) but not the lock or the map insert. + +**Pros**: tiny diff, no API change. + +**Cons**: marginal gain (~3.6 ns / ~711 ns = 0.5%). Doesn't fix the LRW-vs-LRU semantic gap. + +## Recommendation + +**Option A**, with **A2 as a follow-up** if A's measurements justify it. + +- **Why not A2 directly?** Crossing the cache/eviction package boundary is a bigger refactor; A is the smaller verifiable step. If A delivers (say) 30% of the achievable win, A2 buys the remaining 70%. Land A first, measure, decide on A2 separately. +- **Why not B?** The 4 × 5 explosion is real cost. Users *do* swap algorithms (LRU → CAWOLFU is the recommended upgrade for hot-key workloads); the abstraction earns its keep. +- **Why not C?** The semantic gap (LRW vs LRU) is a correctness issue, not a perf one. C doesn't address it. + +## Implementation plan + +1. **Spike LRU.** Add `evictPrev/evictNext *Item` to `cache.Item`; add `IAlgorithmItemAware`; rewrite `LRU` to operate on `*Item`. Behind a feature option (`WithItemAwareEviction(true)`). Leave the legacy path intact. +1. **Bench.** Compare `BenchmarkHyperCache_SetParallel` baseline vs. spike. If under 30% improvement, stop and reconsider — there's some other bottleneck and Option A's premise is wrong. +1. **Migrate the 4 remaining algorithms.** Mechanical port; each algorithm replaces its `map[string]*algoNode` with direct `*Item` pointer use. +1. **Wire `Get` to call `TouchItem`** so LRU semantically becomes "least recently used", not just written. Update doc + CHANGELOG to call out the behavior fix. +1. **Default `WithItemAwareEviction(true)`** in v2.x; gate legacy via an explicit opt-out for one release. +1. **Remove legacy `IAlgorithm.Set/Get/Delete`** in v3. + +## Migration + +- **For library users on v2.x**: zero changes. The new path is opt-in; default stays untyped. Bench-aware users opt-in for the win. +- **For library users on v3**: `IAlgorithm` is gone. Custom algorithm authors port to `IAlgorithmItemAware` (small mechanical change — main difference is operating on `*Item` instead of `(key, any)`). + +## Risks + +| Risk | Likelihood | Mitigation | +| ---- | ---------- | ---------- | +| Item pointer fields confuse JSON serialization (DistMemory replication) | High | Tag fields `json:"-" cbor:"-"`; assert in unit tests that wire format excludes them. | +| Hidden assumption that Item is value-copyable (e.g., `cloned := *it`) | Medium | Audit all `*it` deref + reassign sites. Pointer fields stay valid across copies *for reads*, but lifecycle is owned by the algorithm — copies must not be inserted into the LRU list. | +| Pool reuse leaves stale `evictPrev/Next` pointers | Medium | `ItemPoolManager.Put` already does `*it = Item{}`; the zeroing covers the new fields. Verify with a reuse-after-evict test. | +| Algorithm-specific fields (clockBit) waste memory when a different algorithm is configured | Low | Layout: 2 pointers + atomic.Bool + atomic.Uint64 = ~24 bytes added. Already pooled; no per-Set alloc. The waste is per-Item-lifetime, not per-op. | +| Sharded eviction interaction breaks | Low | Option A keeps per-shard list heads/tails; Sharded just routes `SetItem` to the right shard. Same routing logic as today. | + +## Open questions + +- **Cross-shard moves under DistMemory rebalance**: when an Item moves shards (rebalancer), its `evictPrev/Next` pointers reference the *old* shard's list. The receiving side must reset them and link into the new shard's list. Rebalancer code in [pkg/backend/dist_memory.go](../../pkg/backend/dist_memory.go) needs a hook here. Probably trivial: rebalance-receive sets `it.evictPrev = nil; it.evictNext = nil` before insert; algorithm's SetItem links it normally. +- **Eviction algorithm sharding interaction**: with Item-embedded state, do we still need [pkg/eviction/sharded.go](../../pkg/eviction/sharded.go) at all? The per-shard list head/tail handles parallelism. Possibly the Sharded wrapper becomes a no-op once A lands. Decide after measurements. +- **ARC's ghost lists**: ARC tracks evicted keys in B1/B2 ghost lists. These hold *keys* (no Item), so they can stay as today's `map[string]struct{}`. ARC needs minor surgery, not a rewrite. + +## Decision criteria + +Land A if: + +- `BenchmarkHyperCache_SetParallel` improves ≥ 30% vs current. +- `BenchmarkHyperCache_GetParallel` improves materially (Get now touches LRU; expect 5-15% regression because of the new write — acceptable trade for correctness). +- All eviction-contract tests pass with the new path. +- `RUN_INTEGRATION_TEST=yes go test -race -count=10 -shuffle=on -timeout=20m ./...` is green. + +Reject A and revisit if any criterion fails. + +## Spike outcome — REJECTED + +Implemented as `WithItemAwareEviction(true)` opt-in: + +- New unexported fields `evictPrev`, `evictNext`, `inEvictList` on `cache.Item` +- `IAlgorithmItemAware` interface in [pkg/eviction/eviction.go](../../pkg/eviction/eviction.go) +- `LRUItemAware` ([pkg/eviction/lru_item_aware.go](../../pkg/eviction/lru_item_aware.go)) and `ShardedItemAware` ([pkg/eviction/sharded_item_aware.go](../../pkg/eviction/sharded_item_aware.go)) +- HyperCache hot paths route via `recordSet` / `recordTouch` / `recordDelete` / `evictNext` + +Benchmark vs legacy (sharded both sides; M4 Pro; `go test -bench -count=5`; benchstat): + +```text + │ legacy │ item-aware │ + │ sec/op │ sec/op vs base │ +HyperCache_SetParallel-14 205.5n ¹ 182.0n ¹ -11.44% +HyperCache_GetParallel-14 91.03n ¹ 139.60n ¹ +53.36% +geomean 136.8n 159.4n +16.54% +``` + +**Both gates fail:** + +- Set: -11.44% gain vs ≥30% target. +- Get: +53.36% regression vs 5-15% budget. + +Geomean is +16.54% slower overall. + +### Why the premise was wrong + +The RFC's hypothesis: "the duplicate hash + map lookup in the legacy +path is the dominant cost; eliminating it via embedded Item state will +deliver ≥30% Set improvement." The data falsifies it. + +- The duplicate map savings are **real but small** (~12% Set). + Sharded contention reduction is what already dominates the legacy + path's perf, not the second hash/map. +- The Get regression (+53%) is the **honest cost of correct LRU + semantics**. Legacy's "LRU is really LRW" was free precisely because + Get skipped the algorithm's mutex entirely. Making Get touch LRU + (regardless of the Item-aware vs map-based path) costs roughly this + much under contention. + +### Disposition + +Per the RFC's own discipline (`Reject A and revisit if any criterion fails`): + +1. **Do not migrate the remaining 4 algorithms** (LFU, Clock, ARC, CAWOLFU). Spike LRU only. +1. **Keep `WithItemAwareEviction` as experimental opt-in.** Documented as + "slower on Get, semantically-correct LRU." Default stays legacy. +1. **Do not pursue Option A2** (co-located locks) — the win Option A + would have justified A2 isn't there to amortize the bigger refactor. +1. **The "Get does not touch LRU" semantic gap is a separate concern** + that could be addressed inside the legacy path (have HyperCache.Get + call `evictionAlgorithm.Get(key)`) at similar cost to the Item-aware + Touch — i.e., the cost is fundamental to "real LRU", not specific + to either path. + +### Final disposition: code removed + +After preserving the spike as `WithItemAwareEviction` for one round, we +removed it entirely. Reasoning: + +- **The hypothesis was falsified.** Keeping experimental code that + didn't meet its bar is debt — every future linter run, every + dependency bump, every `Item` layout change has to consider it. For + a user benefit nobody actually asked for. +- **"Experimental, slower, but more correct" is a confusing knob.** + Users would have to understand both the LRU-vs-LRW gap *and* the + per-op trade-off to opt-in correctly. Nobody reads opt-in docs that + carefully. +- **`Item` bloat propagates conceptually.** `cache.Item` IS the wire + format for DistMemory replication; adding eviction-state fields + (even unexported with `json:"-"`) weakens the mental model "Item is + what gets serialized." Smaller surface = clearer guarantees. +- **CLAUDE.md prime directive #4 (Stay on task).** "Don't add features + beyond what the task requires." The spike was the test; the test + failed; the smallest correct response is to revert. + +What was removed (one cleanup commit, ~400 lines deleted): + +- `WithItemAwareEviction` option +- `IAlgorithmItemAware` interface + `LRUItemAware` + `ShardedItemAware` + types +- `Item.evictPrev/evictNext/inEvictList` fields + 6 accessor methods +- `HyperCache.recordSet/recordTouch/recordDelete/evictNext` helpers +- `itemAwareAlg` / `itemAwareEviction` fields on HyperCache +- `lru_item_aware_test.go`, `hypercache_item_aware_eviction_benchmark_test.go` + +What stays: + +- This RFC. The engineering artifact is the *measurement and + reasoning*, not the code. + +### Lessons for future eviction work + +- **Sharded contention is the dominant cost on the eviction hot path, + not dual-map overhead.** A future RFC targeting Set throughput + should look at lock-free rotation (atomic CAS on linked-list nodes, + hazard pointers, RCU) rather than data layout. +- **"Real LRU" has a fundamental Get-side cost** under contention + (~50% on this codebase, our measurement). The legacy path's "LRU is + least-recently-written" is free precisely because Get skips the + algorithm mutex. If a user files a correctness bug, the smallest + fix is a one-line option that has `HyperCache.Get` call + `evictionAlgorithm.Get(key)` on the legacy path — same cost as the + Item-aware path, zero infrastructure. **Don't pre-build that + option until a user asks for it.** diff --git a/docs/rfcs/0002-generic-item-typing.md b/docs/rfcs/0002-generic-item-typing.md new file mode 100644 index 0000000..2e9dfaf --- /dev/null +++ b/docs/rfcs/0002-generic-item-typing.md @@ -0,0 +1,292 @@ +# RFC 0002 — `Item[V any]` generic value typing + +- **Status**: Draft +- **Target**: v2.x (wrapper, additive) → v3.0 (deep generics, breaking) +- **Owners**: TBD +- **Related code**: [pkg/cache/v2/item.go](../../pkg/cache/v2/item.go), [pkg/cache/v2/cmap.go](../../pkg/cache/v2/cmap.go), [hypercache_io.go](../../hypercache_io.go) + +## Summary + +Eliminate the `any`-based ergonomics of `HyperCache.Get` (every call site type-asserts) by offering a typed wrapper API in v2.x and a fully-generic `Item[V]` chain in v3. The two-phase approach gives users the typed surface they want now without forcing the deeper refactor immediately. + +## Background + +`Item.Value` is `any`: + +```go +// pkg/cache/v2/item.go:62-72 +type Item struct { + Key string + Value any + // ... +} +``` + +User code on every Get: + +```go +val, ok := cache.Get(ctx, "session:42") +if !ok { + return nil +} +session, ok := val.(*Session) // unsafe in general; tedious +``` + +Wrong type assertions panic at runtime. There is no compile-time guarantee that what was Set as `*Session` is what Get returns. In a high-stakes financial environment this is a class of latent bug the type system *can* prevent — generics are the right tool. + +`HyperCache` already takes one type parameter `T backend.IBackendConstrain` to bind the cache to a specific backend impl. Adding a second value-type parameter is the natural next step. + +### What's actually generic-able + +The whole stack stores `Item.Value`: + +- `pkg/cache/v2.ConcurrentMap` — `map[string]*Item` +- `pkg/eviction.IAlgorithm` — `Set(key string, value any)`, `Get(key string) (any, bool)` +- `pkg/backend.IBackend[T]` — `Set(ctx, *Item)`, `Get(ctx, key) (*Item, bool)` +- `HyperCache[T]` — public CRUD methods + +Eviction algorithms don't *inspect* the value (LRU/LFU/Clock track recency/frequency, not the bytes); they could stay `any`. ConcurrentMap, IBackend, and HyperCache all touch the value semantically. + +### What complicates this + +- **Heterogeneous values are real.** Some users cache (URL, []byte) and (sessionID, *Session) in the same instance. Forcing a single V regresses that workflow. +- **Backends serialize values** (Redis, RedisCluster). The serializer accepts `any` today via reflection. With concrete V, we either need V-aware serializers or keep the boundary at `any`. +- **DistMemory replicates Items across the wire** as JSON. Generic V vs `any` is invisible at the JSON layer; serialization stays string-keyed. +- **The pool** (`ItemPoolManager`) holds `*Item`. Generic Item[V] pools are per-V — type-erased pools don't work without reflection. + +## Goals + +1. **Compile-time type safety** for Get/Set on the common single-V case. +1. **Zero forced migration** for v2.x users — typed surface is opt-in. +1. **No double type assertion**: a typed-wrapper `Get` returns `V`, not `(any, type-assert-then-V)`. +1. **No runtime cost beyond a single type assertion** (in v2.x wrapper) → zero in v3 (full generics). + +## Non-goals + +- Removing the untyped `HyperCache` API. It stays for heterogeneous use cases. +- Generics on the eviction algorithm layer. Algorithms don't read V; staying `any` saves a parameter. +- Generics on the backend tag `T`. Already typed; no change. + +## Options + +### Option A — `hypercache.Typed[V]` wrapper (v2.x; additive) + +A thin wrapper holding an existing `*HyperCache[T]` plus a phantom V: + +```go +// hypercache_typed.go (new) +package hypercache + +type Typed[T backend.IBackendConstrain, V any] struct { + hc *HyperCache[T] +} + +func NewTyped[T backend.IBackendConstrain, V any](hc *HyperCache[T]) *Typed[T, V] { + return &Typed[T, V]{hc: hc} +} + +func (t *Typed[T, V]) Set(ctx context.Context, key string, value V, expiration time.Duration) error { + return t.hc.Set(ctx, key, value, expiration) +} + +func (t *Typed[T, V]) Get(ctx context.Context, key string) (V, bool) { + var zero V + + raw, ok := t.hc.Get(ctx, key) + if !ok { + return zero, false + } + + v, ok := raw.(V) + if !ok { + // Cache held the wrong type for this key — treat as miss to + // avoid surfacing a runtime panic to the caller. Document this + // explicitly: typed wrappers SHOULD be the only writer for a + // given key, and cross-typed reads are caller error. + return zero, false + } + + return v, true +} + +// Same wrapping for GetOrSet, GetMultiple, GetWithInfo, Remove, etc. +``` + +Construction: + +```go +hc, _ := hypercache.NewInMemoryWithDefaults(ctx, 10_000) +sessions := hypercache.NewTyped[backend.InMemory, *Session](hc) + +sessions.Set(ctx, "u:42", &Session{...}, time.Hour) +s, ok := sessions.Get(ctx, "u:42") // s is *Session +``` + +**Pros**: + +- **Zero breaking changes.** Users who don't want types keep what they have. +- **Tiny diff.** New file, ~150 lines, no touches to existing code paths. +- **Composable.** A single underlying `HyperCache[T]` can have multiple `Typed[T, V]` views over disjoint keyspaces (`sessions`, `tokens`, etc.). +- **Sets the v3 expectation.** Once we ship the typed surface, users who want it migrate; the demand signal informs whether v3's deep generics are worth the cost. + +**Cons**: + +- **Still `any` underneath.** Each Get does one type-assert at the wrapper layer. Cost: a single type-check (~1-2 ns). +- **Wrong-type-stored returns miss-not-error.** Documented as caller-side discipline ("don't share a key across Typed instances of different V"); not enforced. +- **Doesn't simplify `Item.Value`** — internal callers still see `any`. + +### Option B — Deep generics: `Item[V]`, `ConcurrentMap[V]`, `HyperCache[T, V]` + +`cache.Item` becomes `cache.Item[V any]`: + +```go +type Item[V any] struct { + Key string + Value V + LastAccess time.Time + // ... +} +``` + +`ConcurrentMap[V]`: + +```go +type ConcurrentMap[V any] struct { /* shards map[string]*Item[V] */ } +``` + +`IBackend[T, V]`: + +```go +type IBackend[T IBackendConstrain, V any] interface { + Get(ctx context.Context, key string) (*Item[V], bool) + Set(ctx context.Context, item *Item[V]) error + // ... +} +``` + +`HyperCache[T, V]`. Two type parameters everywhere. + +**Pros**: + +- **Fully type-safe.** No `any` in user-facing types. +- **Zero runtime cost.** Type assertions are gone. +- **Items can be specialized.** An `Item[[]byte]` skips reflection-based size estimation that today's `SetSize` uses; the fast paths in [pkg/cache/v2/item.go:117-143](../../pkg/cache/v2/item.go#L117-L143) become the only path. + +**Cons**: + +- **Massive breaking change.** Every public type that mentions Item or HyperCache changes shape. +- **Heterogeneous workloads regress.** `Item[any]` works but is awkward; users mixing types must write `HyperCache[T, any]` everywhere. +- **Pool complications.** `ItemPoolManager` becomes `ItemPoolManager[V]`; you need one pool per V. The DistMemory rebalancer (which moves Items between cache instances) must be V-typed too. Real plumbing pain. +- **Two type parameters.** `HyperCache[backend.InMemory, *Session]` is a mouthful; users will type-alias. +- **Backends that serialize (Redis) can't statically dispatch on V** without further machinery (V-typed serializers, registries). The boundary cost doesn't disappear — it moves. + +### Option C — Generic on ConcurrentMap + HyperCache only; backends keep `any` + +Mid-point: typed at the layers users see, untyped at the persistence boundary. + +```go +type HyperCache[T IBackendConstrain, V any] struct { + backend backend.IBackend[T] // backend stays IBackend[T], not [T, V] + // ... +} + +func (h *HyperCache[T, V]) Set(ctx context.Context, key string, value V, exp time.Duration) error { + item := h.itemPool.Get() + item.Value = value // V → any; loses type info at backend boundary + return h.backend.Set(ctx, item) +} + +func (h *HyperCache[T, V]) Get(ctx context.Context, key string) (V, bool) { + var zero V + + item, ok := h.backend.Get(ctx, key) + if !ok { + return zero, false + } + + v, ok := item.Value.(V) + if !ok { + return zero, false + } + + return v, true +} +``` + +**Pros**: + +- Typed user-facing API. +- Backends untouched (Redis serializer code unchanged). +- Less invasive than B. + +**Cons**: + +- **Same runtime cost as Option A** (type-assertion at HyperCache boundary). +- **Larger diff than A** for the same runtime behavior. +- **Doesn't enable the Item[V] specializations** that motivate B. + +## Recommendation + +**Two-phase**: + +- **v2.x — Option A.** Ship `hypercache.Typed[V]` as a thin wrapper. Zero breaking changes. Captures 90% of the ergonomic win (no caller-side type asserts) at 10% of the cost. +- **v3.x — Option B**, conditional on usage signal from A. + - If most v2.x users adopt `Typed[V]` and report friction, that's strong evidence for full generics. + - If usage is mixed (many users still want untyped for heterogeneous keys), keep the wrapper as the recommended pattern and skip B. + +This avoids spending the deep-generics budget speculatively. Option A is reversible; Option B is not. + +## Implementation plan (Option A, v2.x) + +1. Add `hypercache_typed.go` with `Typed[T, V]`. Wrap each public method on `HyperCache[T]` (Set, Get, GetOrSet, GetWithInfo, GetMultiple, Remove, Clear, List). +1. `GetMultiple` returns `map[string]V` and `map[string]error` — wrong-type entries land in the error map under a new sentinel `ErrTypeMismatch`. +1. Document the wrapper as the recommended access pattern in the package doc. Examples in [`__examples/typed/`](../../__examples/typed/) (new). +1. Tests: confirm the wrapper round-trips Set→Get cleanly for several V types ([]byte, struct, pointer, slice, map). Confirm cross-typed read returns miss without panic. + +## Implementation plan (Option B, v3.0 — sketch only) + +If we commit to v3: + +1. Generate `pkg/cache/v3` parallel to v2 with `Item[V]` and `ConcurrentMap[V]`. Don't touch v2 — let it ship in parallel. +1. `IBackend[T, V]`: rewrite the four backends. InMemory and DistMemory keep typed Items; Redis-family backends accept `Item[V]` and serialize V via a `Serializer[V]` interface (callers register one per V they cache). +1. `HyperCache[T, V]`. The existing constructor `New[T](ctx, bm, config)` becomes `New[T, V](ctx, bm, config)`. Type alias `New[T] = New[T, any]` preserves the unchanged-API path for users who don't want V. +1. ItemPoolManager becomes `ItemPoolManager[V]`. One pool per `(V)` typedef. +1. CHANGELOG entry: explicit list of generic signatures users must update; codemod recipe with `gofmt -r`. + +Estimated diff: 2-4 weeks of focused work + extensive test re-baselining. Don't sequence behind any other v3 work — make it the v3 anchor. + +## Migration + +- **v2.x users (Option A landed)**: no change required. Adopt `Typed[V]` opportunistically. +- **v2.x → v3 with full generics (Option B landed)**: every `*HyperCache[T]` becomes `*HyperCache[T, V]`; pick V (likely `any` for code that doesn't want to commit). For typed call sites that already used `Typed[V]`, drop the wrapper — the underlying API now matches. + +## Risks + +| Risk | Likelihood | Mitigation | +| ---- | ---------- | ---------- | +| Wrapper's silent type-mismatch-as-miss surprises users | Medium | Document loudly; provide `GetTyped` that returns `(V, error)` with `ErrTypeMismatch` for callers who need the distinction. | +| Users mix `Typed[V]` and untyped Get on the same key, get inconsistent reads | Low | Document as caller error; add an example showing the wrong pattern with a comment. | +| Deep generics (B) blow up compile time / binary size | Low | Go generics are largely zero-cost via stenciling; measure on a representative repo before committing to v3. | +| Serializer-per-V registry (B) explodes API surface | Medium | Provide a default reflection-based serializer for backwards compat; concrete V serializers are opt-in. | +| Eviction algorithms need updating (B) | Low | They don't read V; keep `IAlgorithm` `any`-based or take `*Item[any]`. Decoupling from V is the right call regardless. | + +## Open questions + +- **Should `Typed[V]` enforce V via a constraint?** E.g., require V to be `comparable` so the wrapper can compare against zero-V. Probably no — comparable rules out slices/maps which are common cache values. +- **`ErrTypeMismatch` shape**: should the wrapper's Get return `(V, bool, error)` or just `(V, bool)`? The latter matches the existing API (Get returns `(any, bool)`); the former is friendlier to callers who want to distinguish miss-vs-bad-type. +- **Coexistence with Phase-5d lifecycle ctx**: the wrapper passes ctx through unchanged; no interaction. +- **DistMemory replication of typed Items (B)**: serialized Item carries V's JSON shape; the receiving node decodes against the same V. Cross-version V drift is a wire-format concern (today's `any` covers it because reflection picks up the right shape; with V we'd want a versioned schema). Defer the schema-evolution story to its own RFC if/when B lands. + +## Decision criteria + +Land Option A if: + +- It compiles. +- Round-trip tests pass for representative V types. +- Wrapper Get+Set adds ≤ 5 ns/op overhead vs untyped Get+Set. + +Land Option B if: + +- A has been in production for ≥ 1 release cycle. +- Adoption metric (e.g., GitHub-search, internal grep) shows ≥ 50% of new code uses `Typed[V]`. +- A v3-cut is otherwise on the roadmap (Item-embedded eviction from RFC 0001 is a natural travel companion). diff --git a/hypercache_typed.go b/hypercache_typed.go new file mode 100644 index 0000000..d1ec027 --- /dev/null +++ b/hypercache_typed.go @@ -0,0 +1,174 @@ +package hypercache + +import ( + "context" + "time" + + "github.com/hyp3rd/hypercache/internal/sentinel" + "github.com/hyp3rd/hypercache/pkg/backend" + cache "github.com/hyp3rd/hypercache/pkg/cache/v2" +) + +// Typed is a thin wrapper around HyperCache that adds compile-time +// type-safety on Set/Get/GetOrSet/GetMultiple/GetWithInfo without +// breaking the existing untyped API. Internally Item.Value is still +// stored as any; the wrapper performs a single type assertion on read +// and rejects cross-typed entries as ErrTypeMismatch. +// +// See docs/rfcs/0002-generic-item-typing.md for the full design. +// +// Lifecycle: Typed does NOT own the underlying HyperCache. Callers are +// responsible for calling hc.Stop themselves; multiple Typed instances +// of different V parameters can share a single underlying cache. +// +// Discipline: do not mix Typed[V1] and Typed[V2] (or Typed and untyped) +// reads against the same key — the wrapper treats wrong-type entries as +// ErrTypeMismatch on Get, but the storage layer has no notion of V. +type Typed[T backend.IBackendConstrain, V any] struct { + hc *HyperCache[T] +} + +// NewTyped wraps an existing HyperCache instance to provide the typed +// surface. The underlying cache continues to function unchanged for any +// callers that hold a reference to it. +func NewTyped[T backend.IBackendConstrain, V any](hc *HyperCache[T]) *Typed[T, V] { + return &Typed[T, V]{hc: hc} +} + +// Underlying returns the wrapped HyperCache. Use it for operations the +// typed wrapper deliberately does not expose (List, Stop, Capacity, +// stats, etc.) — those operate on metadata that is V-agnostic and +// adding type parameters would only obscure the call site. +func (t *Typed[T, V]) Underlying() *HyperCache[T] { return t.hc } + +// Set stores value under key. expiration ≤ 0 means no expiry. +// Mirrors HyperCache.Set; the V constraint enforces the value type at +// compile time so callers cannot accidentally Set the wrong shape. +func (t *Typed[T, V]) Set(ctx context.Context, key string, value V, expiration time.Duration) error { + return t.hc.Set(ctx, key, value, expiration) +} + +// Get retrieves the typed value for key. Returns the zero value and +// false on miss, expired entry, or type mismatch — the latter is +// treated as a miss so a single Get can't panic on a stale heterogeneous +// entry. Use GetTyped if the caller needs to distinguish miss from +// type-mismatch. +func (t *Typed[T, V]) Get(ctx context.Context, key string) (V, bool) { + var zero V + + raw, ok := t.hc.Get(ctx, key) + if !ok { + return zero, false + } + + v, ok := raw.(V) + if !ok { + return zero, false + } + + return v, true +} + +// GetTyped is the explicit-error variant of Get. Returns: +// - (V, nil) on hit +// - (zero, ErrKeyNotFound) on miss / expired +// - (zero, ErrTypeMismatch) when the stored value is not assertable to V +// +// Callers who want to surface "wrong type was stored under this key" +// (e.g., as an alerting signal that a Typed[V1] and Typed[V2] are +// fighting over the same key) should use this form. +func (t *Typed[T, V]) GetTyped(ctx context.Context, key string) (V, error) { + var zero V + + raw, ok := t.hc.Get(ctx, key) + if !ok { + return zero, sentinel.ErrKeyNotFound + } + + v, ok := raw.(V) + if !ok { + return zero, sentinel.ErrTypeMismatch + } + + return v, nil +} + +// GetWithInfo returns the cache.Item plus the typed value. The Item is +// returned as-is (Value still typed as any); use the second return for +// the V-typed value when assignment cleanliness matters. Returns +// (nil, zero, false) on miss / expired / type-mismatch (same fail-soft +// semantics as Get). +func (t *Typed[T, V]) GetWithInfo(ctx context.Context, key string) (*cache.Item, V, bool) { + var zero V + + item, ok := t.hc.GetWithInfo(ctx, key) + if !ok { + return nil, zero, false + } + + v, ok := item.Value.(V) + if !ok { + return nil, zero, false + } + + return item, v, true +} + +// GetOrSet returns the existing value when present, or stores value and +// returns it. If the existing entry is the wrong type, the wrapper +// surfaces ErrTypeMismatch rather than overwriting (avoids silent +// data loss when two callers fight over a key). +func (t *Typed[T, V]) GetOrSet(ctx context.Context, key string, value V, expiration time.Duration) (V, error) { + var zero V + + raw, err := t.hc.GetOrSet(ctx, key, value, expiration) + if err != nil { + return zero, err + } + + v, ok := raw.(V) + if !ok { + return zero, sentinel.ErrTypeMismatch + } + + return v, nil +} + +// GetMultiple is the typed analog of HyperCache.GetMultiple. Hits land +// in the result map under their key; misses (sentinel.ErrKeyNotFound) +// and type mismatches (sentinel.ErrTypeMismatch) land in the error map. +// Mirrors the dual-map shape of the underlying API. +func (t *Typed[T, V]) GetMultiple(ctx context.Context, keys ...string) (map[string]V, map[string]error) { + rawHits, errs := t.hc.GetMultiple(ctx, keys...) + + hits := make(map[string]V, len(rawHits)) + + for k, raw := range rawHits { + v, ok := raw.(V) + if !ok { + // Promote the type mismatch into the error map so the + // caller sees a uniform success/failure split. + errs[k] = sentinel.ErrTypeMismatch + + continue + } + + hits[k] = v + } + + return hits, errs +} + +// Remove deletes one or more keys. V is unused but kept on the receiver +// for API symmetry — callers don't need to know which Typed instance +// owns a key to delete it. +func (t *Typed[T, V]) Remove(ctx context.Context, keys ...string) error { + return t.hc.Remove(ctx, keys...) +} + +// Clear removes every entry from the underlying cache. Affects entries +// stored by other Typed instances and untyped callers — it operates on +// the storage layer, not on V. +func (t *Typed[T, V]) Clear(ctx context.Context) error { + return t.hc.Clear(ctx) +} diff --git a/hypercache_typed_test.go b/hypercache_typed_test.go new file mode 100644 index 0000000..a772880 --- /dev/null +++ b/hypercache_typed_test.go @@ -0,0 +1,224 @@ +package hypercache_test + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/hyp3rd/hypercache" + "github.com/hyp3rd/hypercache/internal/sentinel" + "github.com/hyp3rd/hypercache/pkg/backend" +) + +// session is a value type used to exercise Typed[*session] round-trips +// against a typed wrapper — represents a typical user-defined cache value +// (struct pointer, not a stdlib type). +type session struct { + UserID string + Token string +} + +// newTypedTestCache spins up an in-memory HyperCache + a Typed[T, V] +// wrapper sharing it. Returns both so tests can poke at the underlying +// cache for cross-typed scenarios. +func newTypedTestCache[V any](t *testing.T) (*hypercache.HyperCache[backend.InMemory], *hypercache.Typed[backend.InMemory, V]) { + t.Helper() + + hc, err := hypercache.NewInMemoryWithDefaults(context.Background(), 100) + require.NoError(t, err) + + t.Cleanup(func() { _ = hc.Stop(context.Background()) }) + + return hc, hypercache.NewTyped[backend.InMemory, V](hc) +} + +// TestTyped_RoundTrip is the happy-path: Set then Get returns the typed +// value with no caller-side type assertion. +func TestTyped_RoundTrip(t *testing.T) { + t.Parallel() + + _, typed := newTypedTestCache[*session](t) + + want := &session{UserID: "u-42", Token: "tok-abc"} + + require.NoError(t, typed.Set(context.Background(), "session:42", want, time.Hour)) + + got, ok := typed.Get(context.Background(), "session:42") + require.True(t, ok, "Get should hit") + require.Equal(t, want, got) +} + +// TestTyped_RoundTrip_PrimitiveValue covers the [...] stdlib-typed case; +// users frequently cache []byte payloads and the wrapper must not require +// pointer-only V. +func TestTyped_RoundTrip_PrimitiveValue(t *testing.T) { + t.Parallel() + + _, typed := newTypedTestCache[[]byte](t) + + want := []byte("hello, typed") + require.NoError(t, typed.Set(context.Background(), "k", want, time.Hour)) + + got, ok := typed.Get(context.Background(), "k") + require.True(t, ok) + require.Equal(t, want, got) +} + +// TestTyped_GetMiss checks the zero-value-and-false return on miss. +func TestTyped_GetMiss(t *testing.T) { + t.Parallel() + + _, typed := newTypedTestCache[string](t) + + got, ok := typed.Get(context.Background(), "no-such-key") + require.False(t, ok) + require.Empty(t, got, "miss should return zero V") +} + +// TestTyped_TypeMismatchTreatedAsMiss verifies the wrapper's fail-soft +// semantics: when the underlying cache holds a value of a different +// type, Get returns (zero, false) rather than panicking. +func TestTyped_TypeMismatchTreatedAsMiss(t *testing.T) { + t.Parallel() + + hc, typed := newTypedTestCache[*session](t) + + // Store an int via the underlying cache, then read via the typed + // wrapper expecting *session. + require.NoError(t, hc.Set(context.Background(), "k", 42, time.Hour)) + + got, ok := typed.Get(context.Background(), "k") + require.False(t, ok, "wrong-type read should miss, not panic") + require.Nil(t, got) +} + +// TestTyped_GetTypedSurfacesMismatch is the explicit-error counterpart: +// callers who want to distinguish miss from wrong-type use GetTyped. +func TestTyped_GetTypedSurfacesMismatch(t *testing.T) { + t.Parallel() + + hc, typed := newTypedTestCache[*session](t) + + // Miss → ErrKeyNotFound. + _, err := typed.GetTyped(context.Background(), "no-such-key") + require.ErrorIs(t, err, sentinel.ErrKeyNotFound) + + // Wrong type stored → ErrTypeMismatch. + require.NoError(t, hc.Set(context.Background(), "k", 42, time.Hour)) + + _, err = typed.GetTyped(context.Background(), "k") + require.ErrorIs(t, err, sentinel.ErrTypeMismatch) +} + +// TestTyped_GetWithInfo verifies the metadata + typed-value triple +// return surface. *cache.Item carries the access count, expiration, etc. +func TestTyped_GetWithInfo(t *testing.T) { + t.Parallel() + + _, typed := newTypedTestCache[string](t) + + require.NoError(t, typed.Set(context.Background(), "k", "v", time.Hour)) + + item, value, ok := typed.GetWithInfo(context.Background(), "k") + require.True(t, ok) + require.NotNil(t, item) + require.Equal(t, "v", value) + require.Equal(t, "k", item.Key) +} + +// TestTyped_GetOrSet covers both branches: existing value and freshly +// stored value. +func TestTyped_GetOrSet(t *testing.T) { + t.Parallel() + + _, typed := newTypedTestCache[string](t) + + // First call stores the value. + got, err := typed.GetOrSet(context.Background(), "k", "v1", time.Hour) + require.NoError(t, err) + require.Equal(t, "v1", got) + + // Second call returns the stored value, ignoring the new one. + got, err = typed.GetOrSet(context.Background(), "k", "v2-not-stored", time.Hour) + require.NoError(t, err) + require.Equal(t, "v1", got) +} + +// TestTyped_GetOrSetTypeMismatch verifies that GetOrSet refuses to +// silently overwrite a wrong-type entry. +func TestTyped_GetOrSetTypeMismatch(t *testing.T) { + t.Parallel() + + hc, typed := newTypedTestCache[string](t) + + // Pre-populate with the wrong type. + require.NoError(t, hc.Set(context.Background(), "k", 42, time.Hour)) + + got, err := typed.GetOrSet(context.Background(), "k", "v", time.Hour) + require.ErrorIs(t, err, sentinel.ErrTypeMismatch) + require.Empty(t, got) +} + +// TestTyped_GetMultiple covers the dual-map result shape with a mix of +// hits, misses, and type mismatches. +func TestTyped_GetMultiple(t *testing.T) { + t.Parallel() + + hc, typed := newTypedTestCache[string](t) + + require.NoError(t, typed.Set(context.Background(), "k1", "v1", time.Hour)) + require.NoError(t, typed.Set(context.Background(), "k2", "v2", time.Hour)) + // Inject a wrong-type entry under k3 via the underlying cache. + require.NoError(t, hc.Set(context.Background(), "k3", 42, time.Hour)) + + hits, errs := typed.GetMultiple(context.Background(), "k1", "k2", "k3", "missing") + + require.Equal(t, map[string]string{"k1": "v1", "k2": "v2"}, hits) + require.ErrorIs(t, errs["k3"], sentinel.ErrTypeMismatch) + require.ErrorIs(t, errs["missing"], sentinel.ErrKeyNotFound) +} + +// TestTyped_Remove verifies the wrapper does not need V on the +// delete path — Remove is V-agnostic. +func TestTyped_Remove(t *testing.T) { + t.Parallel() + + _, typed := newTypedTestCache[string](t) + + require.NoError(t, typed.Set(context.Background(), "k", "v", time.Hour)) + require.NoError(t, typed.Remove(context.Background(), "k")) + + _, ok := typed.Get(context.Background(), "k") + require.False(t, ok) +} + +// TestTyped_SharedUnderlyingCache exercises the documented composition +// pattern: one HyperCache, two Typed views over disjoint keyspaces. +// Confirms each view sees its own keys and doesn't trip over the other's +// (the wrapper provides no namespace isolation; callers handle that). +func TestTyped_SharedUnderlyingCache(t *testing.T) { + t.Parallel() + + hc, _ := newTypedTestCache[string](t) // discard the string view + + sessions := hypercache.NewTyped[backend.InMemory, *session](hc) + tokens := hypercache.NewTyped[backend.InMemory, string](hc) + + require.NoError(t, sessions.Set(context.Background(), "session:42", &session{UserID: "u-42"}, time.Hour)) + require.NoError(t, tokens.Set(context.Background(), "token:abc", "secret", time.Hour)) + + s, ok := sessions.Get(context.Background(), "session:42") + require.True(t, ok) + require.Equal(t, "u-42", s.UserID) + + tok, ok := tokens.Get(context.Background(), "token:abc") + require.True(t, ok) + require.Equal(t, "secret", tok) + + // Underlying() exposes the shared cache for users who need V-agnostic + // operations like List or Stop. + require.NotNil(t, sessions.Underlying()) + require.Same(t, sessions.Underlying(), tokens.Underlying()) +} diff --git a/internal/sentinel/sentinel.go b/internal/sentinel/sentinel.go index 2855360..0a5a575 100644 --- a/internal/sentinel/sentinel.go +++ b/internal/sentinel/sentinel.go @@ -72,6 +72,9 @@ var ( // ErrUnauthorized is returned when an HTTP request to the dist transport is missing or carries an invalid auth token. ErrUnauthorized = ewrap.New("unauthorized") + // ErrTypeMismatch is returned by the typed cache wrapper when a stored value is not assertable to the wrapper's V parameter. + ErrTypeMismatch = ewrap.New("cached value type mismatch") + // ErrNotOwner is returned when a distributed backend instance is not the ring owner for a key. ErrNotOwner = ewrap.New("not ring owner")