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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to