xiangfu0 commented on PR #19303: URL: https://github.com/apache/pinot/pull/19303#issuecomment-5485829066
Addressed the seven items from the latest review on head `ea22c5d4d416987d4ec7bf3704edd168337b8d6c`: 1. Replaced `maybeRefreshBlocking()` with generation-aware coalescing. One caller refreshes the captured writer sequence while same-generation callers wait; a caller targeting a genuinely newer sequence performs the next refresh. The test now deterministically holds the winner after it publishes the in-progress state, proves another caller entered the production wait branch, then verifies all eight calls complete with exactly one refresh. I also added an 8-reader/1-writer workload and compared the original and updated heads below. 2. Empty bitmaps return before NRT refresh and before search-executor submission. The test occupies the only search worker, requires the empty-filter call to finish within one second, and asserts zero refreshes. 3. Added mutable HNSW selectivity coverage at 100%, 10%, 1%, and 0.1% over 25,000 vectors. Updated-head recall/p50/p95 were: `0.3550/486.5/900.1 µs`, `0.8600/798.0/1170.3 µs`, `1.0000/338.3/431.4 µs`, and `1.0000/207.0/242.5 µs`. 4. Reworked the live-writer test into phased, deterministic overlap inside Lucene filter evaluation. Cleanup force-terminates synchronization and interrupts/joins the writer with a bound. 5. Made the IVF-PQ case non-vacuous: the query document and nearest unfiltered neighbor are excluded, and the test requires the exact farther allowed documents. 6. Renamed the shared class to `BasePinotDocIdBitmapFilterQuery` and added direct scorer/cache/identity/visitor tests. 7. Rewrote the single commit message and PR body to match the actual implementation; the stale per-instance-directory claim and stored-field wording are gone. Concurrent workload, same revised harness and settings (`4k` initial + `4k` adds, 8 readers, 1% filter): | Head | Ingest docs/s | Recall@10 | p50 (µs) | p95 (µs) | Wall QPS | | --- | ---: | ---: | ---: | ---: | ---: | | Original `322c2bd1f6` | 12,512.2 | 1.0000 | 7,800.6 | 287,008.0 | 123.4 | | Updated `ea22c5d4d4` | 13,445.1 | 1.0000 | 9,050.8 | 260,266.0 | 134.0 | Validation: 77 targeted vector tests pass, `VectorUpsertTableTest` passes under both query engines, `pinot-perf` compiles, and Spotless/Checkstyle/license format/license check pass on all five affected modules. @Jackie-Jiang please take another look when convenient. -- 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]
