mirror of
https://github.com/redis/redis.git
synced 2026-07-15 13:08:59 -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>
108 lines
2.5 KiB
Makefile
108 lines
2.5 KiB
Makefile
|
|
# find the OS
|
|
uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not')
|
|
|
|
warning_cflags = -W -Wall -Wno-missing-field-initializers
|
|
ifeq ($(uname_S),Darwin)
|
|
SHOBJ_CFLAGS ?= $(warning_cflags) -dynamic -fno-common -g -ggdb -std=gnu11 -O2
|
|
SHOBJ_LDFLAGS ?= -bundle -undefined dynamic_lookup
|
|
else # Linux, others
|
|
SHOBJ_CFLAGS ?= $(warning_cflags) -fno-common -g -ggdb -std=gnu11 -O2
|
|
SHOBJ_LDFLAGS ?= -shared
|
|
endif
|
|
|
|
CLANG := $(findstring clang,$(shell sh -c '$(CC) --version | head -1'))
|
|
|
|
ifeq ($(SANITIZER),memory)
|
|
ifeq (clang, $(CLANG))
|
|
LD=clang
|
|
MALLOC=libc
|
|
CFLAGS+=-fsanitize=memory -fsanitize-memory-track-origins=2 -fno-sanitize-recover=all -fno-omit-frame-pointer
|
|
LDFLAGS+=-fsanitize=memory
|
|
else
|
|
$(error "MemorySanitizer needs to be compiled and linked with clang. Please use CC=clang")
|
|
endif
|
|
endif
|
|
|
|
|
|
# This is a hack to override the default CC. When running with SANITIZER=memory
|
|
# tough we want to keep the compiler as clang as MSan is not supported for gcc
|
|
ifeq ($(uname_S),Linux)
|
|
ifneq ($(SANITIZER),memory)
|
|
LD = gcc
|
|
CC = gcc
|
|
endif
|
|
endif
|
|
|
|
# OS X 11.x doesn't have /usr/lib/libSystem.dylib and needs an explicit setting.
|
|
ifeq ($(uname_S),Darwin)
|
|
ifeq ("$(wildcard /usr/lib/libSystem.dylib)","")
|
|
LIBS = -L /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib -lsystem
|
|
endif
|
|
endif
|
|
|
|
TEST_MODULES = \
|
|
commandfilter.so \
|
|
basics.so \
|
|
testrdb.so \
|
|
fork.so \
|
|
infotest.so \
|
|
propagate.so \
|
|
misc.so \
|
|
hooks.so \
|
|
blockonkeys.so \
|
|
blockonbackground.so \
|
|
scan.so \
|
|
datatype.so \
|
|
datatype2.so \
|
|
auth.so \
|
|
keyspace_events.so \
|
|
blockedclient.so \
|
|
getkeys.so \
|
|
getchannels.so \
|
|
test_lazyfree.so \
|
|
timer.so \
|
|
defragtest.so \
|
|
keyspecs.so \
|
|
hash.so \
|
|
zset.so \
|
|
stream.so \
|
|
mallocsize.so \
|
|
aclcheck.so \
|
|
list.so \
|
|
subcommands.so \
|
|
reply.so \
|
|
cmdintrospection.so \
|
|
eventloop.so \
|
|
moduleconfigs.so \
|
|
moduleconfigstwo.so \
|
|
publish.so \
|
|
usercall.so \
|
|
postnotifications.so \
|
|
moduleauthtwo.so \
|
|
rdbloadsave.so \
|
|
crash.so \
|
|
internalsecret.so \
|
|
configaccess.so \
|
|
test_keymeta.so \
|
|
keymeta_notify.so \
|
|
postnotifications_perkey_metadata.so \
|
|
atomicslotmigration.so
|
|
|
|
.PHONY: all
|
|
|
|
all: $(TEST_MODULES)
|
|
|
|
32bit:
|
|
$(MAKE) CFLAGS="-m32" LDFLAGS="-m32"
|
|
|
|
%.xo: %.c ../../src/redismodule.h
|
|
$(CC) -I../../src $(CFLAGS) $(SHOBJ_CFLAGS) -fPIC -c $< -o $@
|
|
|
|
%.so: %.xo
|
|
$(LD) -o $@ $^ $(SHOBJ_LDFLAGS) $(LDFLAGS) $(LIBS)
|
|
|
|
.PHONY: clean
|
|
|
|
clean:
|
|
rm -f $(TEST_MODULES) $(TEST_MODULES:.so=.xo)
|