jimczi commented on PR #16622: URL: https://github.com/apache/lucene/pull/16622#issuecomment-5956882012
Thanks for this, and sorry it has been sitting. A few things moved under it. #16749 landed, so `Lucene99FlatVectorsReader.getMergeInstance()` now reopens the `.vec` through its own mapping, sequential and no-reuse, released by `finishMerge()`. That makes this change more valuable than when you opened it: the gap no longer costs just a read advice hint, it means quantized fields never get the separate mapping at all. Your approach still looks right to me. It forwards to the raw reader instead of reimplementing anything, so I don't think you need to copy what #16749 did, only to reach it. Two things that changed meaning though: `finishMerge()` forwarding is now load-bearing rather than symmetry. The flat reader's merge instance holds a refcounted mapping, so without the forward it stays open until the reader is closed. `Lucene104ScalarQuantizedVectorsReader` has changed under #16682, #16687, #16696 and #16705, so the PR conflicts today and the copy constructors may need adjusting. Could you rebase on main and confirm the shape still fits? Happy to review after that. -- 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]
