Skip to content

Honour key_prefix in the Redis scheduler store: index and change notices - #85

Draft
b7r6 wants to merge 2 commits into
mainfrom
fix/redis-index-prefix-honours-key-prefix
Draft

b7r6 wants to merge 2 commits into
mainfrom
fix/redis-index-prefix-honours-key-prefix

Conversation

@b7r6

@b7r6 b7r6 commented Oct 22, 2026 •

Copy link
Copy Markdown
Owner

What and why

RedisStore::search_by_index_prefix created its search index over the schema's bare key prefix (aa_, fc_, …) and named it the same way, ignoring the store's own key_prefix. Every key the store writes carries that prefix, so with key_prefix: "sched:" the index covered nothing: FT.INFO reported num_docs 0 beside thirteen sched:aa_… hashes, the matcher's FT.AGGREGATE found no queued action, and a Redis-backed scheduler queued everything and dispatched nothing. A bazel build //... sat at (17 actions, 0 running) until killed.

The index's PREFIX is now <key_prefix><schema prefix>, and its name sits under the store's prefix too, so two stores with different prefixes on one Redis cannot share an index whose PREFIX only one of them wrote. With an empty key_prefix both are unchanged, byte for byte.

Second commit, same root. The store publishes a key-change notice with the key as Redis holds it, behind the prefix, while RedisSubscriptionManager keeps subscriptions under the bare store key. With a prefix no notice ever woke a subscriber, so every state change waited for the subscriber's own timeout re-read: the same build took 385–443 s on a prefixed scheduler store against 150–197 s without, on one binary. The manager now strips its store's prefix off a notice before the lookup and ignores notices from outside it.

How was this verified?

  • New test_search_by_index_covers_the_store_key_prefix drives the create path on a prefixed mock store: the first FT.AGGREGATE answers "No such index", and the FT.CREATE that follows must name the prefixed index and cover sched:test:. Red without the fix (unprefixed FT.CREATE), green with it.
  • New a_notice_behind_the_store_prefix_wakes_the_bare_key_subscriber: subscribes a bare key on a manager with key_prefix: "sched:", publishes the prefixed key, expects the wake. Times out without the fix, passes with it.
  • cargo test -p nativelink-store --test redis_store_test: 70 passed. cargo test -p nativelink-scheduler --test store_awaited_action_db_test: 14 passed. cargo clippy -p nativelink-store --tests clean.
  • Found on a single-node soak (upstream HEAD + fixes, Redis 8.4 with RediSearch 8.4.10, experimental_backend.redis, key_prefix: "sched:"): the scheduler wedged on the first build. The same fleet with no key_prefix runs the build clean, which is what every upstream example and test does, so the bug only shows when a prefix is set.

Risk

Low. The default (empty prefix) produces identical commands. A deployment that already runs a prefixed Redis scheduler store cannot exist in a working state, since it never dispatched; after this change its index is created fresh under the prefixed name.

AI assistance

Written with Claude Code (Claude Fable 5.1); the author reproduced the wedge on a soak fleet, reviewed the fix and the test, and ran the verification above.

🤖 Generated with Claude Code

@github-actions

This comment has been minimized.

@b7r6 b7r6 changed the title Index the keys a prefixed Redis store actually writes Honour key_prefix in the Redis scheduler store: index and change notices Oct 22, 2026
@b7r6

b7r6 commented Oct 22, 2026

Copy link
Copy Markdown
Owner Author

End-to-end on the railgun soak fleet (3 workers, 16 slots each, bazel build //... of this repo, 1,234 remote actions, Redis 8.4 with RediSearch 8.4.10, scheduler store key_prefix: "sched:", experimental_pub_sub_channel set). Scheduler process only is swapped; CAS and workers stay on the soak branch binary.

scheduler binary key_prefix forced re-execution pass cold pass
soak branch (no fix) sched: wedged on the first build: (17 actions, 0 running) for 25 min, FT.INFO num_docs 0 beside 13 sched:aa_… hashes –
soak branch (no fix) none 150–197 s 312 s
soak branch + commit 1 sched: 385–406 s (builds, but every state change waits for the subscriber's timeout re-read) 491 s
soak branch + commits 1 and 2 sched: 164–170 s 329 s

Four passes clean on the final row, no scheduler timeouts, only the three first-touch "no such index" lines at startup.

b7r6 and others added 2 commits October 22, 2026 20:35
`RedisStore::search_by_index_prefix` created its search index over the
schema's bare key prefix (`aa_`, `fc_`, …) and named it the same way,
ignoring the store's own `key_prefix`. Every key the store writes carries
that prefix, so with `key_prefix: "sched:"` the index covered nothing:
`FT.INFO` reported `num_docs 0` beside thirteen `sched:aa_…` hashes, the
matcher's `FT.AGGREGATE` found no queued action, and a Redis-backed
scheduler queued everything and dispatched nothing. A `bazel build //...`
sat at `(17 actions, 0 running)` until killed (railgun soak, 2026-10-22).

The index's `PREFIX` is now `<key_prefix><schema prefix>`, and its name
sits under the store's prefix too, so two stores with different prefixes
on one Redis cannot share an index whose `PREFIX` only one of them wrote.
With an empty `key_prefix` both are unchanged, byte for byte.

The test drives the create path on a prefixed mock store: the first
`FT.AGGREGATE` answers "No such index", and the `FT.CREATE` that follows
must name the prefixed index and cover `sched:test:`. Without the fix it
fails on the unprefixed `FT.CREATE`.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
The store publishes a key-change notice with the key as Redis holds it,
behind `key_prefix`; `RedisSubscriptionManager` keeps subscriptions under
the bare store key and looked the notice up as published. With a prefix
the two never met, so no notice woke anyone and every state change waited
for the subscriber's own timeout re-read. On the soak fleet that was the
same `bazel build //...` taking 385–443 s on a prefixed scheduler store
against 150–197 s without the prefix, on one binary.

The manager now strips its store's prefix off a notice before the lookup,
and drops a notice from outside the prefix, which is another store's on a
shared channel. With an empty prefix nothing changes.

The test subscribes a bare key on a manager with `key_prefix: "sched:"`,
consumes the subscription's initial change, publishes the prefixed key and
expects the wake within five seconds. Without the fix it times out.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant