Skip to content

Commit

Permalink
fix rollover alias supports restored searchable snapshot index
Browse files Browse the repository at this point in the history 
Signed-off-by: kkewwei <[email protected]>
  • Loading branch information
@kkewwei
kkewwei committed Oct 25, 2024
1 parent b2d537a commit c9b2ee0
Show file tree
Hide file tree
Showing 6 changed files with 215 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- [Workload Management] Fixing Create/Update QueryGroup TransportActions to execute from non-cluster manager nodes ([16422](https://github.com/opensearch-project/OpenSearch/pull/16422))
- Fix flaky test in `testApproximateRangeWithSizeOverDefault` by adjusting totalHits assertion logic ([#16434](https://github.com/opensearch-project/OpenSearch/pull/16434#pullrequestreview-2386999409))
- Revert changes to upload remote state manifest using minimum codec version([#16403](https://github.com/opensearch-project/OpenSearch/pull/16403))
- Fix Rollover alias supports restored searchable snapshot index([#16483](https://github.com/opensearch-project/OpenSearch/pull/16483))

### Security

Expand Down
4 changes: 2 additions & 2 deletions ...rc/main/java/org/opensearch/action/admin/indices/alias/TransportIndicesAliasesAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.ack.ClusterStateUpdateResponse;
import org.opensearch.cluster.block.ClusterBlockException;
import org.opensearch.cluster.block.ClusterBlockLevel;
import org.opensearch.cluster.block.ClusterBlocks;
import org.opensearch.cluster.metadata.AliasAction;
import org.opensearch.cluster.metadata.AliasMetadata;
import org.opensearch.cluster.metadata.IndexAbstraction;
Expand Down Expand Up @@ -123,7 +123,7 @@ protected ClusterBlockException checkBlock(IndicesAliasesRequest request, Cluste
for (IndicesAliasesRequest.AliasActions aliasAction : request.aliasActions()) {
Collections.addAll(indices, aliasAction.indices());
}
return state.blocks().indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, indices.toArray(new String[0]));
return ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, state);
}

@Override
Expand Down
13 changes: 7 additions & 6 deletions ...r/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.ClusterStateUpdateTask;
import org.opensearch.cluster.block.ClusterBlockException;
import org.opensearch.cluster.block.ClusterBlockLevel;
import org.opensearch.cluster.block.ClusterBlocks;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.metadata.Metadata;
Expand All @@ -62,8 +62,10 @@
import org.opensearch.transport.TransportService;

import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -127,11 +129,10 @@ protected ClusterBlockException checkBlock(RolloverRequest request, ClusterState
request.indicesOptions().expandWildcardsClosed()
);

return state.blocks()
.indicesBlockedException(
ClusterBlockLevel.METADATA_WRITE,
indexNameExpressionResolver.concreteIndexNames(state, indicesOptions, request)
);
return ClusterBlocks.indicesWithRemoteSnapshotBlockedException(
new HashSet<>(Arrays.asList(indexNameExpressionResolver.concreteIndexNames(state, indicesOptions, request))),

Check warning on line 133 in server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java#L132-L133 

Added lines #L132 - L133 were not covered by tests
state
);
}

@Override
Expand Down
7 changes: 3 additions & 4 deletions .../java/org/opensearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
import org.opensearch.cluster.ack.ClusterStateUpdateResponse;
import org.opensearch.cluster.block.ClusterBlockException;
import org.opensearch.cluster.block.ClusterBlockLevel;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.block.ClusterBlocks;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.metadata.MetadataUpdateSettingsService;
import org.opensearch.cluster.service.ClusterService;
Expand Down Expand Up @@ -118,9 +118,8 @@ protected ClusterBlockException checkBlock(UpdateSettingsRequest request, Cluste
return globalBlock;
}
if (request.settings().size() == 1 && // we have to allow resetting these settings otherwise users can't unblock an index
IndexMetadata.INDEX_BLOCKS_METADATA_SETTING.exists(request.settings())
|| IndexMetadata.INDEX_READ_ONLY_SETTING.exists(request.settings())
|| IndexMetadata.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING.exists(request.settings())) {
ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS.stream()
.anyMatch(booleanSetting -> booleanSetting.exists(request.settings()))) {
return null;
}

Expand Down
26 changes: 25 additions & 1 deletion server/src/main/java/org/opensearch/cluster/block/ClusterBlocks.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,23 @@
package org.opensearch.cluster.block;

import org.opensearch.cluster.AbstractDiffable;
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.Diff;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.metadata.MetadataIndexStateService;
import org.opensearch.common.Nullable;
import org.opensearch.common.annotation.PublicApi;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.util.set.Sets;
import org.opensearch.core.common.io.stream.BufferedChecksumStreamOutput;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.core.common.io.stream.StreamOutput;
import org.opensearch.core.common.io.stream.VerifiableWriteable;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.index.IndexModule;

import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumMap;
import java.util.HashMap;
Expand All @@ -57,6 +61,7 @@
import static java.util.Collections.emptySet;
import static java.util.Collections.unmodifiableSet;
import static java.util.stream.Collectors.toSet;
import static org.opensearch.index.IndexModule.INDEX_STORE_TYPE_SETTING;

/**
* Represents current cluster level blocks to block dirty operations done against the cluster.
Expand All @@ -66,7 +71,11 @@
@PublicApi(since = "1.0.0")
public class ClusterBlocks extends AbstractDiffable<ClusterBlocks> implements VerifiableWriteable {
public static final ClusterBlocks EMPTY_CLUSTER_BLOCK = new ClusterBlocks(emptySet(), Map.of());

public static final Set<Setting<Boolean>> INDEX_DATA_READ_ONLY_BLOCK_SETTINGS = Set.of(
IndexMetadata.INDEX_READ_ONLY_SETTING,
IndexMetadata.INDEX_BLOCKS_METADATA_SETTING,
IndexMetadata.INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING
);
private final Set<ClusterBlock> global;

private final Map<String, Set<ClusterBlock>> indicesBlocks;
Expand Down Expand Up @@ -276,6 +285,21 @@ public ClusterBlockException indicesAllowReleaseResources(String[] indices) {
return new ClusterBlockException(indexLevelBlocks);
}

public static ClusterBlockException indicesWithRemoteSnapshotBlockedException(Collection<String> concreteIndices, ClusterState state) {
for (String index : concreteIndices) {
if (state.blocks().indexBlocked(ClusterBlockLevel.METADATA_WRITE, index)) {
IndexMetadata indexMeta = state.metadata().index(index);
if (indexMeta != null
&& (IndexModule.Type.REMOTE_SNAPSHOT.match(indexMeta.getSettings().get(INDEX_STORE_TYPE_SETTING.getKey())) == false
|| ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS.stream()
.anyMatch(booleanSetting -> booleanSetting.exists(indexMeta.getSettings())))) {
return state.blocks().indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, concreteIndices.toArray(new String[0]));
}
}
}
return null;
}

@Override
public String toString() {
if (global.isEmpty() && indices().isEmpty()) {
Expand Down
177 changes: 177 additions & 0 deletions server/src/test/java/org/opensearch/cluster/block/ClusterBlocksTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,40 @@

package org.opensearch.cluster.block;

import com.carrotsearch.randomizedtesting.RandomizedTest;

import org.opensearch.Version;
import org.opensearch.cluster.ClusterName;
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.metadata.AliasMetadata;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.metadata.Metadata;
import org.opensearch.common.io.stream.BytesStreamOutput;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Settings;
import org.opensearch.core.common.io.stream.BufferedChecksumStreamOutput;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.index.IndexModule;
import org.opensearch.index.IndexSettings;
import org.opensearch.test.OpenSearchTestCase;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import static org.opensearch.cluster.block.ClusterBlockTests.randomClusterBlock;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_METADATA_BLOCK;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_BLOCK;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_ALLOW_DELETE_BLOCK;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_READ_ONLY_BLOCK;
import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_WRITE_BLOCK;
import static org.opensearch.cluster.metadata.IndexMetadata.REMOTE_READ_ONLY_ALLOW_DELETE;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_CREATION_DATE;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;

public class ClusterBlocksTests extends OpenSearchTestCase {

Expand Down Expand Up @@ -52,4 +80,153 @@ public void testWriteVerifiableTo() throws Exception {
clusterBlocks2.writeVerifiableTo(checksumOut2);
assertEquals(checksumOut.getChecksum(), checksumOut2.getChecksum());
}

public void testGlobalBlock() {
String index = "test-000001";
ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));
Set<String> indices = new HashSet<>();
indices.add(index);

// no global blocks
{
stateBuilder.blocks(ClusterBlocks.EMPTY_CLUSTER_BLOCK);
ClusterState clusterState = stateBuilder.build();
clusterState.blocks();
assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
}

// has global block
{
for (ClusterBlock block : Arrays.asList(
INDEX_READ_ONLY_BLOCK,
INDEX_READ_BLOCK,
INDEX_WRITE_BLOCK,
INDEX_METADATA_BLOCK,
INDEX_READ_ONLY_ALLOW_DELETE_BLOCK,
REMOTE_READ_ONLY_ALLOW_DELETE
)) {
stateBuilder.blocks(ClusterBlocks.builder().addGlobalBlock(block).build());
ClusterState clusterState = stateBuilder.build();
clusterState.blocks();
assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
}
}
}

public void testIndexWithBlock() {
String index = "test-000001";
Set<String> indices = new HashSet<>();
indices.add(index);
ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));
stateBuilder.blocks(ClusterBlocks.builder().addIndexBlock(index, IndexMetadata.INDEX_METADATA_BLOCK));
stateBuilder.metadata(Metadata.builder().put(createIndexMetadata(index, false, null, null), false));
ClusterState clusterState = stateBuilder.build();
clusterState.blocks();
assertNotNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, stateBuilder.build()));
}

public void testRemoteIndexBlock() {
String remoteIndex = "remote_index";
Set<String> indices = new HashSet<>();
indices.add(remoteIndex);
ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));

{
IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, null, null);
stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false));
stateBuilder.blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata));

ClusterState clusterState = stateBuilder.build();
assertTrue(clusterState.blocks().hasIndexBlock(remoteIndex, IndexMetadata.REMOTE_READ_ONLY_ALLOW_DELETE));
clusterState.blocks();
assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
}

// searchable snapshot index with block
{
Setting<Boolean> setting = RandomizedTest.randomFrom(new ArrayList<>(ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS));
IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, null, setting);
stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false));
stateBuilder.blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata));
ClusterState clusterState = stateBuilder.build();
clusterState.blocks();
assertNotNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
}
}

public void testRemoteIndexWithoutBlock() {
String remoteIndex = "remote_index";
ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));

String alias = "alias1";
IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, alias, null);
String index = "test-000001";
IndexMetadata indexMetadata = createIndexMetadata(index, false, alias, null);
stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false).put(indexMetadata, false));

Set<String> indices = new HashSet<>();
indices.add(remoteIndex);
ClusterState clusterState = stateBuilder.build();
clusterState.blocks();
assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(indices, clusterState));
}

public void testRemoteIndexWithIndexBlock() {
String index = "test-000001";
String remoteIndex = "remote_index";
String alias = "alias1";
{
ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));
IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, alias, null);
IndexMetadata indexMetadata = createIndexMetadata(index, false, alias, null);
stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false).put(indexMetadata, false))
.blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata));
ClusterState clusterState = stateBuilder.build();
clusterState.blocks();
assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(index), clusterState));
clusterState.blocks();
assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(remoteIndex), clusterState));
}

{
ClusterState.Builder stateBuilder = ClusterState.builder(new ClusterName("cluster"));
Setting<Boolean> setting = RandomizedTest.randomFrom(new ArrayList<>(ClusterBlocks.INDEX_DATA_READ_ONLY_BLOCK_SETTINGS));
IndexMetadata remoteSnapshotIndexMetadata = createIndexMetadata(remoteIndex, true, alias, setting);
IndexMetadata indexMetadata = createIndexMetadata(index, false, alias, null);
stateBuilder.metadata(Metadata.builder().put(remoteSnapshotIndexMetadata, false).put(indexMetadata, false))
.blocks(ClusterBlocks.builder().addBlocks(remoteSnapshotIndexMetadata));
ClusterState clusterState = stateBuilder.build();
assertNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(index), clusterState));
assertNotNull(ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singleton(remoteIndex), clusterState));
}
}

private IndexMetadata createIndexMetadata(String index, boolean isRemoteIndex, String alias, Setting<Boolean> blockSetting) {
IndexMetadata.Builder builder = IndexMetadata.builder(index).settings(createIndexSettingBuilder(isRemoteIndex, blockSetting));
if (alias != null) {
AliasMetadata.Builder aliasBuilder = AliasMetadata.builder(alias);
return builder.putAlias(aliasBuilder.build()).build();
}
return builder.build();
}

private Settings.Builder createIndexSettingBuilder(boolean isRemoteIndex, Setting<Boolean> blockSetting) {
Settings.Builder builder = Settings.builder()
.put(IndexMetadata.SETTING_INDEX_UUID, "abc")
.put(SETTING_VERSION_CREATED, Version.CURRENT)
.put(SETTING_CREATION_DATE, System.currentTimeMillis())
.put(SETTING_NUMBER_OF_SHARDS, 1)
.put(SETTING_NUMBER_OF_REPLICAS, 1);

if (isRemoteIndex) {
builder.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey())
.put(IndexSettings.SEARCHABLE_SNAPSHOT_REPOSITORY.getKey(), "repo")
.put(IndexSettings.SEARCHABLE_SNAPSHOT_ID_NAME.getKey(), "snapshot");
}
if (blockSetting != null) {
builder.put(blockSetting.getKey(), true);
}

return builder;
}
}

0 comments on commit c9b2ee0

Please sign in to comment.