You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "xiangfu0 (via GitHub)" <gi...@apache.org> on 2026/09/05 01:26:02 UTC
[PR] Reopen the mutable HNSW searcher on a background thread instead of on the query path [pinot]
xiangfu0 opened a new pull request, #19467:
URL: https://github.com/apache/pinot/pull/19467
## Problem
A filtered vector search on a consuming segment must see every row the query's visible-document set names,
including rows still in the writer's RAM buffer, so it reopened the near-real-time searcher on the query thread.
That reopen flushes the writer and builds an HNSW graph over the flushed rows. Under continuous ingestion nearly
every filtered query finds new rows, so **segment creation was driven by query rate rather than ingestion rate**.
## Change
Queries publish the writer generation they need and wait; one background thread per index performs the reopen, so
concurrent queries needing the same generation share one flush instead of forcing one each.
## Measured
Same harness, same machine, previous behavior vs this change. The workload is added here
(`BenchmarkVectorFilterWorkloads`), since no existing scenario exercised query-time reopen at all.
| ingest | metric | before | after |
|---|---|---|---|
| 2000 docs/s, 8 readers | QPS | 1104 | **1645** |
| | p50 / p95 / p99 | 4.0 / 14.7 / 61.3 ms | **3.2 / 9.5 / 34.4 ms** |
| unthrottled (~18k docs/s) | QPS | 36 | **146** |
| | p50 | 111.8 ms | **15.5 ms** |
Ingestion throughput is unchanged in both. Roughly 8 queries are served per reopen, so reopens fall about 4x at
the default while every query metric improves.
`refreshMinIntervalMs` was chosen from a sweep rather than picked. Raising it cuts reopens further but costs
throughput and latency fast — at 10ms the result lands *below* the behavior it replaces (400 QPS, p50 14.4ms).
1ms is the setting that beats the old behavior on throughput, p50, p95 and p99 simultaneously.
## Why not `ControlledRealTimeReopenThread`
Two reasons, both bearing on correctness here.
`ReferenceManager#doMaybeRefresh` calls `notifyRefreshListenersRefreshed(refreshed)` from a `finally`, and that
class's listener ignores the flag and publishes its searching generation regardless. **A reopen that threw still
advertises the generation it merely attempted** — a query would then search a stale searcher and silently drop
rows its filter names, which is worse than the cost being fixed. Its loop also reopens on a fixed cadence whether
or not anyone is waiting, which on a consuming segment always flushes, since rows are always buffered.
The loop here publishes only after a reopen returns normally, and runs only while a query is waiting.
## Failure behavior
A reopen failure or a wait past `refreshWaitTimeoutMs` (default 5s) **fails the query** rather than answering it
from a searcher that may not hold its rows. This is a new failure mode on this path — previously such a query
blocked until the refresh finished. Retries back off with a cap and the error log is rate-limited, so a
persistently failing writer cannot spin the loop or flood the log.
## Config
Two new HNSW-only keys, both registered with `VectorIndexConfigValidator` and parsed there exactly as the
consumer parses them, so a config the controller accepts cannot then halt ingestion on a server:
- `refreshMinIntervalMs` (default 1) — spacing between reopens; 0 disables spacing.
- `refreshWaitTimeoutMs` (default 5000) — how long a filtered query waits before failing.
Rolling upgrade is safe: unknown keys are already tolerated by the validator, and no wire, segment or ZK format
changes.
## Known trade-off
This starts one thread per (consuming segment x vector column), where Pinot's `RealtimeLuceneIndexRefreshManager`
caps text-index refresh at 1 thread server-wide. Reusing it is not a drop-in — it is fixed-cadence round-robin
with no generation contract, and the correctness requirement here is a wait on a writer sequence number. Moving
reopen execution onto a bounded, server-configured pool while keeping the generation handshake here is the
natural follow-up.
Follow-up to #19303 and #19464.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Reopen the mutable HNSW searcher on a background thread instead of on the query path [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19467:
URL: https://github.com/apache/pinot/pull/19467#issuecomment-5548640977
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/19467?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `10.08403%` with `107 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 57.77%. Comparing base ([`e6787e1`](https://app.codecov.io/gh/apache/pinot/commit/e6787e178645fd3c5a887e2aba7e0de8343cd524?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`439e85d`](https://app.codecov.io/gh/apache/pinot/commit/439e85d7634e231f24fe11872574af4b04b1d962?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/19467?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...local/realtime/impl/vector/MutableVectorIndex.java](https://app.codecov.io/gh/apache/pinot/pull/19467?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Frealtime%2Fimpl%2Fvector%2FMutableVectorIndex.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL3ZlY3Rvci9NdXRhYmxlVmVjdG9ySW5kZXguamF2YQ==) | 0.00% | [107 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19467?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (e6787e1) and HEAD (439e85d). Click for more details.
>
> <details><summary>HEAD has 24 uploads less than BASE</summary>
>
>| Flag | BASE (e6787e1) | HEAD (439e85d) |
>|------|------|------|
>|unittests|2|1|
>|java-25|6|1|
>|temurin|6|1|
>|lane-a|2|0|
>|integration|4|0|
>|integration1|2|0|
>|lane-b|2|0|
>|integration2|2|0|
>|unittests2|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #19467 +/- ##
============================================
- Coverage 67.69% 57.77% -9.93%
+ Complexity 1430 1 -1429
============================================
Files 3488 2685 -803
Lines 224306 164182 -60124
Branches 35416 26661 -8755
============================================
- Hits 151850 94855 -56995
- Misses 60447 61298 +851
+ Partials 12009 8029 -3980
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/19467/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration](https://app.codecov.io/gh/apache/pinot/pull/19467/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/19467/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/19467/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-25](https://app.codecov.io/gh/apache/pinot/pull/19467/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.77% <10.08%> (-9.93%)` | :arrow_down: |
| [lane-a](https://app.codecov.io/gh/apache/pinot/pull/19467/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [lane-b](https://app.codecov.io/gh/apache/pinot/pull/19467/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/19467/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.77% <10.08%> (-9.93%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/19467/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.77% <10.08%> (-9.93%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/19467/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.77% <10.08%> (-0.03%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/19467/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/pinot/pull/19467?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Reopen the mutable HNSW searcher on a background thread instead of on the query path [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #19467:
URL: https://github.com/apache/pinot/pull/19467#issuecomment-5556131243
Documentation follow-up: https://github.com/pinot-contrib/pinot-docs/pull/1033
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Reopen the mutable HNSW searcher on a background thread instead of on the query path [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #19467:
URL: https://github.com/apache/pinot/pull/19467
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org