Skip to content

Commit

Permalink
[Segment Replication + Remote Store] Removing exception throw when re…
Browse files Browse the repository at this point in the history 
…mote store is used with system indices (#8235)

* Removing throwing of exception when remote store is used with system indices.

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

* Add check of feature flag enabled.

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

* Add unit test to improve code coverage.

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

* Add warning log.

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

---------

Signed-off-by: Rishikesh1159 <rishireddy1159@gmail.com>
  • Loading branch information
@Rishikesh1159
Rishikesh1159 authored Jun 23, 2023
1 parent f21d73e commit 3b04ef4
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 27 deletions.
41 changes: 26 additions & 15 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,6 +12,8 @@
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;
Expand Down Expand Up @@ -131,21 +133,30 @@ public void testDefaultRemoteStoreNoUserOverride() throws Exception {
private static final String SYSTEM_INDEX_NAME = ".test-system-index";

public void testSystemIndexWithRemoteStoreClusterSetting() throws Exception {
IllegalArgumentException illegalArgumentException = expectThrows(
IllegalArgumentException.class,
() -> createIndex(SYSTEM_INDEX_NAME)
);
assertThat(
illegalArgumentException.getMessage(),
containsString(
"Cannot enable ["
+ SETTING_REMOTE_STORE_ENABLED
+ "] when ["
+ SETTING_REPLICATION_TYPE
+ "] is "
+ ReplicationType.DOCUMENT
)
);
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 {
Expand Down
27 changes: 15 additions & 12 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,6 +72,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 @@ -136,6 +137,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 @@ -898,8 +900,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 @@ -939,16 +951,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));
return;
Expand Down
30 changes: 30 additions & 0 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 @@ -1715,6 +1715,36 @@ 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 3b04ef4

Please sign in to comment.