-
Notifications
You must be signed in to change notification settings - Fork 140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FEATURE] Integrate Jvector engine as another vector engine of choice #2386
Comments
Thanks @sam-herman. Jvector looks very cool and would be a good addition. I think to add this, we would need clean/formalize engine abstractions/interfaces to make vector engine's truly extendable. We have done this to some degree, but definitely need to go further. |
@jmazanec15 can you share the plans around this? JFYI, I will probably have a POC ready in about a week or so with JVector integrated in k-NN. Let me know what is the best issue/RFC to contribute to the intended design around it. |
There arent any formal plans, but a trail of issues:
I think long term goal is to provide interface for new engines and clean separation between core k-NN plugin and different engines. We started doing that in 2.17 with https://github.com/opensearch-project/k-NN/tree/main/src/main/java/org/opensearch/knn/index/engine. Theres still work todo, but idea is to get away from having branches on engines. |
Thanks for sharing those. |
@navneet1v @jmazanec15 I added more bullet points in the description to the "added value" category. I'm hoping to add the output of a few benchmarks to support those pretty soon. But hopefully for the time being it can provide some qualitative measure. |
@sam-herman thanks for providing the details. Just to provide a clarity I am not against adding JVector in k-NN plugin, I think in initial integration this should be an Optional Vector engine. Please find my response below:
K-NN plugin is very much involved/invested in FAISS and native code(BTW lucene is also coming up with Faiss integration: https://github.com/apache/lucene/pull/14178/files). Even though JVector has the benefits from a k-NN plugin standpoint if you look at, Faiss gives us all the different quantization including 32x compression based search. So JVector is not just a drop in replacement.
This is really good, but since Faiss/Lucene HNSW Index which are created at segment level. And segments become searchable only when refresh/flush on IndexReaders is completed. We don't go into scenarios where we need to do search during indexing. Hence, this capability might not even be used.
In k-NN plugin we currently support 1x, 2x, 4x, 8x, 16x, 32x and above quantization techniques. So the quantizations as feature is present, so would like to know more on this.
In 2.17 version of k-NN plugin we launched disk based vector search support https://opensearch.org/docs/latest/search-plugins/knn/disk-based-vector-search/ which has quantization and re-ranking support. I think we should compare this performance of k-NN disk based vector search with JVector disk ann implementation. This will help us take a informed decision.
With Faiss and new Quantization framework support for both PQ and BQ is present in k-NN plugin. But some of the techniques like ADC is still getting worked upon. @Vikasht34 and @jmazanec15 can add more here. But this is where I think we can reuse some of the Quantization techniques provided by JVector.
@jmazanec15 and @Vikasht34 please comment on this.
This one I want to understand more. Since vector indices in Opensearch are stored at segment level how you think a transfer will look like? and what is the meaning of encoded vectors here. Can you please help ans following questions:
|
@navneet1v It sounds like the long term approach would be to push it down through Lucene in a similar way to FAISS which I'm open to.
What are your thoughts? |
@navneet1v I thought about this a little bit more, I do have a higher level concern regarding this statement. I think the main issue here that not all users/developers would like their KNN functionality to depend on native libraries. Also I am not aware that we have made such decision in the project as a whole to have such strong dependency for KNN functionality. In fact this is a step backwards from the original tenets of pure Java based portable implementation that OpenSearch/Lucene had been following through many years. So to summarize my concerns with the above statement:
|
I think Faiss integration in Lucene is in Sandbox, if this shift will happen
I like the idea of extras as a module. Even though other things you mentioned around builds etc, I would be able to comment once we have some POC available. But direction wise I am aligned on that. I do believe that just having the codec classes might not only work, but I don't want to bias your thinking here. If we can reduce change radius to codec that will be pretty awesome to have. Please feel free to put up a small POC on which we can iterate upon. |
The reason why I am saying k-NN is much more involved as part of JNI is, because we have added more abstractions and extended the functionalities of Faiss. One example is building the Loading and writing layer for integrating Faiss with IndexInput. We have also added capabilities for parent child relationship on top of faiss. Hence that statement.
From the inception of k-NN plugin back from opendistro for elasticsearch k-NN plugin was using JNI code. Since k-NN functionality is part of the plugin and doesn't make any impact on the min distribution, I think plugins are feel free to execute and explore any language they want.
Yes this was an interesting GH issue created sometime back, and even if you see we agree that we should move vectors fasade to core if core believes that Vector query is a core search feature. But at the same time, we are trying to make the OpenSearch core engine light weight by moving its functionalities to plugins, modules and core libs. So its kind of conflicting in that sense. |
@sam-herman @navneet1v For jvector, why not implement as a knnvectorformat and then we can hook it up as a per-field format and create mapping integration like lucene in our codec (improving the abstractions along the way)? This has come up in other places: #2414 (comment). I really think long term extension point will be this per-field vector format, its just going to take some time to get there - but having our own codec makes it difficult to integrate with other features. It seems to me though that if jvector has its own perfield KnnVectorsFormat, then we can (somewhat) easily wire it in to the existing codec - no additional jvector codec needed - thus it will be very lightweight. In terms of optional, I think initially we should integrate it into the knn codec and make it experimental, not optional. This will let people test it and give us an idea if this is something we should make optional over the long term. But, it gives some wiggle room on long term decisions, to collect more data. That being said, we should do our best to sandbox it so it doesnt cause any issues in non-experimental environments. As part of this, we would need to ensure there arent any controversial dependencies.
Yes, we are looking to enhance faiss + BQ with ADC. Still working on proposal. |
@jmazanec15 I have given this some thought, unfortunately I don't see much benefit for us for jVector being labeled as experimental suggestion doesn't provide any value to us at the moment:
Which is why I am strongly convinced moving KNN facade/query/mappers into core is the best way forward. I created this issue with core maintainers: opensearch-project/OpenSearch#17338 (EDIT: Fixed link here) |
@sam-herman Moving KNN facade/query/mappers etc to core is something which we all agree should do. But there is also an alternative path for this migration which I think we should explore, which goes like this:
This will allow the current functionality of Vector Engine in OpenSearch is not broken, feature development can still happen in k-NN plugin and all consumers of OpenSearch are also able to easily consume these changes. |
@navneet1v correct me if there is something I misunderstood by your suggestion however making KNN plugin extendable by other plugins seem to me like it may have the following issues:
I am still convinced core extension would be the right path forward, every other path I'm afraid would be a long detour which will make us work twice as much and twice as hard. |
Is your feature request related to a problem?
Currently k-NN plugin supports 3 engines, Nmslib, Faiss and Lucene.
In this change I would like to integrate JVector as another engine of choice.
There are a number of unique advantages to doing so:
What solution would you like?
Introduce JVector into K-NN plugin as another supported engine.
** Benchmarks **
Will be adding some benchmarks to illustrate the above advantages...
What alternatives have you considered?
NA
The text was updated successfully, but these errors were encountered: