feat: enable client-side bulk update/delete operations with SearchIndex (RAAE-1326)#651
Open
vishal-bala wants to merge 9 commits into
Open
feat: enable client-side bulk update/delete operations with SearchIndex (RAAE-1326)#651vishal-bala wants to merge 9 commits into
SearchIndex (RAAE-1326)#651vishal-bala wants to merge 9 commits into
Conversation
Redis has no server-side delete/update-by-query, so mutating many documents meant hand-rolling a scan-and-set loop. Add ergonomic, filter-driven bulk operations to SearchIndex and AsyncSearchIndex: - delete_by_filter(): resolves matching keys and removes them with non-blocking UNLINK in batches, re-querying offset 0 each round (mirrors clear(); not bound by MAXSEARCHRESULTS). - update_by_filter(): partial field-set on all matches. Collects matching keys via FT.AGGREGATE ... LOAD @__key WITHCURSOR (stable, unbounded), then applies HSET (hash) or JSON.MERGE (JSON) in batches. JSON.MERGE follows RFC 7396: nested objects merge recursively, arrays replace wholesale, None deletes a path. Both support dry_run (count preview), a match-all guard (allow_all to override; clear() remains the intentional wipe), an on_progress callback, and a tunable batch_size. Also batch the existing drop_documents()/drop_keys() and make drop_keys() cluster-safe via a per-key UNLINK fallback (_unlink_batch). Covered by tests/integration/test_bulk_operations.py (sync + async, hash + JSON, guards, dry-run, multi-batch cursor, batched drops). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bulk delete_by_filter/update_by_filter run as batched writes, not a single transaction: atomic per document but not across the match set, with no rollback. Add a "Note" to both docstrings and a "Durability and partial failure" paragraph to the concepts page explaining the atomicity boundary, that a crash leaves partial changes persisted, and that re-running the (idempotent) call is the recovery path. Also flag the concurrent-delete-then-recreate caveat for updates. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- _agg_row_to_key: replace the terse zip(slice, slice) dict trick with an
explicit index lookup ("value after the __key field") plus a comment
describing the flat [field, value, ...] aggregation row shape.
- Add blank lines and intent comments in drop_documents/drop_keys/
_unlink_batch (sync + async) and the cursor loops so the cluster vs
standalone branches and termination condition read clearly.
No behavior change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Consolidated fixes from documentation/code/security/system-design/ product reviews of the bulk delete/update feature: - Memory (P0): update_by_filter no longer buffers all matched keys in client memory. Since a RediSearch aggregation cursor cannot be read while the index is written (verified: it hangs), keys are staged in a temporary server-side Redis list (RPUSH during the cursor drain, then LPOP batches to write). Client memory is now O(batch_size). Temp key is TTL-guarded and cleaned up in a finally. - Return contract (P0): delete_by_filter/update_by_filter now return a BulkResult(matched, processed, completed, dry_run) instead of a bare int (int-compatible via __int__). delete signals completed=False and logs a warning when the runaway backstop trips instead of silently under-reporting. - on_progress (P0): callback signature is now (processed, matched) so it can drive a progress bar; documented as sync-only and abort-on-raise. - Cursor leak (P1): _iter_keys_by_filter now releases the server-side cursor (FT.CURSOR DEL) in a finally and sets MAXIDLE. - Docs (P1): fix drop_documents "safe on cluster" (it raises without a shared hash tag) + add Raises; warn that JSON update values must match the document layout (nested path), are written without schema validation, and are static-only; add an "Operational considerations" section (live-traffic reindex load, memory, cluster per-key writes, no resumability) and a filter-injection caution. - Tests: add JSON delete, JSON multi-batch update, JSON None-deletes-path, BulkResult contract, and staging-key-cleanup coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per the v1 decision, revert update_by_filter from the temporary server-side staging list to simple client-side key buffering. The staging list bounded client memory but relocated it onto the shared, paid production instance (single-node/single-slot hotspot that can trigger cross-tenant eviction or OOM) and introduced a TTL-driven silent truncation bug on multi-hour runs. Client buffering keeps the cost on the isolated worker; filter partitioning is the documented scale story. Also from the final review round: - Drop BulkResult int-compat (__int__/__index__). It was a leaky half-drop-in (TypeError on +/sum, always-truthy, != int) solving a non-problem, and it masked the completed flag. Callers read .processed / .matched / .completed explicitly. - Fix cursor idle timeout: redis-py's cursor(max_idle=...) takes seconds and multiplies by 1000, so 300_000 meant ~83h, not 5min. Now 300s. - Docs: replace the staging/server-memory framing with client-buffer + partition-the-filter guidance; note update progress callbacks begin only in the write phase; drop int-compat wording. - Tests: drop the staging-key test; assert BulkResult fields directly. Removes the two P1s the staging list introduced; all prior P0/P1s stay resolved. make format + mypy clean; bulk suite green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Align with RedisVL's existing naming convention: drop_* removes documents (drop_keys, drop_documents), while delete() removes the index (FT.DROPINDEX). Using "drop" for the filter-based document deletion keeps the library internally consistent and avoids overloading delete() / colliding with index.delete(). update_by_filter is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SearchIndex (RAAE-1326)
SearchIndex (RAAE-1326)SearchIndex (RAAE-1326)
vishal-bala
marked this pull request as ready for review
July 23, 2026 07:20
update_by_filter resolves matching keys before it writes, so a document can be deleted by another client in that window; a plain HSET/JSON.MERGE would then recreate it as a partial (schema-incomplete) document. Each write is now conditional on the key still existing, applied atomically via a small Lua script (no native HSET-if-exists, and JSON.MERGE creates on a missing key). A concurrently-deleted document is skipped, not recreated. As a bonus this makes `processed` exact: it now counts documents actually written (still-existing), so it can be < `matched` under concurrent deletion. Per-key round-trip count is unchanged (EVAL replaces HSET/JSON.MERGE); the window-size analysis showed this matters most for update at scale / on cluster, negligibly for drop. Tests: hash + JSON "skips missing key, does not recreate" coverage. make format + mypy clean; 20 bulk tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e922cf5. Configure here.
Address P2s from the guard review round: - Add an end-to-end test (via on_progress deleting a still-pending match mid-run) proving processed < matched when a doc is deleted between resolution and write. - Add an async test for the existence-guard skip path. - Document that the guard is existence-based, not identity-based: a delete-then-recreate at the same key can still land the update on a different document, so the quiescent-partition guidance stands. 22 bulk tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- _iter_keys_by_filter (sync + async): capture the cursor id BEFORE parsing rows, so the finally block still issues FT.CURSOR DEL if _agg_row_to_key raises on a batch — previously a parse error on the first batch left cid=0 and leaked the server-side cursor until idle timeout. - drop_by_filter docstring: drop the stale "int-compatible (int(result) == processed)" claim — BulkResult no longer defines __int__, so int(result) would raise; direct the reader to the fields. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rbs333
approved these changes
Jul 24, 2026
rbs333
left a comment
Collaborator
There was a problem hiding this comment.
Love this! At one point in history we had talked about adding something like this but it felt to the wayside. Only thing I would check is to build the docs and make sure the updates are linked properly!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Motivation
Redis has no "delete/update by query." To remove or change many documents today, you scan the index for matching keys and issue the writes yourself, batch by batch. It works, but every team rewrites the same loop — and gets the sharp edges (batching, cluster hash-slots, pagination limits) wrong in slightly different ways.
This PR adds two filter-driven bulk operations to
SearchIndexandAsyncSearchIndex, so a common request becomes one call:Both return a
BulkResult(matched,processed,completed,dry_run) and acceptdry_run=Truefor a no-op count preview, anon_progress(processed, matched)callback, and a tunablebatch_size.Implementation
Redis still can't mutate-by-query, so the work is client-side; the value is doing it correctly behind one call.
drop_by_filterresolves matching keys and removes them with non-blockingUNLINK, re-querying from offset 0 each round. Because deleted docs leave the result set, the set shrinks to empty on its own — the same trickclear()uses — which sidesteps RediSearch'sMAXSEARCHRESULTSpaging cap. A count-based backstop guards against a runaway loop under heavy concurrent inserts; if it trips, the result reportscompleted=Falseand logs a warning rather than silently under-reporting.update_by_filterapplies a partial update:HSETfor hash indexes,JSON.MERGE(RFC 7396) at the document root for JSON. An update doesn't remove a doc from the match set, so the shrink-to-empty trick can't work. Instead it resolves the full key set viaFT.AGGREGATE ... WITHCURSOR(unbounded, unlikeFT.SEARCH), then writes in pipelined batches. The two phases are strictly separated because reading an aggregation cursor while the same index is being written hangs the cursor (verified against Redis 8). The cursor is always released (FT.CURSOR DEL) on exit.Naming follows RedisVL's existing convention —
drop_*removes documents (drop_keys,drop_documents), whiledelete()removes the index.drop_by_filterslots into the first group;delete()is left untouched.drop_keys/drop_documentsnow chunk large inputs and stay cluster-safe (per-keyUNLINKacross hash slots).Caveats
update_by_filtermemory scales with the match count (all keys are resolved before writing). For very large match sets, partition the filter and run in waves.update_by_filterrecreates it as a partial document.valuesmerge at the document root, so keys must match the document's JSON layout, not the schema field name (a field indexed at$.metadata.statusneeds{"metadata": {"status": ...}}). Values are written without schema validation; only static values are supported.Tag,Num, ...) — never string-concatenate untrusted input into a filter, since here an injected predicate mutates data.Future steps
update_by_filter(e.g.price *= 1.1).Testing
tests/integration/test_bulk_operations.py— 18 tests covering sync/async, hash/JSON, the match-all guard,dry_run, multi-batch cursor iteration, JSON nested-merge andNone-deletes-path, and the batcheddrop_*helpers.make formatandmake check-types(mypy) pass.Jira: RAAE-1326
Note
Medium Risk
Bulk filter APIs can delete or mutate large document sets with non-atomic batch semantics; filter injection or overly broad filters are destructive, though match-all guards and existence-guarded updates reduce some failure modes.
Overview
Adds client-side bulk mutation on
SearchIndexandAsyncSearchIndexbecause Redis has no delete/update-by-query.drop_by_filtermatches documents via a filter, then removes them in batches withUNLINK, re-querying from offset 0 (likeclear) so paging limits do not block large deletes. It returnsBulkResult(matched,processed,completed,dry_run), supportsdry_run,allow_allfor match-all guards,on_progress, and a runaway backstop that can setcompleted=False.update_by_filterdoes partial updates:HSETon hash indexes andJSON.MERGEat$on JSON. It first resolves all matching keys withFT.AGGREGATE+ cursor, then writes in batches (read/write phases are separated because cursors cannot be read while mutating). Updates use Lua scripts that only write if the key still exists, so concurrent deletes are skipped rather than resurrected as partial docs.drop_keysanddrop_documentsnow acceptbatch_size(default 500), chunk large lists, and use per-keyUNLINKon cluster to avoidCROSSSLOT.Documentation in
docs/concepts/search-and-indexing.mdcovers semantics, JSON path caveats, durability, and scale guidance. Integration tests intests/integration/test_bulk_operations.pycover hash/JSON, async, guards, dry-run, concurrency skips, and batched drops.Reviewed by Cursor Bugbot for commit cacde65. Bugbot is set up for automated code reviews on this repo. Configure here.