Add Model cache to cache model templates in memory by jmazanec15 · Pull Request #46 · opensearch-project/k-NN

jmazanec15 · 2021-06-16T16:51:37Z

Description

For OpenSearch indices that require training, a trained model template index will need to be created and stored in the Model system index before ingestion can begin. In order for the plugin to create a segment for indices that require training, a template will need to be loaded from the Model system index and passed to the jni. If the KNNDocValuesConsumer were to make a get request to this index for each segment creation operation, indexing will be slow. The purpose of this cache is to speed up this operation by storing the model template index in memory on a given node.

Cache size is determined by a new cluster setting that is updateable. On update, the cache will be rebuilt.

Additionally, this PR refactors ModelIndex into ModelDao interface. This helps for testing. However, no core functionality of the model index has been changed.

Unit tests for the cache have been added to validate.

This PR does not introduce APIs or functionality to warmup or remove entries from the cache. In the future, we can investigate adding these APIs and functionality.

Issues Resolved

#27

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed as per the DCO using --signoff

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.

Signed-off-by: John Mazanec <jmazane@amazon.com>

VijayanB · 2021-06-16T17:19:42Z

+            (s) -> Long.toString(KNN_DEFAULT_MODEL_CACHE_SIZE_IN_BYTES),
+            (s) -> {
+                    long value = Long.parseLong(s);
+                    if (value < KNN_MIN_MODEL_CACHE_SIZE_IN_BYTES) {


nit: why not combine both into single check and error message as "value must be > 100 KB and <= 80 MB, so that user knows required setting at first failure attempt?

Sure will update

VijayanB · 2021-06-16T17:21:26Z

+
+    private static Logger logger = LogManager.getLogger(ModelCache.class);
+
+    private static ModelCache INSTANCE;


Why Upper case? i believe only constants are in Upper case

Good catch, will update

Signed-off-by: John Mazanec <jmazane@amazon.com>

VijayanB

Thanks for answers. it looks good. Will keep it as it is.

vamshin · 2021-06-30T20:45:26Z

+        @Override
+        public void delete(String modelId, ActionListener<DeleteResponse> listener) {
+            if (!isCreated()) {
+                throw new IllegalStateException("Cannot delete model \"" + modelId + "\". Model index does not exist.");


If the Model index is deleted which means model is deleted right? Why should this be an exception? May be mark as logger.info for debugging purpose?

My thinking for throwing an exception is that a user should not try to delete something that isnt there. I can switch to a log statement and return.

Btw, this function is subject to change. For instance, what happens when someone tries to delete a model that is in use by an index? This could cause problems. I will need to think it through a little more once I start implementing Model Index management APIs.

Signed-off-by: John Mazanec <jmazane@amazon.com>

vamshin · 2021-07-01T06:04:24Z

        @Override
        public void put(String modelId, KNNEngine knnEngine, byte[] modelBlob, ActionListener<IndexResponse> listener) {
+            if (!isCreated()) {
+                throw new IllegalStateException("Cannot put model in index before index has been initialized");


Question: Is it possible Model index not created and we end up with this put? If its the case, why not we create Model index on the first put and proceed?

It is possible. I think that makes sense. Will update.

Signed-off-by: John Mazanec <jmazane@amazon.com>

vamshin

LGTM! Thanks

…t#46) Signed-off-by: Jack Mazanec <jmazane@amazon.com>

Signed-off-by: Jack Mazanec <jmazane@amazon.com>

…t#46) Signed-off-by: Jack Mazanec <jmazane@amazon.com>

…t#46) Signed-off-by: Jack Mazanec <jmazane@amazon.com> Signed-off-by: Martin Gaievski <gaievski@amazon.com>

…t#46) Signed-off-by: Jack Mazanec <jmazane@amazon.com>

jmazanec15 added 3 commits June 16, 2021 09:17

Add model cache to cache model templates

bc0e49c

Signed-off-by: John Mazanec <jmazane@amazon.com>

Update model cache setting to long

a42b36f

Signed-off-by: John Mazanec <jmazane@amazon.com>

Add model cache setting to settings list

5f9f982

Signed-off-by: John Mazanec <jmazane@amazon.com>

VijayanB reviewed Jun 16, 2021

View reviewed changes

Clean up code

f181ee6

Signed-off-by: John Mazanec <jmazane@amazon.com>

jmazanec15 changed the base branch from faiss-develop to feature/faiss-support June 24, 2021 17:00

jmazanec15 requested a review from vamshin June 28, 2021 16:26

Merge branch 'feature/faiss-support' into model-cache

21d2f7b

VijayanB added the Enhancements Increases software capabilities beyond original client specifications label Jun 29, 2021

VijayanB assigned jmazanec15 Jun 29, 2021

VijayanB reviewed Jun 29, 2021

View reviewed changes

Comment thread src/main/java/org/opensearch/knn/indices/ModelCache.java

VijayanB reviewed Jun 29, 2021

View reviewed changes

Comment thread src/main/java/org/opensearch/knn/indices/ModelDao.java

VijayanB approved these changes Jun 30, 2021

View reviewed changes

jmazanec15 force-pushed the model-cache branch from 0017e7d to 21d2f7b Compare June 30, 2021 19:53

vamshin reviewed Jun 30, 2021

View reviewed changes

jmazanec15 added 6 commits June 30, 2021 14:42

Use longSetting

18dee98

Signed-off-by: John Mazanec <jmazane@amazon.com>

Update default to 10 Mb

712b213

Signed-off-by: John Mazanec <jmazane@amazon.com>

Remove private put method

539bf14

Signed-off-by: John Mazanec <jmazane@amazon.com>

Remove exception in delete

10e69de

Signed-off-by: John Mazanec <jmazane@amazon.com>

Update default to 50 Mb

37839d9

Signed-off-by: John Mazanec <jmazane@amazon.com>

Remove index created check in get model

5ed5e9e

Signed-off-by: John Mazanec <jmazane@amazon.com>

vamshin reviewed Jul 1, 2021

View reviewed changes

Create index if it doesnt exist put

919fd0e

Signed-off-by: John Mazanec <jmazane@amazon.com>

vamshin approved these changes Jul 1, 2021

View reviewed changes

jmazanec15 merged commit e4f42ff into opensearch-project:feature/faiss-support Jul 1, 2021

jmazanec15 added a commit to jmazanec15/k-NN-1 that referenced this pull request Oct 22, 2021

Add Model cache to cache model templates in memory (opensearch-projec…

6de9905

…t#46) Signed-off-by: Jack Mazanec <jmazane@amazon.com>

jmazanec15 added a commit that referenced this pull request Oct 22, 2021

Add Model cache to cache model templates in memory (#46)

64ed992

Signed-off-by: Jack Mazanec <jmazane@amazon.com>

martin-gaievski pushed a commit to martin-gaievski/k-NN that referenced this pull request Mar 7, 2022

Add Model cache to cache model templates in memory (opensearch-projec…

df6cc65

…t#46) Signed-off-by: Jack Mazanec <jmazane@amazon.com>

martin-gaievski pushed a commit to martin-gaievski/k-NN that referenced this pull request Mar 7, 2022

Add Model cache to cache model templates in memory (opensearch-projec…

af45ba5

…t#46) Signed-off-by: Jack Mazanec <jmazane@amazon.com> Signed-off-by: Martin Gaievski <gaievski@amazon.com>

martin-gaievski pushed a commit to martin-gaievski/k-NN that referenced this pull request Mar 30, 2022

Add Model cache to cache model templates in memory (opensearch-projec…

3bd99c5

…t#46) Signed-off-by: Jack Mazanec <jmazane@amazon.com>

jingqimao77-spec pushed a commit to jingqimao77-spec/k-NN that referenced this pull request Mar 15, 2026

Add Model cache to cache model templates in memory (opensearch-projec…

b23110b

…t#46) Signed-off-by: Jack Mazanec <jmazane@amazon.com>


		private static Logger logger = LogManager.getLogger(ModelCache.class);

		private static ModelCache INSTANCE;

Conversation

jmazanec15 commented Jun 16, 2021

Description

Issues Resolved

Check List

Uh oh!

VijayanB Jun 16, 2021

Choose a reason for hiding this comment

Uh oh!

jmazanec15 Jun 16, 2021

Choose a reason for hiding this comment

Uh oh!

VijayanB Jun 16, 2021

Choose a reason for hiding this comment

Uh oh!

jmazanec15 Jun 16, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

VijayanB left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vamshin Jun 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmazanec15 Jun 30, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vamshin Jul 1, 2021

Choose a reason for hiding this comment

Uh oh!

jmazanec15 Jul 1, 2021

Choose a reason for hiding this comment

Uh oh!

vamshin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vamshin Jun 30, 2021 •

edited

Loading