Skip to content

Commit

Permalink
[Segment Replication] Remove Restriction of strictly using only docre…
Browse files Browse the repository at this point in the history 
…p for system indices and hidden indices (opensearch-project#8200)

* remove logic of strictly using only docrep for system indices and hidden indices.

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

* Remove segrep-system indices related tests.

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

* Fix failing tests.

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>

---------

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
Signed-off-by: Rishikesh Pasham <62345295+Rishikesh1159@users.noreply.github.com>
  • Loading branch information
@Rishikesh1159 @baba-devv
Rishikesh1159 authored and baba-devv committed Jul 29, 2023
1 parent dfcc342 commit 01e20e8
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 251 deletions.
66 changes: 0 additions & 66 deletions ...usterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,8 @@
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 @@ -60,40 +52,6 @@ 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 @@ -165,28 +123,4 @@ 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: 0 additions & 52 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,24 +12,15 @@
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 @@ -68,20 +59,6 @@ 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 @@ -130,35 +107,6 @@ 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: 6 additions & 22 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 @@ -72,7 +72,6 @@
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,7 +136,6 @@
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 @@ -585,8 +583,7 @@ private ClusterState applyCreateIndexRequestWithV1Templates(
settings,
indexScopedSettings,
shardLimitValidator,
indexSettingProviders,
systemIndices.validateSystemIndex(request.index())
indexSettingProviders
);
int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, null);
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards);
Expand Down Expand Up @@ -650,8 +647,7 @@ private ClusterState applyCreateIndexRequestWithV2Template(
settings,
indexScopedSettings,
shardLimitValidator,
indexSettingProviders,
systemIndices.validateSystemIndex(request.index())
indexSettingProviders
);
int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, null);
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards);
Expand Down Expand Up @@ -731,8 +727,7 @@ private ClusterState applyCreateIndexRequestWithExistingMetadata(
settings,
indexScopedSettings,
shardLimitValidator,
indexSettingProviders,
sourceMetadata.isSystem()
indexSettingProviders
);
final int routingNumShards = getIndexNumberOfRoutingShards(aggregatedIndexSettings, sourceMetadata);
IndexMetadata tmpImd = buildAndValidateTemporaryIndexMetadata(currentState, aggregatedIndexSettings, request, routingNumShards);
Expand Down Expand Up @@ -815,8 +810,7 @@ static Settings aggregateIndexSettings(
Settings settings,
IndexScopedSettings indexScopedSettings,
ShardLimitValidator shardLimitValidator,
Set<IndexSettingProvider> indexSettingProviders,
boolean isSystemIndex
Set<IndexSettingProvider> indexSettingProviders
) {
// 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 @@ -900,18 +894,8 @@ static Settings aggregateIndexSettings(
indexSettingsBuilder.put(IndexMetadata.SETTING_INDEX_PROVIDED_NAME, request.getProvidedName());
indexSettingsBuilder.put(SETTING_INDEX_UUID, UUIDs.randomBase64UUID());

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);
}
updateReplicationStrategy(indexSettingsBuilder, request.settings(), settings);
updateRemoteStoreSettings(indexSettingsBuilder, request.settings(), settings);

if (sourceMetadata != null) {
assert request.resizeType() != null;
Expand Down
Loading

0 comments on commit 01e20e8

Please sign in to comment.