Run benchmarks nightly and cover the LLAPI (search) path by gabsow · Pull Request #1629 · RedisJSON/RedisJSON · GitHub

Why

Two gaps, both surfaced while looking into RED-213492:

Nothing measured performance ahead of a release. The benchmark suite only ran on demand, gated on the run-benchmark PR label (benchmark-trigger.yml).
The shared C API was not benchmarked at all. All 44 existing specs drive JSON.SET / JSON.GET / JSON.ARRAPPEND / JSON.NUMINCRBY / JSON.NUMMULTBY. The surface RediSearch actually consumes — getAt / getArray — had no coverage, and it is not reachable over the protocol, being an in-process C ABI. So a change in the cost of an indexed read of a packed array was invisible to CI.

What changed

Nightly perf run. A perf-nightly job in event-nightly.yml calling the existing benchmark-runner.yml, included in test-summary needs and in job-failures so a regression reaches the same Slack report as the rest of the nightly.

LLAPI benchmark coverage. The LLAPI test consumer module already exposes the C API as LLAPI.* commands, so benchmark-flow.yml now builds it and passes a second --module_path (redisbench-admin's --module_path is action="append", and --required-module ReJSON is a presence check, so the extra module is fine). Four specs:

spec	what it measures
llapi_getat_scan_packed_f32_320 / _1280	a packed f32 array read element by element via getAt(), the pattern a vector read uses — the pair exists so per-element cost can be compared across a 4x size jump
llapi_getat_last_packed_f32_1280	one indexed read, at the last index
llapi_getarray_packed_f32_1280	the bulk buffer path, as a baseline

The scan pair is the point: a per-element cost that grows with array length is the signal, and no absolute rps number gives you that.

LLAPI.GETAT_SCAN is new in the test module (tests/pytest/llapi_test_module/module.c), with flow-test coverage in test_llapi.py for both the happy path and the not-an-array error path.

Vectors are built by an init_commands EVAL rather than a checked-in dataset. The values are chosen to be f32-exact but outside f16 range, so the array packs as f32 — with the obvious 0.25-step values it packs as f16 instead, which is not representative of a vector workload.

One builder for the test module. The .so build recipe moves out of tests.sh into llapi_test_module/build.sh, so the flow tests and the benchmark flow share it and cannot drift. It prints the path on stdout and sends everything else to stderr so callers can capture it.

Validation

test_llapi.py: 20/20 pass (RLTest, module built from this branch, Redis 8.2), including the new testLLAPIGetAtScan and the extended testLLAPIErrorsGetAt.
The four specs' init_commands were run verbatim against a live server: each returns OK, and LLAPI.GETARRAY confirms both arrays report type f32 with the expected lengths (320 / 1280).
Module compiles clean under -Wall; all touched workflow files parse as YAML; bash -n clean on tests.sh.

Notes for the reviewer

This adds EC2 spend: the benchmarks provision hosts via terraform and the run step has a 240-minute timeout, now nightly across 3 matrix members. If that is too much, the knob is benchmark_glob to run a subset nightly.
Loading the LLAPI consumer module on every benchmark run slightly changes the benchmarked environment for the existing specs. It only registers commands and intercepts nothing, so historical baselines should be unaffected — worth a second opinion.
Not in this PR: RC/release-tag coverage. event-tag.yml does not call the benchmark runner, so tagged builds still run no benchmarks; pushes to a release branch do pick up perf-nightly via the nightly push trigger.

Generated by Claude Code

Note

Medium Risk
Nightly AWS benchmark runs add EC2 cost and a 240-minute timeout; loading the extra LLAPI module on all benchmark hosts slightly changes the environment for existing specs. No production or auth code is modified.

Overview
Schedules the existing benchmark runner as a nightly perf-nightly job and folds its failures into the Slack test summary.

Adds four specs that drive the shared C API (getAt / getArray) through the LLAPI test module on packed f32 arrays (320 vs 1280), matching the RediSearch vector-read pattern. benchmark-flow.yml now builds llapi_test.so and loads it beside RedisJSON on every remote run.

The test module gains LLAPI.GETAT_SCAN (with pytest coverage). Its compile recipe moves into a shared build.sh used by both flow tests and CI so the two cannot drift.

Reviewed by Cursor Bugbot for commit 7e487d9. Bugbot is set up for automated code reviews on this repo. Configure here.

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

The benchmark suite only ran on demand, via the `run-benchmark` PR label, so nothing measured performance ahead of a release. Add a `perf-nightly` job to the nightly event that calls the existing benchmark runner, and include it in the test summary so regressions reach the same Slack report as the rest. None of the existing 44 specs touched the shared C API (the LLAPI): they drive JSON.SET/GET/ARRAPPEND/NUMINCRBY/NUMMULTBY only. The surface RediSearch actually consumes - getAt/getArray - was therefore unmeasured, and it is not reachable over the protocol at all, being an in-process C ABI. The LLAPI test consumer module already exposes it as LLAPI.* commands, so load that module alongside the module under test and benchmark through it: - llapi_getat_scan_packed_f32_{320,1280}: reads a packed f32 array element by element via getAt(), the pattern a vector read uses. The pair exists so the per-element cost can be compared across a 4x size jump - a per-element cost that grows with array length is the signal, which no absolute rps number gives you. - llapi_getat_last_packed_f32_1280: one indexed read at the last index. - llapi_getarray_packed_f32_1280: the bulk buffer path, as a baseline. LLAPI.GETAT_SCAN is new in the test module; the vectors are built by an init_commands EVAL, whose values are chosen to be f32-exact but outside f16 range so the array packs as f32 rather than a narrower type. The .so build recipe moves out of tests.sh into llapi_test_module/build.sh so the flow tests and the benchmark flow share one builder and cannot drift.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.97%. Comparing base (28f4bb1) to head (7e487d9).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1629   +/-   ##
=======================================
  Coverage   85.97%   85.97%           
=======================================
  Files          15       15           
  Lines        5305     5305           
=======================================
  Hits         4561     4561           
  Misses        744      744

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7e487d9. Configure here.

Partial scan reported as success

Medium Severity

LLAPI.GETAT_SCAN breaks out of the getAt loop on failure but still replies with a successful integer count. Unlike LLAPI.GETAT, which returns an error when getAt fails, a mid-scan failure looks like a normal reply to redis-benchmark, so nightly LLAPI specs can keep reporting healthy ops/sec while indexed reads are broken.

Reviewed by Cursor Bugbot for commit 7e487d9. Configure here.

Perf failures skip Slack alert

Medium Severity

perf-nightly is wired into test-summary job-failures so nightly Slack should report perf job failures, but the benchmark run step uses continue-on-error: true, so a failed redisbench-admin run still leaves the job successful. Redisbench, module-load, and LLAPI spec failures therefore do not appear in the Slack failure list the PR aims to extend.

Additional Locations (1)

.github/workflows/benchmark-flow.yml#L84-L88

Reviewed by Cursor Bugbot for commit 7e487d9. Configure here.

gabsow requested a review from AvivDavid23 August 20, 2026 17:28

gabsow force-pushed the claude/redis-slack-archive-0pw1o9 branch from 80ba510 to 7e487d9 Compare August 20, 2026 17:30

cursor Bot reviewed Aug 20, 2026

View reviewed changes

gabsow mentioned this pull request Aug 20, 2026

Index the backing slice in IValue::get_index instead of iter().nth() #1630

Closed

+                  japi->freeJson(buf);
+                  japi->freeIter(it);
+                  RedisModule_ReplyWithLongLong(ctx, read);
+                  return REDISMODULE_OK;

                     redis-ref: ${{ needs.prepare-values.outputs.redis-ref }}
                     send-slack-message: ${{ github.event_name == 'schedule' && (github.ref == 'refs/heads/master' || github.ref == 'refs/heads/main') || (github.event_name == 'workflow_dispatch' && inputs.send-slack-message == true) }}
-                    job-failures: ${{ needs.build-linux-x64.result == 'failure' && 'build-linux-x64 ' || '' }}${{ needs.build-linux-arm64.result == 'failure' && 'build-linux-arm64 ' || '' }}${{ needs.macos.result == 'failure' && 'macos ' || '' }}${{ needs.linux-valgrind.result == 'failure' && 'linux-valgrind ' || '' }}${{ needs.linux-sanitizer.result == 'failure' && 'linux-sanitizer ' || '' }}${{ needs.random-traffic.result == 'failure' && 'random-traffic' || '' }}
+                    job-failures: ${{ needs.build-linux-x64.result == 'failure' && 'build-linux-x64 ' || '' }}${{ needs.build-linux-arm64.result == 'failure' && 'build-linux-arm64 ' || '' }}${{ needs.macos.result == 'failure' && 'macos ' || '' }}${{ needs.linux-valgrind.result == 'failure' && 'linux-valgrind ' || '' }}${{ needs.linux-sanitizer.result == 'failure' && 'linux-sanitizer ' || '' }}${{ needs.random-traffic.result == 'failure' && 'random-traffic' || '' }}${{ needs.perf-nightly.result == 'failure' && ' perf-nightly' || '' }}

Run benchmarks nightly and cover the LLAPI (search) path by gabsow · Pull Request #1629 · RedisJSON/RedisJSON · GitHub

Repository navigation

Run benchmarks nightly and cover the LLAPI (search) path - #1629

Run benchmarks nightly and cover the LLAPI (search) path#1629
gabsow wants to merge 1 commit into
masterfrom
claude/redis-slack-archive-0pw1o9

gabsow commented Aug 20, 2026 •
edited by cursor Bot
Loading

Uh oh!

Uh oh!

CLAassistant commented Aug 20, 2026

Uh oh!

codecov Bot commented Aug 20, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Aug 20, 2026

Uh oh!

cursor Bot Aug 20, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Run benchmarks nightly and cover the LLAPI (search) path by gabsow · Pull Request #1629 · RedisJSON/RedisJSON · GitHub

Repository navigation

Run benchmarks nightly and cover the LLAPI (search) path - #1629

Run benchmarks nightly and cover the LLAPI (search) path#1629gabsow wants to merge 1 commit intomasterRedisJSON/RedisJSON:masterfrom claude/redis-slack-archive-0pw1o9RedisJSON/RedisJSON:claude/redis-slack-archive-0pw1o9Copy head branch name to clipboard

Conversation

gabsow commented Aug 20, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What changed

Validation

Notes for the reviewer

Uh oh!

CLAassistant commented Aug 20, 2026

Uh oh!

codecov Bot commented Aug 20, 2026

Codecov Report

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 20, 2026

Choose a reason for hiding this comment

Partial scan reported as success

Uh oh!

cursor Bot Aug 20, 2026

Choose a reason for hiding this comment

Perf failures skip Slack alert

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Run benchmarks nightly and cover the LLAPI (search) path#1629
gabsow wants to merge 1 commit into
masterfrom
claude/redis-slack-archive-0pw1o9

gabsow commented Aug 20, 2026 •
edited by cursor Bot
Loading