Skip to content

Commit

Permalink
remove logic of strictly using only docrep for system indices and hid…
Browse files Browse the repository at this point in the history 
…den indices.

Signed-off-by: Rishikesh1159 <[email protected]>
  • Loading branch information
@Rishikesh1159
Rishikesh1159 committed Jun 21, 2023
1 parent 68c1c86 commit aa9b732
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 166 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);
}

}
25 changes: 6 additions & 19 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 @@ -583,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 @@ -648,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 @@ -729,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 @@ -813,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 @@ -898,7 +894,7 @@ 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);
updateReplicationStrategy(indexSettingsBuilder, request.settings(), settings);
updateRemoteStoreSettings(indexSettingsBuilder, request.settings(), settings);

if (sourceMetadata != null) {
Expand Down Expand Up @@ -939,16 +935,7 @@ 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));
}
Expand Down
100 changes: 19 additions & 81 deletions server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -817,8 +817,7 @@ public void testAggregateSettingsAppliesSettingsFromTemplatesAndRequest() {
Settings.EMPTY,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);

assertThat(aggregatedIndexSettings.get("template_setting"), equalTo("value1"));
Expand Down Expand Up @@ -880,8 +879,7 @@ public void testRequestDataHavePriorityOverTemplateData() throws Exception {
Settings.EMPTY,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);

assertThat(resolvedAliases.get(0).getSearchRouting(), equalTo("fromRequest"));
Expand All @@ -903,8 +901,7 @@ public void testDefaultSettings() {
Settings.EMPTY,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);

assertThat(aggregatedIndexSettings.get(SETTING_NUMBER_OF_SHARDS), equalTo("1"));
Expand All @@ -919,8 +916,7 @@ public void testSettingsFromClusterState() {
Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 15).build(),
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);

assertThat(aggregatedIndexSettings.get(SETTING_NUMBER_OF_SHARDS), equalTo("15"));
Expand Down Expand Up @@ -957,8 +953,7 @@ public void testTemplateOrder() throws Exception {
Settings.EMPTY,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);
List<AliasMetadata> resolvedAliases = resolveAndValidateAliases(
request.index(),
Expand Down Expand Up @@ -997,8 +992,7 @@ public void testAggregateIndexSettingsIgnoresTemplatesOnCreateFromSourceIndex()
Settings.EMPTY,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);

assertThat(aggregatedIndexSettings.get("templateSetting"), is(nullValue()));
Expand Down Expand Up @@ -1220,8 +1214,7 @@ public void testRemoteStoreNoUserOverrideConflictingReplicationTypeIndexSettings
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
)
);
assertThat(
Expand Down Expand Up @@ -1252,8 +1245,7 @@ public void testRemoteStoreNoUserOverrideExceptReplicationTypeSegmentIndexSettin
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);
verifyRemoteStoreIndexSettings(
indexSettings,
Expand Down Expand Up @@ -1285,8 +1277,7 @@ public void testRemoteStoreNoUserOverrideIndexSettings() {
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);
verifyRemoteStoreIndexSettings(
indexSettings,
Expand Down Expand Up @@ -1320,8 +1311,7 @@ public void testRemoteStoreDisabledByUserIndexSettings() {
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);
verifyRemoteStoreIndexSettings(indexSettings, "false", null, null, null, ReplicationType.SEGMENT.toString(), null);
}
Expand All @@ -1347,8 +1337,7 @@ public void testRemoteStoreTranslogDisabledByUserIndexSettings() {
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);
verifyRemoteStoreIndexSettings(indexSettings, "true", "my-segment-repo-1", "false", null, ReplicationType.SEGMENT.toString(), null);
}
Expand Down Expand Up @@ -1377,8 +1366,7 @@ public void testRemoteStoreOverrideSegmentRepoIndexSettings() {
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);
verifyRemoteStoreIndexSettings(
indexSettings,
Expand Down Expand Up @@ -1412,8 +1400,7 @@ public void testRemoteStoreOverrideTranslogRepoIndexSettings() {
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);
verifyRemoteStoreIndexSettings(
indexSettings,
Expand Down Expand Up @@ -1447,8 +1434,7 @@ public void testRemoteStoreOverrideReplicationTypeIndexSettings() {
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);
verifyRemoteStoreIndexSettings(indexSettings, null, null, null, null, ReplicationType.DOCUMENT.toString(), null);
}
Expand Down Expand Up @@ -1522,8 +1508,7 @@ public void testSoftDeletesDisabledIsRejected() {
Settings.EMPTY,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);
});
assertThat(
Expand Down Expand Up @@ -1552,8 +1537,7 @@ public void testValidateTranslogRetentionSettings() {
Settings.EMPTY,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);
assertWarnings(
"Translog retention settings [index.translog.retention.age] "
Expand Down Expand Up @@ -1600,8 +1584,7 @@ public void testDeprecatedSimpleFSStoreSettings() {
Settings.EMPTY,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);
assertWarnings(
"[simplefs] is deprecated and will be removed in 2.0. Use [niofs], which offers equal "
Expand All @@ -1620,8 +1603,7 @@ public void testClusterReplicationSetting() {
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);
assertEquals(ReplicationType.SEGMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE));
}
Expand All @@ -1641,56 +1623,12 @@ public void testIndexSettingOverridesClusterReplicationSetting() {
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
Collections.emptySet()
);
// Verify if index setting overrides cluster replication setting
assertEquals(ReplicationType.DOCUMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE));
}

public void testHiddenIndexUsesDocumentReplication() {
Settings settings = Settings.builder().put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT).build();
request = new CreateIndexClusterStateUpdateRequest("create index", "test", "test");
final Settings.Builder requestSettings = Settings.builder();
// Set index setting replication type as DOCUMENT
requestSettings.put("index.hidden", true);
request.settings(requestSettings.build());
Settings indexSettings = aggregateIndexSettings(
ClusterState.EMPTY_STATE,
request,
Settings.EMPTY,
null,
settings,
IndexScopedSettings.DEFAULT_SCOPED_SETTINGS,
randomShardLimitService(),
Collections.emptySet(),
false
);
// Verify replication type is Document Replication
assertEquals(ReplicationType.DOCUMENT.toString(), indexSettings.get(SETTING_REPLICATION_TYPE));
}

public void testSystemIndexUsesDocumentReplication() {
Settings settings = Settings.builder().put(CLUSTER_REPLICATION_TYPE_SETTING.getKey(), ReplicationType.SEGMENT).build();
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 replication type is Document Replication
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 aa9b732

Please sign in to comment.