Repository navigation
refactor: internalize deterministic ramp sampling - #86
Merged
Merged
Conversation
lan17
marked this pull request as ready for review
July 25, 2026 19:01
lan17
commented
Jul 25, 2026
| if (Object.hasOwn(config, "urnPrefix")) { | ||
| throw new TypeError('DialCacheConfig.urnPrefix was renamed to "namespace"'); | ||
| } | ||
| if (Object.hasOwn(config, "rampSampler")) { |
Owner
Author
There was a problem hiding this comment.
should we just remove these checks? Perhaps its better to check that whatever object is passed in only has fields defined in DialCacheConfig. is that possible in ts?
This was referenced Jul 25, 2026
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.
Summary
DialCacheConfig.rampSamplerand the publicCacheRampSample,CacheRampSampler,deterministicRampSampler, andrandomRampSamplerexportsrampSamplerconfiguration explicitly instead of silently ignoring itWhy
The public sampler made a basic ramp percentage depend on arbitrary caller code on the cache hot path. Because the callback could be asynchronous, throw, reject, or return invalid values, it expanded DialCache's liveness and fail-open surface and allowed partial-ramp behavior to vary between instances. The only production implementation used by default was already deterministic.
DialCache now owns the meaning of a partial ramp. Applications that need externally coordinated cohorts can implement that policy in the existing
cacheConfigProviderby returning sparse per-key ramp overrides of0or100.Breaking change
This removes published API from
v0.9.0, so the conventional title deliberately marks the change as breaking. Consumers must removerampSamplerfromnew DialCache(...)and stop importing the removed sampler symbols. Existing partial ramps continue using the same deterministic key-and-layer cohorts.Validation
v22.22.0:corepack pnpm checkcorepack pnpm audit --prod --audit-level high: no known vulnerabilitiesgit diff --check: passedgit merge-tree --write-tree origin/main HEAD: cleancorepack pnpm test:integrationwas attempted, but the local Docker daemon is not running. Testcontainers stopped before executing code withCould not find a working container runtime strategy; all 69 integration tests were skipped. CI remains responsible for the real Redis/Valkey/cluster matrix.