Removing the vec file extension from INDEX_STORE_HYBRID_NIO_EXTENSIONS, to ensure the no performance degradation for vector search via Lucene Engine.

[navneet1v]

Description

Removing the vec file extension from INDEX_STORE_HYBRID_NIO_EXTENSIONS, to ensure the no performance degradation for vector search via Lucene Engine.

This PR: https://github.com/opensearch-project/OpenSearch/pull/8508/files added .vec file extension in INDEX_STORE_HYBRID_NIO_EXTENSIONS and deprecated the setting : INDEX_STORE_HYBRID_MMAP_EXTENSIONS. Which made .vec not to be Mmapped. This resulted in below problems:

1. K-NN plugin was adding the required files like .vec and .vex to INDEX_STORE_HYBRID_MMAP_EXTENSIONS to make sure that files are Mmapped, for best performance. But now as the setting is deprecated the Unit test started to fail and K-NN plugin need to hack around to make sure that unit tests are succeeding. https://github.com/opensearch-project/k-NN/blob/3df8308bbe7ce6559b559f32920c31a6b0ae3f20/src/test/java/org/opensearch/knn/index/KNNSettingsTests.java#L67-L70
2. As the setting is going to be deprecated there is no point in using that setting. Hence to make sure that K-NN plugin can remove this piece of code I am removing .vec file from INDEX_STORE_HYBRID_NIO_EXTENSIONS.

This change will ensure that no custom logic is present in k-NN plugin going forward and if new list get created going forward k-NN plugin doesn't require any change.

Related Issues

NA

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Commits are signed per the DCO using --signoff
• Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[navneet1v]

Do we know what kind of impact this will have on the Lucene vector files?

@msfroh vec and vem lucene files only. These files are used by Lucene to store information related to vectors.

we probably should have always had .vec in the MMap list

Yes we should had it always. The way it was happening earlier was K-NN plugin was adding them(vec and vex files) from 2.5 version of OpenSearch to MMap Extension list. After the release of OpenSearch 2.4 we found that k-NN search for Lucene engine became slower. Please refer these graphs for lucene: opensearch-project/k-NN#576 (comment) . The same was also reported by a customer opensearch-project/k-NN#637 (comment) (detailed analysis is there) after they upgrade to OpenSearch 2.4 from OpenSearch 2.3 . Some workaround was also provided to customers but we fixed the issue in 2.5 release of OpenSearch.

This is the reason I am pushing towards a better solution, which is removing .vec file from NIO file extension list and completely remove the logic from K-NN plugin. If we don't do that K-NN plugin needs to keep on overriding the files in future which is not a scalable solution as these settings can be deprecated and new settings can come in future.

I am adding @martin-gaievski from k-NN plugin maintainers who did the deep-dive if he wants to add anything else.

[navneet1v]

For reference I am adding the quick benchmarks that I did after removing the k-NN logic which add these files in the MMAP.

{
  "metadata": {
    "test_name": "Lucene",
    "test_id": "Lucene Index workflow",
    "date": "08/24/2023 16:02:23",
    "python_version": "3.8.16 (default, Mar  2 2023, 03:21:46) \n[GCC 11.2.0]",
    "os_version": "Linux-5.4.250-173.369.amzn2int.x86_64-x86_64-with-glibc2.17",
    "processor": "x86_64, 16 cores",
    "memory": "21340106752 (used) / 43812683776 (available) / 65887473664 (total)"
  },
  "results": {
    "test_took": 265919.0,
    "query_took_total": 265919.0,
    "query_took_p50": 27.0,
    "query_took_p90": 33.0,
    "query_took_p99": 36.0,
    "query_took_p99.9": 38.5,
    "query_took_p100": 53.4,
    "query_client_time_total": 292433.4,
    "query_client_time_p50": 30.0,
    "query_client_time_p90": 35.0,
    "query_client_time_p99": 38.9,
    "query_client_time_p99.9": 54.9,
    "query_client_time_p100": 301.0,
    "query_memory_kb_total": 0.0,
    "query_recall@K_total": 0.9990970000000001,
    "query_recall@1_total": 1.0
  }
}

if vec and vex files are mmaped.

{
  "metadata": {
    "test_name": "Lucene",
    "test_id": "Lucene Index workflow",
    "date": "08/23/2023 20:47:04",
    "python_version": "3.8.16 (default, Mar  2 2023, 03:21:46) \n[GCC 11.2.0]",
    "os_version": "Linux-5.4.250-173.369.amzn2int.x86_64-x86_64-with-glibc2.17",
    "processor": "x86_64, 16 cores",
    "memory": "20670394368 (used) / 44482461696 (available) / 65887473664 (total)"
  },
  "results": {
    "test_took": 68728.0,
    "query_took_total": 68728.0,
    "query_took_p50": 7.0,
    "query_took_p90": 8.0,
    "query_took_p99": 9.0,
    "query_took_p99.9": 10.0,
    "query_took_p100": 97.0,
    "query_client_time_total": 92300.0,
    "query_client_time_p50": 9.0,
    "query_client_time_p90": 10.0,
    "query_client_time_p99": 11.0,
    "query_client_time_p99.9": 47.0,
    "query_client_time_p100": 277.0,
    "query_memory_kb_total": 0.0,
    "query_recall@K_total": 0.999083,
    "query_recall@1_total": 1.0
  }
}

[msfroh]

Right -- my question was less around the k-nn plugin vector files and more about the Lucene vector files (where both implementations decided to use .vec as an extension).

I'm pretty sure the Lucene vector files should be MMapped -- do we have numbers to confirm?

[navneet1v]

Right -- my question was less around the k-nn plugin vector files and more about the Lucene vector files (where both implementations decided to use .vec as an extension).

I'm pretty sure the Lucene vector files should be MMapped -- do we have numbers to confirm?

@msfroh what I am saying is K-NN plugin uses the Lucene Vector field, when customer try to use Lucene engine for vector search. So, vec and vex are Lucene files only. They are not k-NN plugin specific.

This is the quick benchmarks I have done: #9528 (comment)

[msfroh]

what I am saying is K-NN plugin uses the Lucene Vector field, when customer try to use Lucene engine for vector search. So, vec and vex are Lucene files only. They are not k-NN plugin specific.

Okay, thanks! Now I (almost) get it. I'm a little slow. (So what's up with the .vex versus .vem difference?)

[navneet1v]

what I am saying is K-NN plugin uses the Lucene Vector field, when customer try to use Lucene engine for vector search. So, vec and vex are Lucene files only. They are not k-NN plugin specific.

Okay, thanks! Now I (almost) get it. I'm a little slow. (So what's up with the .vex versus .vem difference?)

@msfroh

Here is the official documentation: https://lucene.apache.org/core/9_7_0/core/org/apache/lucene/codecs/lucene95/Lucene95HnswVectorsFormat.html

So .vex is the main file that has the HNSW graph present. and during the vector search this is an important file
.vem has the metadata info.
.vec has all the vectors.

As per the last deep-dive done by @martin-gaievski vec and vex needs to be mmaped for performance. .vem didn't have impact on the latency. Hence vec and vex needs to be MMapped.

@martin-gaievski please add anything if I have missed.

[martin-gaievski]

what I am saying is K-NN plugin uses the Lucene Vector field, when customer try to use Lucene engine for vector search. So, vec and vex are Lucene files only. They are not k-NN plugin specific.

Okay, thanks! Now I (almost) get it. I'm a little slow. (So what's up with the .vex versus .vem difference?)

@msfroh

Here is the official documentation: https://lucene.apache.org/core/9_7_0/core/org/apache/lucene/codecs/lucene95/Lucene95HnswVectorsFormat.html

So .vex is the main file that has the HNSW graph present. .vem has the metadata info. .vec has all the vectors.

As per the last deep-dive done by @martin-gaievski vec and vex needs to be mmaped for performance. .vem didn't have impact on the latency. Hence vec and vex needs to be MMapped.

@martin-gaievski please add anything if I have missed.

Confirming, as per our testing vec and vex are contributing a lot to performance of both ingestion and search, we didn't see effect of vem beeing mmaped.

[msfroh]

Awesome! Thanks for clearing all of that up.

Feel free to @ me as soon as you've fixed the changelog conflict and I'm happy to merge this.

[navneet1v]

@msfroh fixed the conflicts. Please check

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: SUCCESS ✅
• URL: https://build.ci.opensearch.org/job/gradle-check/23423/
• CommitID: 88f255f