Skip to content

Added optimization for OpenSearch engine which were missing - #7

Closed
navneet1v wants to merge 1 commit into
redis:opensearch.improvementsfrom
navneet1v:redis/opensearch.improvements
Closed

navneet1v wants to merge 1 commit into
redis:opensearch.improvementsfrom
navneet1v:redis/opensearch.improvements

Conversation

@navneet1v

@navneet1v navneet1v commented Jun 26, 2024 •

Copy link
Copy Markdown

Description

This PR added the optimization for Opensearch engine in this benchmarking tool. Along with the optimizations I have also upgraded the OS docker image to latest version(at the time of raising this PR) which is 2.14

Testing

I performed a local testing on a small dataset and a small configuration and I can see improvements in Opensearch results.

Sample run results

Cluster: single node with CI based docker setup

python3 -m run --engines opensearch-default --datasets glove-25-angular

Indexing

{
  "params": {
    "experiment": "opensearch-default",
    "engine": "opensearch",
    "dataset": "glove-25-angular",
    "parallel": 16,
    "method": {
      "parameters": {
        "m": 16,
        "ef_construction": 100
      }
    }
  },
  "results": {
    "post_upload": {},
    "upload_time": 52.828218106064014,
    "total_time": 143.01298881007824
  }
}

Search

{
  "params": {
    "dataset": "glove-25-angular",
    "experiment": "opensearch-default",
    "engine": "opensearch",
    "parallel": 100,
    "config": {
      "knn.algo_param.ef_search": 128
    }
  },
  "results": {
    "total_time": 17.1585195150692,
    "mean_time": 0.013182678549340927,
    "mean_precisions": 0.9283049999999999,
    "std_time": 0.016356453433367828,
    "min_time": 0.0020476150093600154,
    "max_time": 0.7723487680777907,
    "rps": 582.8008640965589,
    "p95_time": 0.030238651664694766,
    "p99_time": 0.07367674464592752
  }
}

Please let me know if I need add this in upstream qadrant repo.

Signed-off-by: Navneet Verma <navneev@amazon.com>
@navneet1v

Copy link
Copy Markdown
Author

In the next PR I will add the support of faiss engine using cosinesiml distance function. I didn't add that functionality because faiss is only getting used when filters are present.

@navneet1v navneet1v changed the title Added improvements for OpenSearch engine which were missing Added optimization for OpenSearch engine which were missing Jun 26, 2024
@navneet1v

Copy link
Copy Markdown
Author

@filipecosta90

@fcostaoliveira

Copy link
Copy Markdown
Collaborator

@navneet1v we've missed this one. will review until end of next week. thank you for raising this!

fcostaoliveira added a commit that referenced this pull request Jul 8, 2026
Ports the engine-agnostic OpenSearch optimizations from the Python tool
(#7) that apply to our lucene-engine
index:

- kNN warmup: GET /_plugins/_knn/warmup/<index> in setup_search so the
  first queries aren't penalised by cold-cache graph loading. Best-effort
  (non-success is logged, not fatal); verified accepted on lucene indices.
- Search response trimming: _source:false + docvalue_fields:[_id] +
  stored_fields:_none_ so only the id is returned. Parsing reads
  fields._id[0] with a fallback to top-level _id (new hit_id helper).
- Indexing tuning: number_of_replicas:0 and refresh_interval:-1 during
  upload, with an explicit refresh() after upload (before force_merge) so
  documents become searchable.

Intentionally NOT ported: the faiss/nmslib engine switch and the
native-only knn.memory.circuit_breaker.limit / index_thread_qty cluster
settings. v1 keeps the lucene engine, which is required for the efficient
kNN filtering fix (#59); those knobs don't apply to lucene.

Verified end-to-end against OpenSearch 2.19.2: random-100 recall stays
1.0000 with trimming + refresh + warmup (a regression to 0.0 without the
refresh confirmed the fix is load-bearing). Unit tests cover build_knn_body
trimming and hit_id docvalue/fallback parsing.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@fcostaoliveira

Copy link
Copy Markdown
Collaborator

Thanks for these OpenSearch optimizations, @navneet1v — closing as superseded by the Rust rewrite (master). The substantive tuning is already in src/bin/vector_db_benchmark/engine/opensearch.rs: refresh_interval: -1 during bulk indexing, force-merge to a single segment after upload, and kNN graph warmup (/_plugins/_knn/warmup). The Rust engine additionally uses the Lucene kNN engine for efficient filtered graph traversal. The only bits not carried over are minor knobs (index_thread_qty, shard/replica count) — happy to track those as a small enhancement if useful.

fcostaoliveira added a commit that referenced this pull request Jul 20, 2026
… in duration mode (#152)

Reliability fixes from the full Redis-vs-Vertex end-to-end run (issue #151),
the localized/validatable subset:

- #3 `--engines-file` was a silent no-op: `read_engine_configs()` globbed
  `experiments/configurations/*.json` and ignored the flag, so
  `--engines-file x.json --engines my-cfg` failed with a confusing "no engines
  match". Now `read_engine_configs(engines_file)` reads ONLY that file when
  given; a missing/malformed file is a hard error, not a fall-through. Verified
  end-to-end (the file's config runs) + unit-tested.

- #1 Vertex stream-update upload treated a transient HTTP 401 as fatal, aborting
  mid-stream and leaving a partially-populated index that returns silently-wrong
  recall. Now a 401 re-mints the access token and retries the (idempotent) batch
  (bounded, same shape as the existing 429/Retry-After handling). This plus the
  existing per-worker 45-min proactive refresh covers #2 (token expiry) for the
  long-upload path.

- #6 `--repetitions` (default 3, best-of-N) is meaningless in --search-duration
  timed mode — it just triples runtime for an upward-biased max. Force 1 there,
  warning if the user set >1.

Deferred (own PRs, noted on the issue): #4 Redis M×EFC index-name collision
(tests hardcode "idx"; needs Redis/Valkey/Dragonfly validation), #5 per-search
watchdog, #7 Vertex indexSyncTime wait, #8 streaming percentiles.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.

2 participants