Skip to content

Commit

Permalink
Set additional settings at index level
Browse files Browse the repository at this point in the history
Signed-off-by: Martin Gaievski <[email protected]>
  • Loading branch information
martin-gaievski committed Jan 23, 2023
1 parent 185b54d commit 3a9ba8d
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 7 deletions.
39 changes: 32 additions & 7 deletions src/main/java/org/opensearch/knn/plugin/KNNPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.opensearch.common.ParseField;
import org.opensearch.index.codec.CodecServiceFactory;
import org.opensearch.index.engine.EngineFactory;
import org.opensearch.index.shard.IndexSettingProvider;
import org.opensearch.indices.SystemIndexDescriptor;
import org.opensearch.knn.index.KNNCircuitBreaker;
import org.opensearch.knn.index.KNNClusterUtil;
Expand Down Expand Up @@ -342,23 +343,47 @@ public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings sett
}

/**
* Plugin can provide additional node settings, that includes new settings or overrides for existing one from core.
* Plugin can provide additional index settings, that includes new settings or overrides for existing one from core.
*
* @return settings that are set by plugin
*/
@Override
public Settings additionalSettings() {
public Collection<IndexSettingProvider> getAdditionalIndexSettingProviders() {
final IndexSettingProvider settingProvider = new IndexSettingProvider() {
@Override
public Settings getAdditionalIndexSettings(
final String indexName,
boolean isDataStreamIndex,
final Settings templateAndRequestSettings
) {
return pluginAdditionalSettings(templateAndRequestSettings);
}
};
return List.of(settingProvider);
}

private void additionalMMapFileExtensionsForHybridFs(final Settings.Builder settingsBuilder) {
// We add engine specific extensions to the core list for HybridFS store type. We read existing values
// and append ours because in core setting will be replaced by override.
// Values are set as cluster defaults and are used at index creation time. Index specific overrides will take priority over values
// that are set here.
final List<String> engineSettings = Arrays.stream(KNNEngine.values())
.flatMap(engine -> engine.mmapFileExtensions().stream())
.collect(Collectors.toList());
.flatMap(engine -> engine.mmapFileExtensions().stream())
.collect(Collectors.toList());
final List<String> combinedSettings = Stream.concat(
IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getDefault(Settings.EMPTY).stream(),
engineSettings.stream()
IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getDefault(Settings.EMPTY).stream(),
engineSettings.stream()
).collect(Collectors.toList());
return Settings.builder().putList(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey(), combinedSettings).build();

settingsBuilder.putList(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey(), combinedSettings);
}

private Settings pluginAdditionalSettings(final Settings templateAndRequestSettings) {
final var settingsBuilder = Settings.builder();
boolean isKnnIndex = Boolean.parseBoolean(templateAndRequestSettings.get(KNNSettings.KNN_INDEX, Boolean.FALSE.toString()));
if (isKnnIndex) {
additionalMMapFileExtensionsForHybridFs(settingsBuilder);
}
return settingsBuilder.build();
}
}
59 changes: 59 additions & 0 deletions src/test/java/org/opensearch/knn/plugin/KNNPluginTests.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.knn.plugin;

import org.opensearch.common.settings.Settings;
import org.opensearch.index.IndexModule;
import org.opensearch.index.shard.IndexSettingProvider;
import org.opensearch.knn.KNNTestCase;
import org.opensearch.knn.index.util.KNNEngine;

import java.util.Collection;
import java.util.List;

public class KNNPluginTests extends KNNTestCase {

private static final String INDEX_NAME = "test_index";

public void testMMapFileExtensionsForHybridFs() {
final KNNPlugin knnPlugin = new KNNPlugin();

final Collection<IndexSettingProvider> additionalIndexSettingProviders = knnPlugin.getAdditionalIndexSettingProviders();

assertEquals(1, additionalIndexSettingProviders.size());

final IndexSettingProvider indexSettingProvider = additionalIndexSettingProviders.iterator().next();
// settings for knn enabled index
final Settings knnIndexSettings = indexSettingProvider.getAdditionalIndexSettings(INDEX_NAME, false, getKnnDefaultIndexSettings());
final List<String> mmapFileExtensionsForHybridFsKnnIndex = knnIndexSettings.getAsList(
IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey()
);

assertNotNull(mmapFileExtensionsForHybridFsKnnIndex);
assertFalse(mmapFileExtensionsForHybridFsKnnIndex.isEmpty());

for (KNNEngine engine : KNNEngine.values()) {
assertTrue(mmapFileExtensionsForHybridFsKnnIndex.containsAll(engine.mmapFileExtensions()));
}

// settings for index without knn
final Settings nonKnnIndexSettings = indexSettingProvider.getAdditionalIndexSettings(INDEX_NAME, false, getNonKnnIndexSettings());
final List<String> mmapFileExtensionsForHybridFsNonKnnIndex = nonKnnIndexSettings.getAsList(
IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey()
);

assertNotNull(mmapFileExtensionsForHybridFsNonKnnIndex);
assertTrue(mmapFileExtensionsForHybridFsNonKnnIndex.isEmpty());
}

private Settings getKnnDefaultIndexSettings() {
return Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0).put("index.knn", true).build();
}

private Settings getNonKnnIndexSettings() {
return Settings.builder().put("number_of_shards", 1).put("number_of_replicas", 0).build();
}
}

0 comments on commit 3a9ba8d

Please sign in to comment.