From f64d39a6973d96444fac8125f1a92265c4925c77 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Wed, 21 Dec 2022 10:12:02 -0800 Subject: [PATCH] Allow mapping service to be null for scenarious of shard recovery from translog (#685) (#687) Signed-off-by: Martin Gaievski (cherry picked from commit c412c8a403cf2c4b6008df7b36847f2ee329e887) Co-authored-by: Martin Gaievski (cherry picked from commit 04f677ef457317832144b71d155276e4af7360ec) --- .../knn/index/codec/KNNCodecVersion.java | 4 +- .../knn/index/codec/KNNCodecServiceTests.java | 67 +++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 src/test/java/org/opensearch/knn/index/codec/KNNCodecServiceTests.java diff --git a/src/main/java/org/opensearch/knn/index/codec/KNNCodecVersion.java b/src/main/java/org/opensearch/knn/index/codec/KNNCodecVersion.java index adbbb01ca..a6d699484 100644 --- a/src/main/java/org/opensearch/knn/index/codec/KNNCodecVersion.java +++ b/src/main/java/org/opensearch/knn/index/codec/KNNCodecVersion.java @@ -56,7 +56,7 @@ public enum KNNCodecVersion { ), (userCodec, mapperService) -> KNN920Codec.builder() .delegate(userCodec) - .knnVectorsFormat(new KNN920PerFieldKnnVectorsFormat(Optional.of(mapperService))) + .knnVectorsFormat(new KNN920PerFieldKnnVectorsFormat(Optional.ofNullable(mapperService))) .build(), KNN920Codec::new ), @@ -71,7 +71,7 @@ public enum KNNCodecVersion { ), (userCodec, mapperService) -> KNN940Codec.builder() .delegate(userCodec) - .knnVectorsFormat(new KNN940PerFieldKnnVectorsFormat(Optional.of(mapperService))) + .knnVectorsFormat(new KNN940PerFieldKnnVectorsFormat(Optional.ofNullable(mapperService))) .build(), KNN940Codec::new ); diff --git a/src/test/java/org/opensearch/knn/index/codec/KNNCodecServiceTests.java b/src/test/java/org/opensearch/knn/index/codec/KNNCodecServiceTests.java new file mode 100644 index 000000000..2599200e6 --- /dev/null +++ b/src/test/java/org/opensearch/knn/index/codec/KNNCodecServiceTests.java @@ -0,0 +1,67 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.knn.index.codec; + +import org.apache.lucene.codecs.Codec; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.common.settings.Settings; +import org.opensearch.index.Index; +import org.opensearch.index.IndexSettings; +import org.opensearch.index.codec.CodecServiceConfig; +import org.opensearch.index.mapper.MapperService; +import org.opensearch.knn.KNNTestCase; + +import org.apache.logging.log4j.Logger; + +import java.util.UUID; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Test for KNNCodecService class with focus on codec by name lookup + */ +public class KNNCodecServiceTests extends KNNTestCase { + private static final String TEST_INDEX = "test-index"; + private static final int NUM_OF_SHARDS = 1; + private static final UUID INDEX_UUID = UUID.randomUUID(); + + private IndexSettings indexSettings; + + @Override + public void setUp() throws Exception { + super.setUp(); + IndexMetadata indexMetadata = mock(IndexMetadata.class); + when(indexMetadata.getIndex()).thenReturn(new Index(TEST_INDEX, INDEX_UUID.toString())); + when(indexMetadata.getSettings()).thenReturn(Settings.EMPTY); + Settings settings = Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, Integer.toString(NUM_OF_SHARDS)).build(); + indexSettings = new IndexSettings(indexMetadata, settings); + } + + public void testGetCodecByName() { + MapperService mapperService = mock(MapperService.class); + Logger loggerMock = mock(Logger.class); + CodecServiceConfig codecServiceConfig = new CodecServiceConfig(indexSettings, mapperService, loggerMock); + KNNCodecService knnCodecService = new KNNCodecService(codecServiceConfig); + Codec codec = knnCodecService.codec(KNNCodecVersion.current().getCodecName()); + assertNotNull(codec); + } + + /** + * This test case covers scenarios when MapperService is null, for instance this may happen during index.close operation. + * In such scenario codec is not really required but is created as part of engine initialization, please check code references below: + * @see EngineConfig.java + * @see IndexShard.java + * @see Engine.java + */ + public void testGetCodecByNameWithNoMapperService() { + Logger loggerMock = mock(Logger.class); + CodecServiceConfig codecServiceConfig = new CodecServiceConfig(indexSettings, null, loggerMock); + KNNCodecService knnCodecService = new KNNCodecService(codecServiceConfig); + Codec codec = knnCodecService.codec(KNNCodecVersion.current().getCodecName()); + assertNotNull(codec); + } +}