A recorded redis DELETE is not a precondition, so cross-correlation deletes always diverge on replay
- Dominant language
- Rust
- Stars
- 2
- Forks
- 1
- Avg merge
- 21h 32m
- Merged PRs (30d)
- 67
Description
A recorded redis `DELETE` proves the key existed, but that evidence never reaches the seed plan. Every delete whose key was set by a *different* correlation therefore diverges on replay, blocking, against a candidate that did nothing wrong.
Measured on `rec-6097dd0-09161413-q8`, run `rp-sbx-6097dd0c92-6097dd0-09161413-q8-0916201249692`: 13 of 14 value divergences are this, all blocking.
## What the run shows
```
DEL cyMerchant_151007bd
recorded: KeyDeleted observed: KeyNotDeleted blocking: true
```
Four things were measured and ruled out before the cause was found, all from the run's own artifacts:
| ruled out | evidence |
|---|---|
| in-memory cache warmth | `get_val` is `replay = Substitute`, so an L1 read replays from the tape and cannot change which redis calls happen |
| L1 eviction reaching L2 | an in-memory `remove` cannot touch a redis key |
| concurrent drive | no `CONCURRENT DRIVE` line in the log; `deja-kernel: complete (driven 100, skipped 0)` in recorded order |
| TTL expiry | the whole drive took **7,948 ms** against a 300s `default_ttl` |
Two structural facts point at the cause:
- each diverged DEL and its matched `KeyNotDeleted` partner are in the **same** correlation, adjacent (5 of 6)
- the `set_key` preceding each diverged DEL is in a **different, earlier** correlation — **6 of 6, no exceptions**
## Cause
`RedisConnectionPool::add_prefix` (`crates/redis_interface/src/module/redis_rs.rs:547` in hyperswitch, `6097dd0c92`) namespaces every physical key by the active correlation during replay:
```rust
#[cfg(feature = "deja")]
if let Some(corr) = deja::replay_key_namespace() {
return format!("{corr}:{physical}");
}
```
So a SET in correlation A writes `A:key` and a DEL in correlation B addresses `B:key`. The DEL correctly finds nothing.
That isolation is deliberate and worth keeping — it is what makes it safe to `Execute` stateful redis ops against a seeded store, with no cross-case collision and no read-modify-write double-apply. The defect is not the namespacing. It is that **the precondition the DEL implies is never seeded into B's namespace.**
## Why it is never seeded
`build_seed_plan` (`crates/deja-runtime/src/replay.rs:2983`) derives preconditions from one source:
```rust
for key in &event.read_set {
```
Reads only. And a redis delete carries **no read set** — neither `set_key` nor `delete_key` declares one, they use the bare `deja::redis` preset, and `read_set` is an `Option` in the derive emitted only when declared (`crates/deja-derive/src/instrument.rs:656`).
So the recorded result `KeyDeleted` — which is direct evidence that the key was present — contributes nothing.
## The same function already states the opposite rule
From `build_seed_plan`, a few lines above the loop:
> Once a correlation CREATES rows in a table, we stop seeding its subsequent reads of that table: it reconstructs its own rows via its writes on replay. **UPDATE/DELETE mutate PRE-EXISTING rows, which remain genuine preconditions to seed.**
That rule is honoured on the `db` arm, because a DB `UPDATE`/`DELETE` carries a read set naming the rows it matched. It is silently not honoured on `redis`, because a redis delete carries none. One stated principle, two boundaries, one implementation.
## Fix shape
Declare a `read_set` on `delete_key` naming the key it deletes, so a recorded `KeyDeleted` becomes a precondition the planner materialises into that correlation's namespace.
**The caveat that needs deciding:** a delete's result gives *presence*, not *value* — `DelReply` is effectively a boolean. So the seed would be a placeholder. That is sound for delete semantics, which only need existence, and the surrounding cases are already covered:
- if the correlation **read** the key before deleting it, that read is in the read set and seeds the correct value; the existing "pristine until the first write" rule resolves the overlap
- if it **only** deletes, nothing observes the placeholder
- a read **after** the delete correctly finds absence
What this must **not** do is borrow the value from the earlier correlation's `SET`. The value is on the tape, but reaching across correlations is exactly what the isolation prevents, and the same file records what that costs: seeding a post-image instead of a pre-image previously produced "13 payment_methods timestamp divergences and their 13 readback misses on run-0811".
## Before a PR
This changes what every correlation's store contains, so the blast radius should be measured rather than assumed:
1. how many keys across a real tape would newly be seeded
2. whether any currently-correct divergence is masked by a placeholder appearing where the recording had nothing
3. whether `set_key` needs the symmetric treatment, or deliberately does not
The property the current scheme gives for free is "a seeded key came from a recorded read". Relaxing it should come with the replacement guarantee written down.
## Scope note
Not a candidate defect and not related to any hyperswitch fix under test — the affected calls are merchant-account cache invalidation during test setup (`POST /accounts`, `GET /accounts/{id}`), not a payment path. It is noise a reviewer has to learn to ignore, which is how the ones worth acting on get missed.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with build_seed_plan in crates/deja-runtime/src/replay.rs, then inspect delete_key instrumentation in crates/deja-derive/src/instrument.rs and Redis namespacing in crates/redis_interface/src/module/redis_rs.rs. Measure the affected keys and replay behavior on the referenced runs before deciding whether delete_key should declare a read_set. Done means the placeholder-presence behavior, any set_key symmetry, and the replacement guarantee are documented and covered by evidence or tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- redis, rust
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100