Skip to content

perf: check each path once per storage-event batch and run paths concurrently - #2072

Merged
phernandez merged 1 commit into
mainfrom
storage-event-batch-concurrency
Nov 6, 2026
Merged

phernandez merged 1 commit into
mainfrom
storage-event-batch-concurrency

Conversation

@phernandez

Copy link
Copy Markdown
Member

Why

A version prune deletes old object versions, and Tigris sends one OBJECT_DELETED notification per version. In production on 2026-11-06 the webhook batches during a prune averaged 30.3 events for 10.3 distinct paths. run_runtime_storage_event_operations handled every event one after another (entity lookup plus a current-object HEAD, about 0.33 s each), so a batch took about 10 s and the cloud webhook queue fell 30+ minutes behind. That delay hits every workspace's sync and external edits, because they share the queue.

What

  • One check per path per batch. Operations are grouped by path in arrival order, and consecutive deletes of one path collapse into a single step that stands for all the events it covers. A delete only asks whether the object is still there, so one check answers them all. Counts stay per event: a collapsed step adds its event count to processed or failed.
  • Paths run concurrently, opt in. run_runtime_storage_event_operations(..., max_concurrent_paths=1). The default keeps today's behavior. Above 1, different paths run concurrently while one path's steps stay in order (a create between two deletes still runs between them). A failure the processor chooses to raise cancels the other paths and fails the batch as that exception, as it stopped the sequential loop before.
  • StorageEventIndexRuntime.max_concurrent_paths passes the setting through project routing.
  • StorageEventOperationProcessor creates its lazy delete resources under a lock, so concurrent paths share one set.

Local Basic Memory keeps the default (one path at a time). Cloud will pass the webhook job's share of the worker's tenant database connection budget, so the fan-out never exceeds connections the job can hold (the cloud #1844 rule).

Tests

New tests in tests/test_runtime_storage_events.py and tests/index/test_storage_event_operation_processor.py:

  • repeated deletes of one path run one check and count every event;
  • a create between deletes keeps each step in order;
  • a failed collapsed check counts every event it covers as failed;
  • paths run concurrently up to the limit and never overlap themselves; the default runs one at a time;
  • a raised failure fails a concurrent batch as itself, not as an ExceptionGroup;
  • concurrent deletes create delete resources once.

Each guard was checked by breaking it: no collapse fails 2 tests, never concurrent fails the cap test, no lock fails the shared-resources test. tests/test_runtime_storage_events.py plus tests/index/ pass (252). Ruff clean; just typecheck reports only the existing pymilvus optional-dependency imports.

🤖 Generated with Claude Code

https://claude.ai/code/session_016N9c6ouiT4Wid3arGR41Dv

…hs concurrently

Deleting old object versions (a version prune) sends one OBJECT_DELETED per
version. In production on 2026-11-06 a webhook batch averaged 30 events for
10 paths, each event checked one after another (entity lookup + current-object
HEAD, ~0.33 s), so a batch took ~10 s and the webhook queue fell behind.

- Operations are grouped by path in arrival order. Consecutive deletes of one
  path collapse to one check, counted as the events it covers (processed or
  failed). A delete only asks whether the object is still there, so one check
  answers them all. A create between deletes stays its own step, in order.
- `run_runtime_storage_event_operations` takes `max_concurrent_paths`
  (default 1: unchanged, one path at a time). Above 1, different paths run
  concurrently; one path's steps stay sequential. A failure the processor
  raises cancels the rest and fails the batch as itself.
- `StorageEventIndexRuntime.max_concurrent_paths` passes it through routing.
- `StorageEventOperationProcessor` creates its delete resources under a lock,
  so concurrent paths share one set.

Tests cover the collapse, ordering across a create, failure counts, the
concurrency cap, the sequential default, a raised failure, and shared delete
resources. Each fails with its guard removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016N9c6ouiT4Wid3arGR41Dv
Signed-off-by: phernandez <paul@basicmachines.co>
@chatgpt-codex-connector

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-06T20:43:46.465946Z e26d04a PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@phernandez phernandez changed the title perf(index): check each path once per storage-event batch and run paths concurrently perf: check each path once per storage-event batch and run paths concurrently Nov 6, 2026
@phernandez
phernandez merged commit 7f7a219 into main Nov 6, 2026
51 of 52 checks passed
@phernandez
phernandez deleted the storage-event-batch-concurrency branch November 6, 2026 20:51
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