Skip to content

[Fix] [DiskSeismic]: take the exact-match path when the ID selector is smaller than k - #52

Merged
chishui merged 2 commits into
opensearch-project:mainfrom
zirui-song-18:fix-disk-seismic-exact-match
Sep 9, 2026
Merged

[Fix] [DiskSeismic]: take the exact-match path when the ID selector is smaller than k#52
chishui merged 2 commits into
opensearch-project:mainfrom
zirui-song-18:fix-disk-seismic-exact-match

Conversation

@zirui-song-18

Copy link
Copy Markdown
Collaborator

Description

SeismicIndex and SeismicScalarQuantizedIndex take an exact-match path when the query carries an enumerable IDSelector of size <= k: they score exactly the selected docs and return all of them. The disk-resident indexes (DiskSeismicIndex, DiskSeismicScalarQuantizedIndex) did not — they always ran the block-budget search and applied the selector only as a post-filter over the top-k' blocks, so a selected doc that fell in no scored block was silently dropped. A field filtered to fewer than k docs could therefore return too few results (or none).

This routes the disk-resident indexes through the exact-match path too:

  • In-RAM and mmap-CSR builds (get_vectors() is doc-id-addressable): reuse ExactMatcher.
  • mmap-loaded serialized index (no in-RAM vectors): add a per-doc locator directory plus a small remainder store, so every doc's full vector is reachable and the selected docs can be scored directly. Most docs point at an existing inline-forward block slot; docs pruned from every posting list (absent from all blocks) keep their full vector in the remainder store. Resolved doc vectors are scored against the dense query and returned.

Also hardened along the way: ExactMatcher now skips selector ids outside the index instead of indexing out of bounds, and the mapped remainder lookup is bounds-checked (fail-closed, matching the rest of the mmap read path).

Format / cost. The serialized index gains a locator-directory + remainder section after the inline forward. Overhead is a flat ~12 bytes/doc (the directory); the remainder is empty whenever posting-list pruning drops nothing. On a ~23.6 GB base_small index this measured +1.2 MB (+0.005%). The directory and remainder are borrowed from the mapping (not copied to the heap), so resident memory is unchanged; normal (non-filtered) queries take a guarded early check and the identical block-budget path, so their latency is unchanged.

Testing

  • New unit tests across float / scalar-quantized × in-RAM / mmap: a selector of size <= k returns every member; the mapped path returns the same members and scores as the in-RAM ExactMatcher; the remainder (fully-pruned-doc) read path; an out-of-range selector member is skipped.
  • Full suite green: 646 nsparse_test tests pass.
  • Real data (base_small, 100k docs): a filter of 8 scattered docs at k'=1 returns all of them with scores matching an exact brute-force dot product.

Issues Resolved

#49

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

…tor is smaller than k

Signed-off-by: Zirui Song <zrsong@amazon.com>

@chishui chishui left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this change impact latency? have you benchmarked?

Comment thread nsparse/disk_seismic_index_base.cpp Outdated
index_mapping_ = std::move(mapped);
}

auto DiskSeismicIndexBase::exact_match_mapped(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not necessary to name with "_mapped"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack

Comment thread nsparse/disk_seismic_index_base.cpp Outdated
static_cast<uint64_t>(doc_id) >= num_locators_) {
continue; // out-of-range member: nothing to score
}
const detail::DocLocator loc = doc_locators_[doc_id];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from 371-399 is basically a get_doc function. Also, I'm wondering if we can use a same exact_match function with get_doc an input to differenciate the exact_match in seismic and disk

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack

Signed-off-by: Zirui Song <zrsong@amazon.com>
@zirui-song-18

Copy link
Copy Markdown
Collaborator Author

does this change impact latency? have you benchmarked?

It won't impact latency.

  ┌────────────────────────┬──────────────────────┬──────────────────────┬──────────────────────────────┐
  │         Metric         │ BEFORE (origin/main) │ AFTER (HEAD 1abc63e) │              Δ               │
  ├────────────────────────┼──────────────────────┼──────────────────────┼──────────────────────────────┤
  │ single-query mean      │ 0.1351 ms            │ 0.1336 ms            │ −1.1% (noise)                │
  ├────────────────────────┼──────────────────────┼──────────────────────┼──────────────────────────────┤
  │ p50                    │ 0.1193 ms            │ 0.1169 ms            │ −2% (noise)                  │
  ├────────────────────────┼──────────────────────┼──────────────────────┼──────────────────────────────┤
  │ p90                    │ 0.2280 ms            │ 0.2249 ms            │ −1.4% (noise)                │
  ├────────────────────────┼──────────────────────┼──────────────────────┼──────────────────────────────┤
  │ p99                    │ 0.3187 ms            │ 0.3122 ms            │ −2% (noise)                  │
  ├────────────────────────┼──────────────────────┼──────────────────────┼──────────────────────────────┤
  │ batch qps (steady)     │ ~7720                │ ~7790                │ +0.9% (noise)                │
  ├────────────────────────┼──────────────────────┼──────────────────────┼──────────────────────────────┤
  │ RssAnon (after search) │ 0.1353 GiB           │ 0.1353 GiB           │ 0                            │
  ├────────────────────────┼──────────────────────┼──────────────────────┼──────────────────────────────┤
  │ RssFile (after search) │ 2.724 GiB            │ 2.683 GiB            │ −41 MB (page-cache variance) │
  ├────────────────────────┼──────────────────────┼──────────────────────┼──────────────────────────────┤
  │ .dat size              │ 23,561,215,384 B     │ 23,564,026,528 B     │ +2,811,144 B (+0.012%)       │
  └────────────────────────┴──────────────────────┴──────────────────────┴──────────────────────────────┘

@chishui
chishui merged commit 354c756 into opensearch-project:main Sep 9, 2026
10 checks passed
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