Skip to content

feat: add operation timeouts for OpenAI embeddings - #1805

Open
xinpingzh wants to merge 1 commit into
MemMachine:mainfrom
xinpingzh:feat/embedding-operation-timeouts
Open

xinpingzh wants to merge 1 commit into
MemMachine:mainfrom
xinpingzh:feat/embedding-operation-timeouts

Conversation

@xinpingzh

Copy link
Copy Markdown

Purpose of the change

Applications that retrieve memory before replying need to limit the time spent waiting for embeddings. An HTTP request timeout alone does not bound an embedding operation containing batches, SDK retries, and application retry backoff.

Description

Add optional search_timeout_seconds and ingest_timeout_seconds to OpenAI-compatible embedder configuration. Each applies to the entire public embedding call, including its batches, chunk requests, retries, and dimensions fallback. Expiry cancels pending work and raises ExternalServiceAPIError with the timeout as its cause; caller cancellation still propagates and partial vectors are not returned.

Both settings default to None for compatibility with existing deployments and slower local providers. The event-memory sample explicitly enables 5 seconds for search and 60 seconds for ingestion. SDK request timeouts and retry settings are unchanged. These are cooperative embedding-stage budgets, not deadlines for the full memory API.

Move operation latency tracking to the public call boundary so expiry is recorded as an error. Batched calls now produce one operation observation covering all batches.

Fixes/Closes

Fixes #1771. Scope proposed in the issue discussion; the compatibility defaults have not yet received maintainer feedback.

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

On Windows, Python 3.12, with no paid model calls:

uv run pytest -q \
  packages/server/server_tests/memmachine_server/common/embedder \
  packages/server/server_tests/memmachine_server/common/configuration/test_embedder_conf.py \
  packages/server/server_tests/memmachine_server/common/configuration/test_configuration.py \
  packages/server/server_tests/memmachine_server/common/resource_manager/test_embedder_manager.py \
  packages/server/server_tests/memmachine_server/common/resource_manager/test_metrics_factory_wiring.py

74 passed, 3 skipped, 27 deselected; three existing testcontainers deprecation warnings. Tests cover cancellation of pending batches/clusters, partial-result rejection, retry/backoff, dimensions fallback, concurrent search/ingestion, caller cancellation, configuration round-tripping and manager wiring, and timeout metrics. A real AsyncOpenAI client with httpx.MockTransport exercises SDK retry cancellation without network access.

ruff check, ruff format --check, and git diff --check passed. Targeted ty checks passed for the embedder, configuration, and changed tests. Checking the manager reports missing optional sentence_transformers, also reproduced on the base version. Full ty check packages reports 25 diagnostics from missing optional packages and Windows time.tzset; it did not pass in this environment. Live-provider/database integration tests were not run.

Checklist

  • Commit includes sign-off and a verified GitHub signature
  • Code follows the project style guidelines
  • Complete diff reviewed, including an independent review
  • Configuration documentation and a sample updated
  • Tests added for the new behavior and relevant regressions passed

Bound search and ingestion embedding calls across batches and retries when configured.

Signed-off-by: xp <[email protected]>

This branch has not been deployed

No deployments
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.

[Bug]: Search embeddings have no timeout, so one slow embeddings response stalls a search for tens of seconds

1 participant