Skip to content

Commit

Permalink
Revert "Remove Restriction of strictly using only docrep for system i…
Browse files Browse the repository at this point in the history 
…ndices and hidden indices. (opensearch-project#8502)" (opensearch-project#8657)

This reverts commit ef4881f.

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
  • Loading branch information
@Rishikesh1159
Rishikesh1159 committed Jul 12, 2023
1 parent 4890548 commit b717320
Show file tree
Hide file tree
Showing 4 changed files with 250 additions and 25 deletions.
66 changes: 66 additions & 0 deletions ...usterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,16 @@
import org.opensearch.index.Index;
import org.opensearch.index.IndexModule;
import org.opensearch.indices.IndicesService;
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.OpenSearchIntegTestCase;
import org.opensearch.test.transport.MockTransportService;

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

import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
import static org.opensearch.indices.IndicesService.CLUSTER_SETTING_REPLICATION_TYPE;
Expand Down Expand Up @@ -52,6 +60,40 @@ protected Settings nodeSettings(int nodeOrdinal) {
.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(SegmentReplicationClusterSettingIT.TestPlugin.class, MockTransportService.TestPlugin.class);
}

public void testSystemIndexWithSegmentReplicationClusterSetting() throws Exception {

// Starting two nodes with primary and replica shards respectively.
final String primaryNode = internalCluster().startNode();
createIndex(SYSTEM_INDEX_NAME);
ensureYellowAndNoInitializingShards(SYSTEM_INDEX_NAME);
final String replicaNode = internalCluster().startNode();
ensureGreen(SYSTEM_INDEX_NAME);
final GetSettingsResponse response = client().admin()
.indices()
.getSettings(new GetSettingsRequest().indices(SYSTEM_INDEX_NAME).includeDefaults(true))
.actionGet();
assertEquals(response.getSetting(SYSTEM_INDEX_NAME, SETTING_REPLICATION_TYPE), ReplicationType.DOCUMENT.toString());

// Verify index setting isSegRepEnabled is false.
Index index = resolveIndex(SYSTEM_INDEX_NAME);
IndicesService indicesService = internalCluster().getInstance(IndicesService.class, primaryNode);
assertEquals(indicesService.indexService(index).getIndexSettings().isSegRepEnabled(), false);
}

public void testIndexReplicationSettingOverridesSegRepClusterSetting() throws Exception {
Settings settings = Settings.builder().put(CLUSTER_SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT).build();
final String ANOTHER_INDEX = "test-index";
Expand Down Expand Up @@ -123,4 +165,28 @@ public void testIndexReplicationSettingOverridesDocRepClusterSetting() throws Ex
assertEquals(indicesService.indexService(anotherIndex).getIndexSettings().isSegRepEnabled(), false);
}

public void testHiddenIndicesWithReplicationStrategyClusterSetting() throws Exception {
final String primaryNode = internalCluster().startNode();
final String replicaNode = internalCluster().startNode();
prepareCreate(
INDEX_NAME,
Settings.builder()
// we want to set index as hidden
.put("index.hidden", true)
).get();
ensureGreen(INDEX_NAME);

// Verify that document replication strategy is used for hidden indices.
final GetSettingsResponse response = client().admin()
.indices()
.getSettings(new GetSettingsRequest().indices(INDEX_NAME).includeDefaults(true))
.actionGet();
assertEquals(response.getSetting(INDEX_NAME, SETTING_REPLICATION_TYPE), ReplicationType.DOCUMENT.toString());

// Verify index setting isSegRepEnabled.
Index index = resolveIndex(INDEX_NAME);
IndicesService indicesService = internalCluster().getInstance(IndicesService.class, primaryNode);
assertEquals(indicesService.indexService(index).getIndexSettings().isSegRepEnabled(), false);
}

}
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,15 +12,24 @@
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.unit.TimeValue;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.indices.SystemIndexDescriptor;
import org.opensearch.index.IndexSettings;
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 @@ -59,6 +68,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 @@ -107,6 +130,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
28 changes: 22 additions & 6 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 @@ -73,6 +73,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.common.xcontent.XContentHelper;
import org.opensearch.core.common.Strings;
import org.opensearch.core.xcontent.NamedXContentRegistry;
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 @@ -584,7 +586,8 @@ private ClusterState applyCreateIndexRequestWithV1Templates(
settings,
indexScopedSettings,
shardLimitValidator,
indexSettingProviders
indexSettingProviders,
systemIndices.validateSystemIndex(request.index())
);
int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, null);
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards);
Expand Down Expand Up @@ -648,7 +651,8 @@ private ClusterState applyCreateIndexRequestWithV2Template(
settings,
indexScopedSettings,
shardLimitValidator,
indexSettingProviders
indexSettingProviders,
systemIndices.validateSystemIndex(request.index())
);
int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, null);
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards);
Expand Down Expand Up @@ -728,7 +732,8 @@ private ClusterState applyCreateIndexRequestWithExistingMetadata(
settings,
indexScopedSettings,
shardLimitValidator,
indexSettingProviders
indexSettingProviders,
sourceMetadata.isSystem()
);
final int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, sourceMetadata);
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards);
Expand Down Expand Up @@ -811,7 +816,8 @@ static Settings aggregateIndexSettings(
Settings settings,
IndexScopedSettings indexScopedSettings,
ShardLimitValidator shardLimitValidator,
Set<IndexSettingProvider> indexSettingProviders
Set<IndexSettingProvider> indexSettingProviders,
boolean isSystemIndex
) {
// Create builders for the template and request settings. We transform these into builders
// because we may want settings to be "removed" from these prior to being set on the new
Expand Down Expand Up @@ -901,8 +907,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);
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
Loading

0 comments on commit b717320

Please sign in to comment.