Introduce extensible KNNEngine API and BuiltinKNNEngine enumeration

[reta]

Description

This is a first change towards supporting extensibility (with respect to KNN/Vector libraries and engines) in the k-NN plugin. The change is small and contained, and does two things:

• introduces KNNEngine interface (by pivoting off the enumeration)
• introduces BuiltinKNNEngine enumeration (basically, mirror of the KNNEngine that exists at the moment) to contain the changeset and usage

Why?

In scope of opensearch-project/OpenSearch#20050, we have discussed the possible ways how new KNN/Vector engines could be introduced, and most importantly, co-exist with the ones provided by K-NN plugin. The KNNEngine is one of the key component here, unfortunately is it designed as enumeration and is not extensible (from the outside) but fixed.

To break that barrier, this first change converts the KNNEngine into interface but keeps the previous revision of the enumeration as BuiltinKNNEngine. It dramatically reduces the changes required (99% of them is just rename KNNEngine -> BuiltinKNNEngine), but at the same time keeps the API / usage mostly unaffected.

What is next?

This changes unblock the possibility to extract KNN Engine/Library SPIs.

Related Issues

Towards opensearch-project/OpenSearch#20050

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

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.

[reta]

@vamshin @navneet1v would really appreciate your thoughts folks, the pull request look large but 99% of changes is just name change KNNEngine -> BuiltinKNNEngine whereas KNNEngine becomes an interface, that's about to be fair. Thank you.

[codecov]

Codecov Report

❌ Patch coverage is 76.32850% with 49 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.27%. Comparing base (224e288) to head (cb6b1c0).

Files with missing lines	Patch %	Lines
...search/knn/index/engine/AbstractKNNEngineBase.java	16.00%	21 Missing ⚠️
...c/main/java/org/opensearch/knn/jni/JNIService.java	50.00%	1 Missing and 10 partials ⚠️
.../opensearch/knn/index/engine/BuiltinKNNEngine.java	91.89%	5 Missing and 1 partial ⚠️
...java/org/opensearch/knn/index/query/KNNWeight.java	50.00%	0 Missing and 2 partials ⚠️
...nsearch/knn/plugin/rest/RestTrainModelHandler.java	33.33%	2 Missing ⚠️
...c/KNN9120Codec/KNN9120HnswBinaryVectorsFormat.java	0.00%	1 Missing ⚠️
...cs/KNN950Codec/KNN950PerFieldKnnVectorsFormat.java	0.00%	1 Missing ⚠️
...cs/KNN990Codec/KNN990PerFieldKnnVectorsFormat.java	0.00%	1 Missing ⚠️
...rg/opensearch/knn/index/engine/EngineResolver.java	90.00%	0 Missing and 1 partial ⚠️
...rg/opensearch/knn/index/query/KNNQueryFactory.java	50.00%	0 Missing and 1 partial ⚠️
... and 2 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #3288      +/-   ##
============================================
- Coverage     83.39%   83.27%   -0.13%     
- Complexity     4267     4274       +7     
============================================
  Files           450      452       +2     
  Lines         15511    15543      +32     
  Branches       2006     2007       +1     
============================================
+ Hits          12936    12944       +8     
- Misses         1780     1804      +24     
  Partials        795      795              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[vamshin]

@vamshin @navneet1v would really appreciate your thoughts folks, the pull request look large but 99% of changes is just name change KNNEngine -> BuiltinKNNEngine whereas KNNEngine becomes an interface, that's about to be fair. Thank you.

Thanks @reta. We have a similar idea here to create a Sandbox module/package. #3283. Should we incorporate the code changes in this direction?

Also regarding the changes, wondering if we could reserve kNNEngine name for built in Engine(we can be clear about it in the javadoc) and choose another name for interface and other implementations can prefix their name to kNNEngine?

[reta]

Thanks @vamshin

Thanks @reta. We have a similar idea here to create a Sandbox module/package. #3283. Should we incorporate the code changes in this direction?

Yes. totally possible as well, the end goal would be to have SPI module, which also aligns well with the sandbox idea. I didn't want to pull out new module for a sake of more contained change but that would come next.

Also regarding the changes, wondering if we could reserve kNNEngine name for built in Engine(we can be clear about it in the javadoc) and choose another name for interface and other implementations can prefix their name to kNNEngine?

This is possible, however that would explode the amount of changes. KNNEngine is a very key type which is used literally everywhere (including https://github.com/Opensearch-project/neural-search), however we could introduce another interface XxxKNNEngine -> KNNEngine if necessary indeed.

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit cb6b1c0.

'Diff too large, requires skip by maintainers after manual review'

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.