mirror of
https://github.com/redis/redis.git
synced 2026-07-15 04:53:26 -04:00
44a6a9f5ef
Some checks failed
CI / test-ubuntu-latest (push) Has been cancelled
CI / test-sanitizer-address (push) Has been cancelled
CI / build-debian-old (push) Has been cancelled
CI / build-macos-latest (push) Has been cancelled
CI / build-32bit (push) Has been cancelled
CI / build-libc-malloc (push) Has been cancelled
CI / build-centos-jemalloc (push) Has been cancelled
CI / build-old-chain-jemalloc (push) Has been cancelled
Codecov / code-coverage (push) Has been cancelled
External Server Tests / test-external-standalone (push) Has been cancelled
External Server Tests / test-external-cluster (push) Has been cancelled
External Server Tests / test-external-nodebug (push) Has been cancelled
Reply-schemas linter / reply-schemas-linter (push) Has been cancelled
Spellcheck / Spellcheck (push) Has been cancelled
## What this PR does
Adds a new module API, `RedisModule_AddPostNotificationJobForKey`,
alongside
the existing `RedisModule_AddPostNotificationJob`. Both schedule work to
run
after a keyspace-notification (KSN) handler returns (so the handler
stays free
of write side-effects), but the new API binds each job to a specific key
and
differs from the existing one in three ways:
| | `AddPostNotificationJob` (existing) | `AddPostNotificationJobForKey`
(new) |
|---|---|---|
| **Callback** | `(ctx, pd)` | `(ctx, key, pd)` — receives the bound
key. |
| **Firing schedule** | Once, at the end of the outermost execution unit
(`firePostExecutionUnitJobs`). | At the tail of **every `call()`** — so
each MULTI/EXEC and EVAL/FCALL sub-command is observed in turn —
**plus** during AOF replay after each replayed command, **plus** the
end-of-execution-unit drain (for standalone commands). |
| **When allowed** | Refused while `server.loading` or on a read-only
replica. | Permitted during AOF replay and on replicas (see below).
Refused if called **outside a KSN handler**. |
Motivation: RED-197766. A module needs to react to each step of an
`HSET` + `HEXPIRE` sequence on the same hash inside a `MULTI/EXEC` block
—
the existing API can't deliver that because it only fires once at the
end of
EXEC.
## Design: one queue, distinguished by the bound key
Per-key jobs share the existing post-notification queue
(`modulePostExecUnitJobs`) rather than living in a separate one. A
queued job
(`RedisModulePostExecUnitJob`) carries an owned `key` reference and a
callback
union:
- **Regular job** (`RM_AddPostNotificationJob`): `key == NULL`,
`cb.callback`
is used. Fires once at the end of the outermost execution unit and may
write
to the keyspace.
- **Per-key job** (`RM_AddPostNotificationJobForKey`): `key != NULL`,
`cb.key_callback` is used and receives the bound key. Fires between
sub-commands and during AOF replay, and may **not** write to the
keyspace.
`key != NULL` is the sole discriminator — no separate type or flag.
### Firing sites
`firePerKeyJobsBetweenSubcommands()` drains only the per-key jobs (those
with
`key != NULL`), leaving regular jobs for the end-of-unit drain. It is
invoked
at three places, all off the universal `afterCommand` hot path so
standalone
commands that never register a per-key job pay nothing:
1. `execCommand()` (`multi.c`) — after each MULTI/EXEC sub-command's
`call()`,
so per-key effects are observable between sibling sub-commands.
2. `scriptCall()` (`script.c`) — after each EVAL/FCALL sub-command's
`call()`.
3. `loadSingleAppendOnlyFile()` (`aof.c`) — after each replayed single
command,
plus a final `firePostExecutionUnitJobs()` at the `cleanup:` path so a
partially-applied command can't leave stragglers. EXEC's sub-commands go
through `call()` during replay and are covered by the per-`call()`
drain.
Per-key jobs registered by a standalone command (no MULTI/EXEC, no
script) are
drained by the existing end-of-execution-unit
`firePostExecutionUnitJobs()`,
which walks the single queue in submission order.
These drain sites are gated by an inline hint
`server.fire_keyed_jobs_between_subcommands`, armed when a per-key job
is
enqueued and cleared after a drain, so the no-jobs case is a single
well-predicted branch on a hot `server` field rather than a cross-TU
call.
## AOF replay & replica support
Unlike the regular API, `RM_AddPostNotificationJobForKey` does **not**
mirror
the `server.loading || repl_slave_ro` guard. The canonical consumer
attaches
**module key metadata** (`RM_SetKeyMeta` / `RM_GetKeyMeta`) to keys it
observes
via KSN. That metadata is neither stored in the AOF nor replicated; it
must be
rebuilt by re-running the same callback over the same KSN stream on
every
instance that applies the command — master, replica, and a process
replaying
its own AOF at startup. The per-key API is therefore permitted in all
those
contexts; the only registration guard is that it must be called from
inside a
KSN dispatch.
**RDB load is intentionally outside this pattern.** RDB load decodes
keys
directly without running commands, so no KSN fires and per-key callbacks
do not
run. A test (`perkey-rdb`) pins this boundary.
## Runtime contract enforcement
A per-key callback MUST only touch non-replicated, non-AOF-persisted
state
(canonically: module key metadata). Writing back into the keyspace would
amplify the AOF on replay and diverge a replica from its master. This is
now
**enforced at runtime**, not merely documented —
`server.firing_keyed_post_notif_jobs`
is set for the duration of each per-key callback, and while it is set:
- `RM_Call(...)` is refused: returns `NULL` with `errno == EINVAL`, or a
`-ERR` call-reply when `CALL_REPLIES_AS_ERRORS` is requested.
- `RM_NotifyKeyspaceEvent` / `RM_NotifyKeyspaceEventWithSubkeys` are
refused
(return `REDISMODULE_ERR`) — a nested notification could enqueue further
per-key jobs mid-drain.
Each refusal is logged once per drain (`LL_WARNING`, deduplicated via
`keyedPostNotifRMCallWarned` / `keyedPostNotifNotifyWarned`). The
canonical
`RM_SetKeyMeta` path is unaffected (metadata does not propagate).
## Implementation summary
- New per-key callback type `RedisModulePostNotifyJobPerKeyFunc`
`(ctx, key, pd)`. The existing `RedisModulePostNotificationJobFunc` is
renamed to `RedisModulePostNotifyJobFunc` with a backward-compatible
typedef
alias so existing module sources keep compiling.
- `RedisModulePostExecUnitJob` gains an owned `key` field and a callback
union.
- `firePerKeyJobsBetweenSubcommands()` drains per-key jobs;
`firePostExecutionUnitJobs()` drains the whole queue at end of unit. A
shared
`executePostExecUnitJob()` helper runs and frees a single job (and
decrefs
the owned key for per-key jobs).
- New `server` fields:
- `firing_keyed_post_notif_jobs` — set while a per-key callback runs;
the
no-write guard backing the `RM_Call` / `RM_NotifyKeyspaceEvent`
refusals.
- `fire_keyed_jobs_between_subcommands` — fast-path hint gating the
explicit
drains in `multi.c` / `script.c` / `aof.c`.
- `in_keyspace_notification` — counter, incremented around the dispatch
loop
in `moduleNotifyKeyspaceEvent`. Defines the scope from which
`RM_AddPostNotificationJobForKey` may be called (nested notifications
nest
cleanly). Calling the API outside this scope returns `REDISMODULE_ERR`.
- `moduleUnregisterPostNotificationJobs()` drops any queued jobs
belonging to a
module being unloaded (wired into `moduleUnregisterCleanup`).
- Because each KSN dispatch is single-key by construction, multi-key
commands
like `MSET`/`MGET` work transparently: they emit one
`notifyKeyspaceEvent`
per key, so the handler can register one per-key job per affected key.
## Testing
A single test module, `pkmeta`
(`tests/modules/postnotifications_perkey_metadata.c`), uses the
canonical
pattern: a KSN handler enqueues a per-key job, and the job attaches
module key
metadata via `RM_SetKeyMeta`. Metadata is neither AOF-persisted nor
replicated,
so its presence after a reload / propagation is direct evidence the
callback
re-ran on that instance. Module-internal counters (fire count, fire log,
blocked-`RM_Call` count, blocked-notification count), kept out of the
keyspace
on purpose, carry the load-bearing assertions.
All tests live in one suite,
`tests/unit/moduleapi/postnotifications_perkey.tcl`:
| Group | Tests | What they pin |
|---|---|---|
| `perkey-aof` | single command rebuilds metadata via AOF reload;
MULTI/EXEC fires once per sub-command; HSET+HEXPIRE in MULTI/EXEC fires
twice (each run for both `debug loadaof` and full restart) | One drain
per replayed command/sub-command during AOF replay; the original
RED-197766 scenario end-to-end. |
| `perkey-rdb` | RDB-only restart does NOT rebuild metadata | Pins that
RDB load is outside the firing pattern. |
| `perkey-aof-replica` | AOF replay on a replica at startup rebuilds
metadata | The carve-out removing the `repl_slave_ro` guard. |
| `perkey-repl` | replica builds metadata from a propagated single
command; replica fires per sub-command for propagated MULTI/EXEC | Both
sides run the per-key job locally; no metadata crosses the replication
stream. |
| `perkey-misuse` | registration refused outside a KSN handler | The
`in_keyspace_notification` scope guard. |
| `perkey-order` | fires once per key in submission order across
MULTI/EXEC; fires between sub-commands inside MULTI/EXEC; fires between
commands inside a script (EVAL); multi-key command fires one job per key
| Ordering and per-sub-command granularity for MULTI/EXEC, scripts, and
multi-key commands. |
| `perkey-contract` | RM_Call refused; refusal repeats per firing and
never writes; RM_NotifyKeyspaceEvent refused; notification refusal
repeats per firing | Runtime enforcement of the no-write contract. |
| `perkey-expire` | lazy expire on read fires the per-key job; active
expire (cron) fires it without a read | Expire-path coverage. |
| `perkey-ptr-safety` | SMOVE first metadata attach must not UAF the
source set | Owned-key lifetime safety. |
The suite is registered in `runtest-moduleapi` and the module in
`tests/modules/Makefile`.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Touches module notification timing, MULTI/EXEC, scripting, and AOF
replay—areas where ordering bugs could affect replication consistency,
though keyspace writes from callbacks are blocked.
>
> **Overview**
> Adds **`RedisModule_AddPostNotificationJobForKey`**, a sibling to
`AddPostNotificationJob` that binds deferred work to a specific key from
a keyspace-notification handler. Per-key jobs share the existing
post-notification queue (`key != NULL`); regular jobs still drain only
at the end of the execution unit, while per-key jobs also run after each
sub-command in **MULTI/EXEC**, **EVAL/FCALL**, and **single-command AOF
replay** (gated by `fire_keyed_jobs_between_subcommands` so the common
path stays cheap).
>
> Runtime contract: per-key callbacks may update local module state
(e.g. **`RM_SetKeyMeta`**) but **`RM_Call`** and
**`RM_NotifyKeyspaceEvent`** are refused while they run; registration
outside KSN dispatch fails via **`in_keyspace_notification`**. Unlike
the regular API, per-key registration is allowed during AOF replay and
on replicas so metadata can be rebuilt from the same KSN stream. Module
unload now drops queued jobs for that module; AOF load **`cleanup`**
flushes remaining regular jobs.
>
> New **`pkmeta`** test module and **`postnotifications_perkey`** suite
cover ordering, AOF/replica rebuild, contract violations, and
expire/evict edge cases.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
022ad9d078. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Filipe Oliveira (Redis) <filipe@redis.com>
Co-authored-by: Moti Cohen <moticless@gmail.com>
70 lines
2.3 KiB
Bash
Executable file
70 lines
2.3 KiB
Bash
Executable file
#!/bin/sh
|
|
# Raise open files limit (macOS default 256 is too low for tests).
|
|
OPEN_FILE_LIMIT=$(ulimit -n 2>/dev/null)
|
|
if [ -n "$OPEN_FILE_LIMIT" ] && [ "$OPEN_FILE_LIMIT" != "unlimited" ] && [ "$OPEN_FILE_LIMIT" -lt 1024 ]; then
|
|
ulimit -n 1024 2>/dev/null || true
|
|
fi
|
|
TCL_VERSIONS="8.5 8.6 8.7 9.0"
|
|
TCLSH=""
|
|
[ -z "$MAKE" ] && MAKE=make
|
|
|
|
for VERSION in $TCL_VERSIONS; do
|
|
TCL=`which tclsh$VERSION 2>/dev/null` && TCLSH=$TCL
|
|
done
|
|
|
|
if [ -z $TCLSH ]
|
|
then
|
|
echo "You need tcl 8.5 or newer in order to run the Redis ModuleApi test"
|
|
exit 1
|
|
fi
|
|
|
|
$MAKE -C tests/modules && \
|
|
$TCLSH tests/test_helper.tcl \
|
|
--single unit/moduleapi/commandfilter \
|
|
--single unit/moduleapi/basics \
|
|
--single unit/moduleapi/fork \
|
|
--single unit/moduleapi/testrdb \
|
|
--single unit/moduleapi/infotest \
|
|
--single unit/moduleapi/moduleconfigs \
|
|
--single unit/moduleapi/infra \
|
|
--single unit/moduleapi/propagate \
|
|
--single unit/moduleapi/hooks \
|
|
--single unit/moduleapi/misc \
|
|
--single unit/moduleapi/blockonkeys \
|
|
--single unit/moduleapi/blockonbackground \
|
|
--single unit/moduleapi/scan \
|
|
--single unit/moduleapi/datatype \
|
|
--single unit/moduleapi/auth \
|
|
--single unit/moduleapi/keyspace_events \
|
|
--single unit/moduleapi/blockedclient \
|
|
--single unit/moduleapi/getchannels \
|
|
--single unit/moduleapi/getkeys \
|
|
--single unit/moduleapi/test_lazyfree \
|
|
--single unit/moduleapi/defrag \
|
|
--single unit/moduleapi/keyspecs \
|
|
--single unit/moduleapi/hash \
|
|
--single unit/moduleapi/zset \
|
|
--single unit/moduleapi/list \
|
|
--single unit/moduleapi/stream \
|
|
--single unit/moduleapi/mallocsize \
|
|
--single unit/moduleapi/datatype2 \
|
|
--single unit/moduleapi/cluster \
|
|
--single unit/moduleapi/aclcheck \
|
|
--single unit/moduleapi/subcommands \
|
|
--single unit/moduleapi/reply \
|
|
--single unit/moduleapi/cmdintrospection \
|
|
--single unit/moduleapi/eventloop \
|
|
--single unit/moduleapi/timer \
|
|
--single unit/moduleapi/publish \
|
|
--single unit/moduleapi/usercall \
|
|
--single unit/moduleapi/postnotifications \
|
|
--single unit/moduleapi/postnotifications_perkey \
|
|
--single unit/moduleapi/async_rm_call \
|
|
--single unit/moduleapi/moduleauth \
|
|
--single unit/moduleapi/rdbloadsave \
|
|
--single unit/moduleapi/crash \
|
|
--single unit/moduleapi/internalsecret \
|
|
--single unit/moduleapi/configaccess \
|
|
--single unit/moduleapi/keymeta \
|
|
--single unit/moduleapi/ksn_notify_side_effect \
|
|
"${@}"
|