Skip to content

Commit

Permalink
Backport PR's opensearch-project#8186 and opensearch-project#8235
Browse files Browse the repository at this point in the history 
Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
  • Loading branch information
@Rishikesh1159
Rishikesh1159 committed Jun 23, 2023
1 parent 4b3b30d commit d8a054a
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 25 deletions.
2 changes: 1 addition & 1 deletion ...nalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexClusterDefaultDocRep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void testDefaultRemoteStoreNoUserOverride() throws Exception {
);
assertThat(
exc.getMessage(),
containsString("Cannot enable [index.remote_store.enabled] when [cluster.indices.replication.strategy] is DOCUMENT")
containsString("Cannot enable [index.remote_store.enabled] when [index.replication.type] is DOCUMENT")
);
}

Expand Down
52 changes: 52 additions & 0 deletions server/src/internalClusterTest/java/org/opensearch/remotestore/CreateRemoteIndexIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,22 @@
import org.junit.Before;
import org.opensearch.action.admin.indices.get.GetIndexRequest;
import org.opensearch.action.admin.indices.get.GetIndexResponse;
import org.opensearch.action.admin.indices.settings.get.GetSettingsRequest;
import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.indices.SystemIndexDescriptor;
import org.opensearch.indices.replication.common.ReplicationType;
import org.opensearch.plugins.Plugin;
import org.opensearch.plugins.SystemIndexPlugin;
import org.opensearch.test.FeatureFlagSetter;
import org.opensearch.test.OpenSearchIntegTestCase;

import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;

import static org.hamcrest.Matchers.containsString;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_ENABLED;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_STORE_REPOSITORY;
Expand Down Expand Up @@ -57,6 +66,20 @@ protected Settings nodeSettings(int nodeOriginal) {
return builder.build();
}

public static class TestPlugin extends Plugin implements SystemIndexPlugin {
@Override
public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings settings) {
return Collections.singletonList(
new SystemIndexDescriptor(SYSTEM_INDEX_NAME, "System index for [" + getTestClass().getName() + ']')
);
}
}

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Arrays.asList(CreateRemoteIndexIT.TestPlugin.class);
}

@Override
protected Settings featureFlagSettings() {
return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.REMOTE_STORE, "true").build();
Expand Down Expand Up @@ -105,6 +128,35 @@ public void testDefaultRemoteStoreNoUserOverride() throws Exception {
);
}

private static final String SYSTEM_INDEX_NAME = ".test-system-index";

public void testSystemIndexWithRemoteStoreClusterSetting() throws Exception {
createIndex(SYSTEM_INDEX_NAME);
ensureGreen(SYSTEM_INDEX_NAME);
final GetSettingsResponse response = client().admin()
.indices()
.getSettings(new GetSettingsRequest().indices(SYSTEM_INDEX_NAME).includeDefaults(true))
.actionGet();
// Verify that Document replication strategy is used
assertEquals(response.getSetting(SYSTEM_INDEX_NAME, SETTING_REPLICATION_TYPE), ReplicationType.DOCUMENT.toString());
assertEquals(response.getSetting(SYSTEM_INDEX_NAME, SETTING_REMOTE_STORE_ENABLED), "false");
}

public void testSystemIndexWithRemoteStoreIndexSettings() throws Exception {
prepareCreate(
SYSTEM_INDEX_NAME,
Settings.builder().put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT).put(SETTING_REMOTE_STORE_ENABLED, true)
).get();
ensureGreen(SYSTEM_INDEX_NAME);
final GetSettingsResponse response = client().admin()
.indices()
.getSettings(new GetSettingsRequest().indices(SYSTEM_INDEX_NAME).includeDefaults(true))
.actionGet();
// Verify that Document replication strategy is used
assertEquals(response.getSetting(SYSTEM_INDEX_NAME, SETTING_REPLICATION_TYPE), ReplicationType.DOCUMENT.toString());
assertEquals(response.getSetting(SYSTEM_INDEX_NAME, SETTING_REMOTE_STORE_ENABLED), "false");
}

public void testRemoteStoreDisabledByUser() throws Exception {
Settings settings = Settings.builder()
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
Expand Down
8 changes: 2 additions & 6 deletions server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@
import static org.opensearch.cluster.node.DiscoveryNodeFilters.OpType.OR;
import static org.opensearch.common.settings.Settings.readSettingsFromStream;
import static org.opensearch.common.settings.Settings.writeSettingsToStream;
import static org.opensearch.indices.IndicesService.CLUSTER_REPLICATION_TYPE_SETTING;

/**
* Index metadata information
Expand Down Expand Up @@ -318,11 +317,8 @@ public void validate(final Boolean value) {}

@Override
public void validate(final Boolean value, final Map<Setting<?>, Object> settings) {
final Object clusterSettingReplicationType = settings.get(CLUSTER_REPLICATION_TYPE_SETTING);
final Object replicationType = settings.get(INDEX_REPLICATION_TYPE_SETTING);
if ((replicationType).equals(ReplicationType.SEGMENT) == false
&& (clusterSettingReplicationType).equals(ReplicationType.SEGMENT) == false
&& value == true) {
if (ReplicationType.SEGMENT.equals(replicationType) == false && value) {
throw new IllegalArgumentException(
"To enable "
+ INDEX_REMOTE_STORE_ENABLED_SETTING.getKey()
Expand All @@ -336,7 +332,7 @@ public void validate(final Boolean value, final Map<Setting<?>, Object> settings

@Override
public Iterator<Setting<?>> settings() {
final List<Setting<?>> settings = List.of(INDEX_REPLICATION_TYPE_SETTING, CLUSTER_REPLICATION_TYPE_SETTING);
final List<Setting<?>> settings = List.of(INDEX_REPLICATION_TYPE_SETTING);
return settings.iterator();
}
},
Expand Down
42 changes: 25 additions & 17 deletions server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import org.opensearch.common.settings.IndexScopedSettings;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.common.xcontent.XContentHelper;
import org.opensearch.env.Environment;
Expand Down Expand Up @@ -137,6 +138,7 @@
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REMOTE_TRANSLOG_STORE_REPOSITORY;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
import static org.opensearch.cluster.metadata.Metadata.DEFAULT_REPLICA_COUNT_SETTING;
import static org.opensearch.common.util.FeatureFlags.REMOTE_STORE;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_REPOSITORY_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING;
import static org.opensearch.indices.IndicesService.CLUSTER_REMOTE_STORE_ENABLED_SETTING;
Expand Down Expand Up @@ -902,8 +904,18 @@ static Settings aggregateIndexSettings(
indexSettingsBuilder.put(IndexMetadata.SETTING_INDEX_PROVIDED_NAME, request.getProvidedName());
indexSettingsBuilder.put(SETTING_INDEX_UUID, UUIDs.randomBase64UUID());

updateReplicationStrategy(indexSettingsBuilder, request.settings(), settings, isSystemIndex);
updateRemoteStoreSettings(indexSettingsBuilder, request.settings(), settings);
if (isSystemIndex || IndexMetadata.INDEX_HIDDEN_SETTING.get(request.settings())) {
logger.warn(
"Setting replication.type: DOCUMENT will be used for Index until Segment Replication supports System and Hidden indices"
);
indexSettingsBuilder.put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT);
if (FeatureFlags.isEnabled(REMOTE_STORE)) {
indexSettingsBuilder.put(SETTING_REMOTE_STORE_ENABLED, false);
}
} else {
updateReplicationStrategy(indexSettingsBuilder, request.settings(), settings);
updateRemoteStoreSettings(indexSettingsBuilder, request.settings(), settings);
}

if (sourceMetadata != null) {
assert request.resizeType() != null;
Expand Down Expand Up @@ -943,19 +955,16 @@ static Settings aggregateIndexSettings(
* @param requestSettings settings passed in during index create request
* @param clusterSettings cluster level settings
*/
private static void updateReplicationStrategy(
Settings.Builder settingsBuilder,
Settings requestSettings,
Settings clusterSettings,
boolean isSystemIndex
) {
if (isSystemIndex || IndexMetadata.INDEX_HIDDEN_SETTING.get(requestSettings)) {
settingsBuilder.put(SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT);
return;
}
private static void updateReplicationStrategy(Settings.Builder settingsBuilder, Settings requestSettings, Settings clusterSettings) {
if (CLUSTER_REPLICATION_TYPE_SETTING.exists(clusterSettings) && INDEX_REPLICATION_TYPE_SETTING.exists(requestSettings) == false) {
settingsBuilder.put(SETTING_REPLICATION_TYPE, CLUSTER_REPLICATION_TYPE_SETTING.get(clusterSettings));
return;
}
if (INDEX_REPLICATION_TYPE_SETTING.exists(requestSettings) == true) {
settingsBuilder.put(SETTING_REPLICATION_TYPE, INDEX_REPLICATION_TYPE_SETTING.get(requestSettings));
return;
}
settingsBuilder.put(SETTING_REPLICATION_TYPE, CLUSTER_REPLICATION_TYPE_SETTING.getDefault(clusterSettings));
}

/**
Expand All @@ -971,20 +980,19 @@ private static void updateRemoteStoreSettings(Settings.Builder settingsBuilder,
return;
}

// Verify REPLICATION_TYPE cluster level setting is not conflicting with Remote Store
if (INDEX_REPLICATION_TYPE_SETTING.exists(requestSettings) == false
&& CLUSTER_REPLICATION_TYPE_SETTING.get(clusterSettings).equals(ReplicationType.DOCUMENT)) {
// Verify index has replication type as SEGMENT
if (ReplicationType.DOCUMENT.equals(ReplicationType.parseString(settingsBuilder.get(SETTING_REPLICATION_TYPE)))) {
throw new IllegalArgumentException(
"Cannot enable ["
+ SETTING_REMOTE_STORE_ENABLED
+ "] when ["
+ CLUSTER_REPLICATION_TYPE_SETTING.getKey()
+ SETTING_REPLICATION_TYPE
+ "] is "
+ ReplicationType.DOCUMENT
);
}

settingsBuilder.put(SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT).put(SETTING_REMOTE_STORE_ENABLED, true);
settingsBuilder.put(SETTING_REMOTE_STORE_ENABLED, true);

String remoteStoreRepo;
if (Objects.equals(requestSettings.get(INDEX_REMOTE_STORE_ENABLED_SETTING.getKey()), "true")) {
Expand Down
31 changes: 30 additions & 1 deletion server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,7 @@ public void testRemoteStoreNoUserOverrideConflictingReplicationTypeIndexSettings
);
assertThat(
exc.getMessage(),
containsString("Cannot enable [index.remote_store.enabled] when [cluster.indices.replication.strategy] is DOCUMENT")
containsString("Cannot enable [index.remote_store.enabled] when [index.replication.type] is DOCUMENT")
);
}

Expand Down Expand Up @@ -1702,6 +1702,35 @@ public void testSystemIndexUsesDocumentReplication() {
assertEquals(ReplicationType.DOCUMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE));
}

public void testRemoteStoreDisabledForSystemIndices() {
Settings settings = Settings.builder()
.put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT)
.put(CLUSTER_REMOTE_STORE_ENABLED_SETTING.getKey(), true)
.put(CLUSTER_REMOTE_STORE_REPOSITORY_SETTING.getKey(), "my-segment-repo-1")
.put(CLUSTER_REMOTE_TRANSLOG_STORE_ENABLED_SETTING.getKey(), true)
.put(CLUSTER_REMOTE_TRANSLOG_REPOSITORY_SETTING.getKey(), "my-translog-repo-1")
.build();
FeatureFlagSetter.set(FeatureFlags.REMOTE_STORE);
request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test");
final Settings.Builder requestSettings = Settings.builder();
request.settings(requestSettings.build());
// set isSystemIndex parameter as true
Settings indexSettings = aggregateIndexSettings(
ClusterState.EMPTY_STATE,
request,
Settings.EMPTY,
null,
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
true
);
// Verify that remote store is disabled.
assertEquals(indexSettings.get(SETTING_REMOTE_STORE_ENABLED), "false");
assertEquals(ReplicationType.DOCUMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE));
}

private IndexTemplateMetadata addMatchingTemplate(Consumer<IndexTemplateMetadata.Builder> configurator) {
IndexTemplateMetadata.Builder builder = templateMetadataBuilder("template1", "te*");
configurator.accept(builder);
Expand Down

0 comments on commit d8a054a

Please sign in to comment.