Skip to content

fix(search): bound temporal analysis for oversized queries - #3140

Open
r266-tech wants to merge 2 commits into
vectorize-io:mainfrom
r266-tech:fix-oversized-temporal-query
Open

fix(search): bound temporal analysis for oversized queries#3140
r266-tech wants to merge 2 commits into
vectorize-io:mainfrom
r266-tech:fix-oversized-temporal-query

Conversation

@r266-tech

Copy link
Copy Markdown
Contributor

Addresses the temporal-analysis failure described in #3134.

What changed

  • Skip optional temporal analysis for default DateparserQueryAnalyzer inputs longer than 512 characters, before period extraction or dateparser loading.
  • Run accepted default-analyzer work in a dedicated two-worker executor with four total running/queued slots and a one-second caller deadline.
  • Fail open to semantic/BM25/graph retrieval on overload, timeout, or parser failure. Timed-out and cancelled work retains its slot until the underlying future actually completes, so abandoned recalls cannot grow the executor queue without bound.
  • Preserve the existing synchronous, uncapped behavior for caller-supplied analyzers because that interface does not promise thread safety.

The reported poison fact was 582,808 characters. For its repeated mandatory pattern on the reference checkout after warm-up, 128/256/512 characters took roughly 0.23/0.44/0.82 seconds while 4,096 took about 6.7 seconds, so 512 keeps normal query room while rejecting the demonstrated extraction-sized input.

Explicit trade-off

ThreadPoolExecutor cannot terminate running parser code. A regex operation that holds the GIL may therefore outlive—and potentially delay—the one-second caller timer. The length cap is the primary defense for the reported failure; the executor bounds admission and prevents unrelated default-executor starvation, but it is not a hard CPU deadline. A timed-out job keeps one of the four slots until it truly finishes. Killable process isolation would add worker lifecycle and serialization constraints and is intentionally left out of this focused fix.

Queries above the cap continue through non-temporal retrieval, so they may return a broader result set rather than wedging the server.

Verification

  • uv run pytest -q tests/test_query_analyzer.py — 424 passed
  • targeted temporal retrieval concurrency/cancellation regression — passed
  • changed-file ruff check, ruff format --check, and ty check — passed
  • the repository-wide hook reached the frontend phase but could not load the locally absent @eslint/js; no unrelated generated/frontend edits are included

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