perf(redis-lua): fast-path HGET / HEXISTS / SISMEMBER inside Lua scripts#567
Merged
perf(redis-lua): fast-path HGET / HEXISTS / SISMEMBER inside Lua scripts#567
Conversation
Follow-up to PR #565 (which fast-pathed the top-level HGET / HEXISTS / SISMEMBER handlers). BullMQ and similar Lua-heavy workloads go through luaScriptContext.cmdHGet etc., where the legacy hashState / setState loaders still issued the full ~8-seek keyTypeAt probe per key touched by a script. A typical BullMQ script (moveToActive etc.) touches 5-10 keys, so that was 40-80 wasted seeks per script before any real work ran. Port the PR #565 pattern verbatim into the Lua command handlers: cmdHGet -> server.hashFieldFastLookup on the HashFieldKey cmdHExists -> server.hashFieldFastExists cmdSIsMember-> server.setMemberFastExists Each handler still checks c.hashes / c.sets first so in-script mutations (HSET + HGET in the same Eval) remain visible -- the fast path only fires when the key has NOT been loaded into the script's local state cache. Misses fall through to the legacy hashState / setState loader so legacy-blob encodings, WRONGTYPE, and TTL- expired keys behave exactly as before. Three small helpers factored out (hgetFromHashState, hexistsFromHashState, sismemberFromSetState) so each command body stays under the cyclomatic-complexity cap. Tests: adapter/redis_lua_collection_fastpath_test.go drives each command through an actual redis.Eval and covers fast-path-hit / in-script-write-visible / miss / WRONGTYPE / TTL- expired cases. Expected effect on BullMQ-style workloads: per-script overhead from keyTypeAt probes drops by roughly (keys_touched × 8) seeks, which is the dominant cost for short scripts that only read a couple of fields per key.
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the Redis adapter’s wide-column collection read fast-paths into the Lua script execution context, so redis.call("HGET"/"HEXISTS"/"SISMEMBER", ...) inside EVAL avoids the expensive keyTypeAt probe when the script-local cache hasn’t loaded the key yet.
Changes:
- Add cache-first + wide-column fast-path lookups to Lua handlers for
HGET,HEXISTS, andSISMEMBER, with slow-path fallback via existinghashState/setState. - Factor small helpers (
hgetFromHashState,hexistsFromHashState,sismemberFromSetState) to keep command bodies small. - Add a new end-to-end test file that exercises the Lua fast paths via real
EVALcalls.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| adapter/redis_lua_context.go | Adds Lua-context fast-paths for HGET/HEXISTS/SISMEMBER with cache-first behavior and slow-path fallback helpers. |
| adapter/redis_lua_collection_fastpath_test.go | New integration tests validating Lua EVAL behavior for the fast paths. |
Comment on lines
+254
to
+270
| func TestLua_SISMEMBER_WRONGTYPE(t *testing.T) { | ||
| t.Parallel() | ||
| nodes, _, _ := createNode(t, 3) | ||
| defer shutdown(nodes) | ||
|
|
||
| ctx := context.Background() | ||
| rdb := redis.NewClient(&redis.Options{Addr: nodes[0].redisAddress}) | ||
| defer func() { _ = rdb.Close() }() | ||
|
|
||
| require.NoError(t, rdb.Set(ctx, "lua:s:str", "x", 0).Err()) | ||
| _, err := rdb.Eval(ctx, | ||
| `return redis.call("SISMEMBER", KEYS[1], "member")`, | ||
| []string{"lua:s:str"}, | ||
| ).Result() | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), "WRONGTYPE") | ||
| } |
Comment on lines
+28
to
+32
| const luaFastPathTTL = 80 * time.Millisecond | ||
|
|
||
| func waitForLuaTTL() { | ||
| time.Sleep(luaFastPathTTL + 50*time.Millisecond) | ||
| } |
…nswer Addresses High-severity finding from the independent review of PR #567. The previous fast path only consulted c.hashes[key] / c.sets[key], which misses the three script-local scenarios where another command earlier in the Eval has published an authoritative type answer via c.cachedType(): 1. A prior DEL sets c.deleted[key]=true but does NOT populate c.hashes[key] unless the hash was already loaded. The fast path would then read pebble at c.startTS and surface pre-script values instead of nil. 2. A prior SET / other-type write populates c.strings[key] etc., changing the logical type to something other than Hash. The slow path's hashState -> keyType -> cachedType chain would return WRONGTYPE; the fast path silently returned the pre-script hash value. 3. A prior RENAME-away of the key, same as (1) for the source. Fix: consult c.cachedType(key) before the fast path. Any authoritative answer (loaded state, explicit deletion, different type) defers to the legacy hashState / setState slow path, which already handles all three cases correctly. Only "unknown" cachedType proceeds to the pebble fast path. Factor hgetFromSlowPath / hexistsFromSlowPath / sismemberFromSlowPath out of each handler to keep the cyclomatic complexity under the linter cap and share the slow-path implementation across the cachedType-cached and fast-path-miss branches. Tests: six new regressions in redis_lua_collection_fastpath_test.go pin each cachedType scenario end-to-end via redis.Eval: - SET-then-HGET / HEXISTS / SISMEMBER -> WRONGTYPE - DEL-then-HGET / HEXISTS / SISMEMBER -> nil / 0 - HSET-then-HGET-on-another-field in the same Eval still honors the cached state for both the seeded field and the unflushed one.
bootjp
added a commit
that referenced
this pull request
Apr 20, 2026
…nswer Addresses High-severity finding from the independent review of PR #567. The previous fast path only consulted c.hashes[key] / c.sets[key], which misses the three script-local scenarios where another command earlier in the Eval has published an authoritative type answer via c.cachedType(): 1. A prior DEL sets c.deleted[key]=true but does NOT populate c.hashes[key] unless the hash was already loaded. The fast path would then read pebble at c.startTS and surface pre-script values instead of nil. 2. A prior SET / other-type write populates c.strings[key] etc., changing the logical type to something other than Hash. The slow path's hashState -> keyType -> cachedType chain would return WRONGTYPE; the fast path silently returned the pre-script hash value. 3. A prior RENAME-away of the key, same as (1) for the source. Fix: consult c.cachedType(key) before the fast path. Any authoritative answer (loaded state, explicit deletion, different type) defers to the legacy hashState / setState slow path, which already handles all three cases correctly. Only "unknown" cachedType proceeds to the pebble fast path. Factor hgetFromSlowPath / hexistsFromSlowPath / sismemberFromSlowPath out of each handler to keep the cyclomatic complexity under the linter cap and share the slow-path implementation across the cachedType-cached and fast-path-miss branches. Tests: six new regressions in redis_lua_collection_fastpath_test.go pin each cachedType scenario end-to-end via redis.Eval: - SET-then-HGET / HEXISTS / SISMEMBER -> WRONGTYPE - DEL-then-HGET / HEXISTS / SISMEMBER -> nil / 0 - HSET-then-HGET-on-another-field in the same Eval still honors the cached state for both the seeded field and the unflushed one.
bootjp
added a commit
that referenced
this pull request
Apr 20, 2026
Addresses Medium-severity findings from the independent review of PR #568. - Arity: legacy cmdZScore returned nil when invoked with only a key because the missing-key check fired before the args[1] read. The fast-path intro indexed args[1] unconditionally and would panic on a 1-arg call (no arity validation in the Lua dispatcher). Add explicit `len(args) < 2` guard returning the standard "wrong number of arguments" error. - cachedType defer: align cmdZScore with the PR #567 follow-up fix. Prior SET on the same key (type change to String), DEL tombstone, or a prior ZADD / ZREM in this Eval must all go through the slow path so zsetState -> keyType -> cachedType produces the correct WRONGTYPE / nil / cached-score answer. The previous c.zsets[key]-only check missed DEL and type-change cases. - Factor zscoreFromSlowPath out to share the slow-path body between the cached and miss branches. Test: TestLua_ZSCORE_ArityTooFew covers the 1-arg panic regression; TestLua_ZSCORE_SetThenZScoreReturnsWrongType and TestLua_ZSCORE_DelThenZScoreReturnsNil pin the two cachedType scenarios end-to-end via redis.Eval. All existing ZSCORE tests still pass.
Owner
Author
|
/gemini review |
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the Redis adapter’s wide-column collection fast paths into the Lua script execution context (redis.call inside EVAL), so Lua-heavy workloads (e.g., BullMQ) benefit from the same reduced-seek lookups as top-level HGET / HEXISTS / SISMEMBER.
Changes:
- Add fast-path wide-column probes to Lua handlers for
HGET,HEXISTS, andSISMEMBER, with a guarded fallback to the existinghashState/setStateslow path to preserve semantics. - Factor small slow-path helpers (
hgetFromHashState,hexistsFromHashState,sismemberFromSetState) to keep command handlers small. - Add end-to-end tests that drive these paths via
redis.Eval, covering hits, misses, TTL-expired keys, WRONGTYPE, and in-script mutations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| adapter/redis_lua_context.go | Implements Lua-context fast paths for HGET/HEXISTS/SISMEMBER with slow-path fallbacks and helper refactors. |
| adapter/redis_lua_collection_fastpath_test.go | Adds Lua EVAL integration tests to pin fast-path behavior and cache/semantics invariants. |
Comment on lines
+1359
to
+1361
| if _, cached := c.cachedType(key); cached { | ||
| return hgetFromSlowPath(c, key, field) | ||
| } |
Comment on lines
+1560
to
+1564
| // authoritative answer, so prior DEL / RENAME / SET / HSET within | ||
| // this Eval is honored before the pebble probe fires. | ||
| if _, cached := c.cachedType(key); cached { | ||
| return hexistsFromSlowPath(c, key, field) | ||
| } |
Comment on lines
+2130
to
+2132
| if _, cached := c.cachedType(key); cached { | ||
| return sismemberFromSlowPath(c, key, member) | ||
| } |
…probes Addresses copilot follow-up on PR #567. cachedType() only returns "cached" when a loaded typed value exists. A key that was loaded and found MISSING (loaded=true, exists=false, e.g. a prior HGET miss) is not considered cached, so repeated HGET / HEXISTS / SISMEMBER on that key kept re-running the wide-column probe + slow-path fallback on every redis.call. Add luaHashAlreadyLoaded / luaSetAlreadyLoaded helpers that report whether a state is populated AND loaded, regardless of exists. Each of cmdHGet / cmdHExists / cmdSIsMember checks the loaded helper first (short-circuit to slow path, which returns immediately when c.hashes / c.sets has a loaded entry) before falling back to cachedType. Preserves the High-severity correctness guards for DEL / RENAME / SET-type-change (those still flow through cachedType); just trims the redundant storage probe on the repeated-miss scenario BullMQ- style scripts tend to produce when polling queue state. Test: TestLua_HGET_RepeatedMissReusesLoadedState pins correctness for 4 successive HGET misses on the same key (returns nil|nil|nil|nil). The seek-count saving itself is pprof-verifiable but not unit-testable cheaply; the guard is what matters here.
bootjp
added a commit
that referenced
this pull request
Apr 21, 2026
…probes Addresses copilot follow-up on PR #567. cachedType() only returns "cached" when a loaded typed value exists. A key that was loaded and found MISSING (loaded=true, exists=false, e.g. a prior HGET miss) is not considered cached, so repeated HGET / HEXISTS / SISMEMBER on that key kept re-running the wide-column probe + slow-path fallback on every redis.call. Add luaHashAlreadyLoaded / luaSetAlreadyLoaded helpers that report whether a state is populated AND loaded, regardless of exists. Each of cmdHGet / cmdHExists / cmdSIsMember checks the loaded helper first (short-circuit to slow path, which returns immediately when c.hashes / c.sets has a loaded entry) before falling back to cachedType. Preserves the High-severity correctness guards for DEL / RENAME / SET-type-change (those still flow through cachedType); just trims the redundant storage probe on the repeated-miss scenario BullMQ- style scripts tend to produce when polling queue state. Test: TestLua_HGET_RepeatedMissReusesLoadedState pins correctness for 4 successive HGET misses on the same key (returns nil|nil|nil|nil). The seek-count saving itself is pprof-verifiable but not unit-testable cheaply; the guard is what matters here.
bootjp
added a commit
that referenced
this pull request
Apr 21, 2026
Addresses two copilot follow-up findings on PR #568. 1. Arity: cmdZScore's `len(args) < 2` accepted extra arguments silently (e.g. `redis.call("ZSCORE", key, member, extra)`), diverging from Redis' arity enforcement. Tighten to `!= 2` and introduce a named zscoreArgCount constant so the //nolint:mnd suppression can come off. 2. Fast-path optimisation gap: cachedType only reports "cached" when a loaded typed value exists. A zset loaded and found MISSING (loaded=true, exists=false -- e.g. a prior ZRANGE miss) was not considered cached, so subsequent ZSCOREs on the same key re-ran the wide-column probe on every call. Add luaZSetAlreadyLoaded helper mirroring luaHashAlreadyLoaded / luaSetAlreadyLoaded from the PR #567 follow-up; cmdZScore now short-circuits to the slow path whenever a loaded state exists. Test: TestLua_ZSCORE_ArityTooMany pins the new arity rejection.
bootjp
added a commit
that referenced
this pull request
Apr 21, 2026
BullMQ and similar Lua-heavy workloads call ZSCORE for delay-queue position checks and job-existence checks; the legacy path went through zsetState (~8-seek keyTypeAt + full zset state init) before a direct GetAt on ZSetMemberKey. Reuse PR #565 / #567's pattern for the single-member ZSET case: - adapter/redis_compat_commands.go: new zsetMemberFastScore helper. Probes store.ZSetMemberKey directly. Probe-first-then-guard ordering matches the post-PR #565 helpers so missing / legacy- blob / wrong-type workloads do not pay the priority-guard seek. Narrow-scope priority guard covers the redisStrKey dual-encoding corruption case only, same scope as hashFieldFastLookup. - adapter/redis_lua_context.go: cmdZScore rewrite. * Enforce exact arity (zscoreArgCount = 2) -- extras rejected, not silently ignored. * luaZSetAlreadyLoaded helper mirrors luaHashAlreadyLoaded / luaSetAlreadyLoaded (from PR #567 follow-up): a loaded zset state (even exists=false) short-circuits to the slow path so repeated ZSCORE on the same missing key does not re-probe. * cachedType defer: prior ZADD / ZREM in this Eval, explicit DEL tombstone, or type change via SET all go through the slow path so zsetState -> keyType -> cachedType produces the correct WRONGTYPE / nil / cached-score answer. * Fast path via zsetMemberFastScore when both guards pass. * zscoreFromSlowPath helper factored out to keep the function body under the cyclomatic-complexity cap. Per-call effect on BullMQ position checks: Before: ~8 seeks (keyTypeAt) + scan / GetAt inside zsetState + GetAt inside memberScore After (fast-path hit): 3 seeks -- ZSetMemberKey GetAt + redisStrKey ExistsAt guard + TTL probe Falls back to the legacy flow on miss so legacy-blob zsets and WRONGTYPE paths retain their prior behaviour. Tests: 10 new ZSCORE cases in redis_lua_collection_fastpath_test.go: fast-path hit / in-script ZADD visible / miss / unknown member / WRONGTYPE / TTL-expired / SET-then-ZSCORE (WRONGTYPE via cachedType guard) / DEL-then-ZSCORE (nil via cachedType guard) / arity too few (1 arg rejected) / arity too many (3 args rejected).
bootjp
added a commit
that referenced
this pull request
Apr 21, 2026
BullMQ and similar Lua-heavy workloads call ZSCORE for delay-queue position checks and job-existence checks; the legacy path went through zsetState (~8-seek keyTypeAt + full zset state init) before a direct GetAt on ZSetMemberKey. Reuse PR #565 / #567's pattern for the single-member ZSET case: - adapter/redis_compat_commands.go: new zsetMemberFastScore helper. Probes store.ZSetMemberKey directly. Probe-first-then-guard ordering matches the post-PR #565 helpers so missing / legacy- blob / wrong-type workloads do not pay the priority-guard seek. Narrow-scope priority guard covers the redisStrKey dual-encoding corruption case only, same scope as hashFieldFastLookup. - adapter/redis_lua_context.go: cmdZScore rewrite. * Enforce exact arity (zscoreArgCount = 2) -- extras rejected, not silently ignored. * luaZSetAlreadyLoaded helper mirrors luaHashAlreadyLoaded / luaSetAlreadyLoaded (from PR #567 follow-up): a loaded zset state (even exists=false) short-circuits to the slow path so repeated ZSCORE on the same missing key does not re-probe. * cachedType defer: prior ZADD / ZREM in this Eval, explicit DEL tombstone, or type change via SET all go through the slow path so zsetState -> keyType -> cachedType produces the correct WRONGTYPE / nil / cached-score answer. * Fast path via zsetMemberFastScore when both guards pass. * zscoreFromSlowPath helper factored out to keep the function body under the cyclomatic-complexity cap. Per-call effect on BullMQ position checks: Before: ~8 seeks (keyTypeAt) + scan / GetAt inside zsetState + GetAt inside memberScore After (fast-path hit): 3 seeks -- ZSetMemberKey GetAt + redisStrKey ExistsAt guard + TTL probe Falls back to the legacy flow on miss so legacy-blob zsets and WRONGTYPE paths retain their prior behaviour. Tests: 10 new ZSCORE cases in redis_lua_collection_fastpath_test.go: fast-path hit / in-script ZADD visible / miss / unknown member / WRONGTYPE / TTL-expired / SET-then-ZSCORE (WRONGTYPE via cachedType guard) / DEL-then-ZSCORE (nil via cachedType guard) / arity too few (1 arg rejected) / arity too many (3 args rejected).
3 tasks
bootjp
added a commit
that referenced
this pull request
Apr 21, 2026
Production pprof after #565 / #567 / #568 showed `cmdZRangeByScore` + `cmdZRangeByScoreAsc` holding 40 active goroutines (top Lua-path signal), driven by BullMQ delay-queue polls. The legacy path loaded ALL zset members via zsetState -> ensureZSetLoaded before filtering by score bounds -- O(N) GetAts per ZRANGEBYSCORE, so an N=1000 delay queue cost several seconds per call and was the dominant residual after the previous keyTypeAt fast-paths landed. Replace the load-then-filter flow with a bounded scan over the pre-sorted score index: - adapter/redis_compat_commands.go adds zsetRangeByScoreFast which translates [startKey, endKey) byte bounds + offset / limit / scoreFilter into a ScanAt / ReverseScanAt on ZSetScoreRangeScanPrefix. Legacy-blob zsets, string-priority corruption, and empty-result-but-zset-is-missing cases bail out with hit=false so the slow path's WRONGTYPE / disambiguation semantics are preserved. zsetRangeEmptyFastResult checks ZSetMetaKey to distinguish "empty range on a live zset" from "no zset here". Split into zsetFastPathEligible, zsetFastScanLimit, zsetScoreScan, decodeZSetScoreRange, finalizeZSetFastRange helpers so the main entrypoint stays under the cyclomatic- complexity cap. - adapter/redis_lua_context.go cmdZRangeByScore consults luaZSetAlreadyLoaded + cachedType (mirrors PR #567 / #568 safety guards) before entering the fast path. The legacy full-load path lives in cmdZRangeByScoreSlow and still runs for in-script mutations / legacy-blob / type-change cases. zsetScoreScanBounds maps zScoreBound (-Inf / value-inc / value-exc / +Inf) to the byte range; exclusive edges are handled post-scan by scoreInRange. Per-call effect on BullMQ-style delay-queue polls with LIMIT: Before: O(N) member GetAts + in-process filter After (fast-path hit): 1 legacy-blob ExistsAt + a single bounded ScanAt (N_range <= offset + limit, clamped by maxWideScanLimit) + 1 priority-guard ExistsAt + 1 TTL probe. Tests: 10 new cases in redis_lua_collection_fastpath_test.go -- fast-path hit / WITHSCORES / LIMIT offset / exclusive bounds / empty range / missing key / WRONGTYPE / in-script ZADD visibility / ZREVRANGEBYSCORE / TTL-expired. All pass under -race along with the existing adapter suite.
bootjp
added a commit
that referenced
this pull request
Apr 21, 2026
## Summary Phase B of the ZSet fast-path plan. Production pprof after PRs #565 / #567 / #568 showed `cmdZRangeByScore` + `cmdZRangeByScoreAsc` holding **40 active goroutines** — the top residual Lua-path signal — driven by BullMQ delay-queue polls. The legacy path loaded ALL zset members via `zsetState` → `ensureZSetLoaded` before filtering by score bounds: O(N) `GetAt`s per `ZRANGEBYSCORE`, so an N=1000 delay queue cost several seconds per call. Replace load-then-filter with a **bounded scan over the pre-sorted score index** (`!zs|scr|<user>|<sortable-score>|<member>`). ### Changes **`adapter/redis_compat_commands.go`** — new `zsetRangeByScoreFast` helper plus small utilities: - Legacy-blob zsets, string-priority corruption, and empty-result-but-zset-missing cases return `hit=false` so the slow path's WRONGTYPE / disambiguation semantics remain intact. - `zsetRangeEmptyFastResult` checks `ZSetMetaKey` to separate "empty range on a live zset" from "no zset here". - Split into `zsetFastPathEligible` / `zsetFastScanLimit` / `zsetScoreScan` / `decodeZSetScoreRange` / `finalizeZSetFastRange` to stay under the cyclomatic-complexity cap. **`adapter/redis_lua_context.go`** — `cmdZRangeByScore` gated on `luaZSetAlreadyLoaded` + `cachedType` (mirrors PR #567 / #568 safety guards): - In-script ZADD / ZREM / DEL / SET / type-change still go through the legacy full-load path (`cmdZRangeByScoreSlow`). - `zsetScoreScanBounds` maps `zScoreBound` (-Inf / value-inc / value-exc / +Inf) to a byte range; exclusive edges are handled post-scan by `scoreInRange`. ### Per-call effect on BullMQ-style delay-queue polls | Scenario | Before | After (fast-path hit) | |---|---|---| | `ZRANGEBYSCORE key 0 now LIMIT 0 10` over N=1000 zset | ~1000 member `GetAt`s | 1 legacy-blob `ExistsAt` + 1 bounded `ScanAt` (≤10 rows) + 1 priority-guard `ExistsAt` + 1 TTL probe | | Empty range | ~1000 `GetAt`s | 1 legacy-blob + 1 `ZSetMetaKey` + 1 TTL | | In-script `ZADD` then `ZRANGEBYSCORE` | unchanged | unchanged (slow path via `luaZSetAlreadyLoaded`) | ## Test plan - [x] `go test -race -short ./adapter/...` passes - [x] 10 new cases in `adapter/redis_lua_collection_fastpath_test.go`: - fast-path hit (ascending) - WITHSCORES - LIMIT + offset - exclusive bounds `(min (max` - empty range on live zset - missing key - WRONGTYPE (key holds a string) - in-script `ZADD` visible to subsequent `ZRANGEBYSCORE` - `ZREVRANGEBYSCORE` (reverse iteration) - TTL-expired zset - [ ] Production: deploy and verify `cmdZRangeByScore` active goroutines drop from ~20+20 back toward the keyTypeAt-style residual. Expect p50 redis.call latency inside Lua to fall from the current 3s toward ms-scale as delay-queue polls stop pulling the whole zset. ## Follow-ups (out of scope) - `ZRANGE` (by rank) streaming — needs the same pattern with positional iteration. - `ZPOPMIN` — writes; needs the streaming scan plus the delete-member transaction plumbing. - `HGETALL` / `HMGET` — separate wide-column prefix scans. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Optimized sorted-set range-by-score fast path for faster, paginated queries with correct inclusive/exclusive bound handling. * **Bug Fixes** * Safer fallback to full-path when results may be incomplete; fixed empty-range, truncation, type-mismatch and TTL/expiration behaviors; negative LIMIT offsets now rejected and negative limits treated as “no limit.” * **Tests** * Added extensive Lua-scripted tests covering range reads, WITHSCORES, LIMIT semantics, exclusive bounds, in-script mutations, TTL, and regressions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
Follow-up to PR #565 (top-level HGET / HEXISTS / SISMEMBER fast-paths). BullMQ and similar Lua-heavy workloads never saw the benefit because
luaScriptContext.cmdHGetetc. still went throughhashState/setState, which paid the fullkeyTypeAtprobe (~8 seeks post-#563) before loading the value.Effect on BullMQ-style scripts
A typical BullMQ script (
moveToActive,moveToCompleted, etc.) touches 5–10 keys. Before this PR, each key cost ~8 seeks just for type classification, i.e. 40–80 wasted seeks per script invocation. After: 1–2 seeks per fast-path hit.Changes
Port #565's pattern verbatim into the Lua command handlers:
cmdHGethashFieldFastLookupcmdHExistshashFieldFastExistscmdSIsMembersetMemberFastExistsEach handler still consults
c.hashes/c.setsfirst, so in-script mutations (HSETfollowed byHGETin the sameEval) stay visible — the fast path only fires when the key has not yet been loaded into the script-local cache. Misses fall through to the legacyhashState/setStateloader so legacy-blob encodings,WRONGTYPE, and TTL-expired keys behave identically to before.Small helpers factored out (
hgetFromHashState,hexistsFromHashState,sismemberFromSetState) to keep each command body under the cyclomatic-complexity cap.Test plan
go test -race -short ./adapter/...passesadapter/redis_lua_collection_fastpath_test.godrives each command through a realredis.Eval:Depends on
hashFieldFastLookup,hashFieldFastExists,setMemberFastExists. This branch is based on perf(redis): fast-path HGET / HEXISTS / SISMEMBER wide-column lookups #565; once perf(redis): fast-path HGET / HEXISTS / SISMEMBER wide-column lookups #565 merges, rebase to main will be clean.