john-mlika opened a new pull request, #16622: URL: https://github.com/apache/lucene/pull/16622
`MergeState` asks every reader for a merge instance when a merge starts, and `Lucene99HnswVectorsReader` passes that on to its flat reader. For quantized fields the flat reader is `Lucene104ScalarQuantizedVectorsReader` (or, for older segments, the `Lucene99` and `Lucene102` quantized readers in backward-codecs), and that's where the request dies: these readers wrap a `rawVectorsReader` but don't override `getMergeInstance()`, so they return `this` and the raw reader never hears about the merge. Same for `finishMerge()`. What that costs depends on the raw reader. In Lucene it is `Lucene99FlatVectorsReader`, whose merge instance is the same reader with its `.vec` input switched to sequential read advice, so quantized fields miss an optimization that plain float fields already get on every merge. A raw reader that keeps a separate input for merging would hand that back instead, and it never gets the chance either. I ran into this in Elasticsearch, where the direct-I/O vector stack has one reader for searches and one for merges and chooses between them in `getMergeInstance()`. On `int8_hnsw` fields the choice never happened, so every merge read the whole source `.vec` through the search reader. With direct I/O that is one 8 KiB device round-trip at a time, and twice over the file, once for the integrity check and once for the copy. Jim Ferenczi fixed the links Elasticsearch owns in elastic/elasticsearch#153423 and left this one in the commit message: "`Lucene104ScalarQuantizedVectorsReader` (`int8_hnsw` / `int4_hnsw`) overrides neither method, so nothing below it is reached for those field types. It keeps its `rawVectorsReader` private with no copy constructor, so the equivalent fix cannot be made from the Elasticsearch subclass and belongs upstream." So here it is. The change is small. Each reader gets a copy constructor that shares the original's open state and takes a different `rawVectorsReader`. `getMergeInstance()` returns a copy built around `rawVectorsReader.getMergeInstance()`, and `finishMerge()` forwards to the raw reader. That is how `Lucene99HnswVectorsReader` already handles its own flat reader. The `fields` map moves out of its field initializer into the main constructor, because a final field with an initializer can't be assigned by the copy constructor. On the two non-final readers the copy constructor is `protected`, and the `getMergeInstance()` javadoc explains that a subclass with state of its own has to override it and build its own copy, or it gets a plain base-class merge instance. `Lucene99ScalarQuantizedVectorsReader` is final, so its constructor stays private. A merge instance is a view over the live reader's resources: the merging thread uses it and nobody closes it, the same as the HNSW readers' merge instances. If the raw reader's `getMergeInstance()` returns `this`, the copy reads through the same objects and the only effect is whatever the raw reader did along the way. No file format changes. I included the two backward-codecs readers because they still serve every merge whose sources are pre-10.4 scalar-quantized or 10.2 binary-quantized segments, which is what an upgrade merge reads for anyone who had quantized vectors before 10.4. I can split them into a follow-up if you'd rather keep this to core. A few limits. This only matters when the merge reads through a pooled reader, so on an index that is serving searches, or NRT. Without a pooled reader, `IndexWriter` opens the sources with a merge context that `MMapDirectory` already maps to sequential advice. Merges that go through `MergePolicy#reorder` or `addIndexes(CodecReader...)` are wrapped in readers that don't propagate merge instances at all, before or after this. And while a merge runs, a quantized field's `.vec` is on sequential advice for concurrent searches that read full-precision vectors, which is already the case for plain float fields. -- 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]
