fix(embeddings): cluster delete via parallel del() so cross-slot keys actually get removed

@zen-agentchecks n/achecks…zen/embeddings-redis-cluster-delete-fix-vr4h16 → master1 files · +12 −22updated 1mo ago

▸Description· 1 comment

Description

Follow-up to #23065. The CI run on production after the cluster TLS fix surfaced this warning (which was also silently present in the last passing Apr 19 run):

⚠️  redis delete failed: Error: All keys in the pipeline should belong to the same slots allocation group

Root cause

deleteRedisKeys (in embeddings/src/pipeline/redis.ts) queued every key into a single redisClient.pipeline() and called .exec(). ioredis Cluster pipelines route to exactly one node, so when the batch contained keys hashing to different hash slots — which is the common case for tr-sidecar:* / tr-id-map:* batches — the cluster rejected the whole pipeline as a unit. The caller at build_tool_index_parallel.ts:1674 logged the warning and continued, so no keys got deleted when the pipeline failed. Stale entries for deleted/inactive tools accumulated in Redis indefinitely.

Fix

Replace the single pipeline with per-key DELs issued via Promise.all. The cluster client already sets enableAutoPipelining: true, so concurrent single-key commands are automatically batched into one pipeline per target node — efficient for cluster mode and slot-correct.

-  const pipeline = redisClient.pipeline()
-  for (const key of keys) {
-    pipeline.del(key)
-  }
-  const execResult = await Result.wrapAsync(() => pipeline.exec())
-  …
+  const deleteResult = await Result.wrapAsync(() =>
+    Promise.all(keys.map((key) => redisClient.del(key))),
+  )
+  if (deleteResult.err) {
+    return Err(String(deleteResult.val))
+  }
+  return Ok(deleteResult.val.reduce((sum, n) => sum + n, 0))

Single-instance (non-cluster) Redis path unchanged.

How did I test this PR

bun run tsc --noEmit in embeddings/ — clean.
Verified both callers (deleteRedisKeysForIds for backfill, resetRedis for full wipe) now go through the slot-correct path.
E2E: next prod-branch run that triggers Build Embeddings Bundle (R2) and reaches the redis-delete code path will exercise this. The warning should disappear and the per-batch summary redis deletes: N keys removed should fire.

@zen-agent1mo ago

CI status

Check	Result
Tests	✅
Lint and type checks	✅
Checks	✅
Endpoint Duplicate CI	✅
Trigger Config	✅
Markdown renderer	✅
validate	✅
Analyze (javascript-typescript) — CodeQL	✅
Analyze (actions) — CodeQL	✅
Analyze (python) — CodeQL	⏳ in_progress (~30 min, normal for this repo)
Secrets Detection	❌ pre-existing platform failure (see below)

Pre-existing failures

Secrets Detection is failing on every recent PR in this repo (10/10 most recent secrets-detection.yml runs, across unrelated branches like fix/mural-disable-pkce, fix/gumroad-disable-pkce, etc.) with conclusion: failure and zero jobs created — i.e., the reusable workflow at ComposioHQ/.github/.github/workflows/secrets-detection.yml@main is failing to dispatch jobs at the orchestration layer. Not introduced by this PR's diff.

Codex review

LGTM, no actionable findings:

The only substantive change replaces cluster-wide Redis pipeline deletes with per-key deletes that are safe across hash slots; the remaining changes are formatter-only. I did not identify any actionable regressions introduced by the patch.

Targeted build check

bun run tsc --noEmit in embeddings/ — clean (exit 0). No unit-test file exists for this module; behavior verified against existing callers (deleteRedisKeysForIds, resetRedis) which both consume the same Result<number, string> contract that this PR preserves.

loading diff…

@zen-agent1mo ago

CI status

Check	Result
Tests	✅
Lint and type checks	✅
Checks	✅
Endpoint Duplicate CI	✅
Trigger Config	✅
Markdown renderer	✅
validate	✅
Analyze (javascript-typescript) — CodeQL	✅
Analyze (actions) — CodeQL	✅
Analyze (python) — CodeQL	⏳ in_progress (~30 min, normal for this repo)
Secrets Detection	❌ pre-existing platform failure (see below)

Pre-existing failures

Codex review

LGTM, no actionable findings:

The only substantive change replaces cluster-wide Redis pipeline deletes with per-key deletes that are safe across hash slots; the remaining changes are formatter-only. I did not identify any actionable regressions introduced by the patch.