Skip to content

Rollover: scope checkBlock to write index only, skipping non-write alias members #21838

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andrross merged 4 commits into from
Jul 1, 2026
+278 −13
Merged

Rollover: scope checkBlock to write index only, skipping non-write alias members #21838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bca10e0
Scope checkBlock to write index only, skipping non-write alias members
mishail May 26, 2026
78dfd3e
Inline single-line method call in `checkBlock` for rollover action
mishail May 26, 2026
27f18eb
Merge branch 'main' into ms/21629-rollover-fix-oss
mishail May 27, 2026
83fed8f
Merge branch 'main' into ms/21629-rollover-fix-oss
mishail May 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 10 additions & 13 deletions ...r/src/main/java/org/opensearch/action/admin/indices/rollover/TransportRolloverAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.opensearch.cluster.ClusterStateUpdateTask;
import org.opensearch.cluster.block.ClusterBlockException;
import org.opensearch.cluster.block.ClusterBlocks;
import org.opensearch.cluster.metadata.IndexAbstraction;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.metadata.Metadata;
Expand All @@ -64,10 +65,8 @@
import org.opensearch.transport.client.Client;

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 @@ -128,17 +127,15 @@ protected RolloverResponse read(StreamInput in) throws IOException {

@Override
protected ClusterBlockException checkBlock(RolloverRequest request, ClusterState state) {
IndicesOptions indicesOptions = IndicesOptions.fromOptions(
true,
true,
request.indicesOptions().expandWildcardsOpen(),
request.indicesOptions().expandWildcardsClosed()
);

return ClusterBlocks.indicesWithRemoteSnapshotBlockedException(
new HashSet<>(Arrays.asList(indexNameExpressionResolver.concreteIndexNames(state, indicesOptions, request))),
state
);
IndexAbstraction indexAbstraction = state.metadata().getIndicesLookup().get(request.getRolloverTarget());
if (indexAbstraction == null) {
return null;
}
IndexMetadata writeIndex = indexAbstraction.getWriteIndex();
if (writeIndex == null) {
return null;
}
return ClusterBlocks.indicesWithRemoteSnapshotBlockedException(Collections.singletonList(writeIndex.getIndex().getName()), state);
}

@Override
Expand Down
268 changes: 268 additions & 0 deletions .../test/java/org/opensearch/action/admin/indices/rollover/TransportRolloverActionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@
import org.opensearch.action.support.PlainActionFuture;
import org.opensearch.cluster.ClusterName;
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.DataStreamTestHelper;
import org.opensearch.cluster.block.ClusterBlockException;
import org.opensearch.cluster.block.ClusterBlocks;
import org.opensearch.cluster.metadata.AliasMetadata;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
Expand All @@ -58,6 +61,7 @@
import org.opensearch.cluster.routing.UnassignedInfo;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.UUIDs;
import org.opensearch.common.collect.Tuple;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.common.util.concurrent.ThreadContext;
Expand All @@ -66,6 +70,7 @@
import org.opensearch.core.common.unit.ByteSizeUnit;
import org.opensearch.core.common.unit.ByteSizeValue;
import org.opensearch.core.index.shard.ShardId;
import org.opensearch.index.IndexModule;
import org.opensearch.index.cache.query.QueryCacheStats;
import org.opensearch.index.cache.request.RequestCacheStats;
import org.opensearch.index.engine.SegmentsStats;
Expand Down Expand Up @@ -98,8 +103,11 @@

import static java.util.Collections.emptyList;
import static org.opensearch.action.admin.indices.rollover.TransportRolloverAction.evaluateConditions;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyBoolean;
import static org.mockito.Mockito.doAnswer;
Expand Down Expand Up @@ -469,6 +477,266 @@ private static Condition<?> createTestCondition() {
return condition;
}

private TransportRolloverAction newRolloverActionForCheckBlock(ClusterState state) {
ClusterService clusterService = mock(ClusterService.class);
when(clusterService.state()).thenReturn(state);
ThreadPool threadPool = mock(ThreadPool.class);
when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY));
IndexNameExpressionResolver resolver = new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY));
MetadataRolloverService rolloverService = new MetadataRolloverService(
threadPool,
mock(MetadataCreateIndexService.class),
mock(MetadataIndexAliasesService.class),
resolver
);
return new TransportRolloverAction(
mock(TransportService.class),
clusterService,
threadPool,
mock(ActionFilters.class),
resolver,
rolloverService,
mock(Client.class)
);
}

public void testCheckBlockSkipsBlockOnNonWriteAliasMember() {
// Given: an alias whose write index is unblocked, and a non-write
// alias member that carries a METADATA_WRITE block (CCR-style)
String alias = "logs-alias";
String writeIndexName = "logs-000002";
String nonWriteMember = "logs-000001";

IndexMetadata writeIndex = IndexMetadata.builder(writeIndexName)
.settings(settings(Version.CURRENT))
.putAlias(AliasMetadata.builder(alias).writeIndex(true).build())
.numberOfShards(1)
.numberOfReplicas(1)
.build();
IndexMetadata blockedNonWriteMember = IndexMetadata.builder(nonWriteMember)
.settings(settings(Version.CURRENT))
.putAlias(AliasMetadata.builder(alias).writeIndex(false).build())
.numberOfShards(1)
.numberOfReplicas(1)
.build();

ClusterState state = ClusterState.builder(ClusterName.DEFAULT)
.metadata(Metadata.builder().put(writeIndex, false).put(blockedNonWriteMember, false))
// INDEX_WRITE_BLOCK has level WRITE only and would not trip checkBlock's METADATA_WRITE
// probe; INDEX_METADATA_BLOCK is the closest standard constant that includes METADATA_WRITE,
// matching the level CCR's INDEX_REPLICATION_BLOCK uses.
.blocks(ClusterBlocks.builder().addIndexBlock(nonWriteMember, IndexMetadata.INDEX_METADATA_BLOCK).build())
.build();

TransportRolloverAction action = newRolloverActionForCheckBlock(state);

// When: checkBlock runs for a rollover targeting the alias
ClusterBlockException result = action.checkBlock(new RolloverRequest(alias, null), state);

// Then: returns null (the bug fix — non-write member is irrelevant)
assertThat(result, is(nullValue()));
}

public void testCheckBlockAbortsWhenWriteIndexHasMetadataWriteBlock() {
// Given: an alias whose write index has a METADATA_WRITE block applied
String alias = "logs-alias";
String writeIndexName = "logs-000002";

IndexMetadata writeIndex = IndexMetadata.builder(writeIndexName)
.settings(settings(Version.CURRENT))
.putAlias(AliasMetadata.builder(alias).writeIndex(true).build())
.numberOfShards(1)
.numberOfReplicas(1)
.build();

ClusterState state = ClusterState.builder(ClusterName.DEFAULT)
.metadata(Metadata.builder().put(writeIndex, false))
// INDEX_METADATA_BLOCK includes METADATA_WRITE level (see sibling test for rationale).
.blocks(ClusterBlocks.builder().addIndexBlock(writeIndexName, IndexMetadata.INDEX_METADATA_BLOCK).build())
.build();

TransportRolloverAction action = newRolloverActionForCheckBlock(state);

// When: checkBlock runs
ClusterBlockException result = action.checkBlock(new RolloverRequest(alias, null), state);

// Then: returns a ClusterBlockException naming only the write index
// (regression guard for the still-correct abort path)
assertThat(result, is(notNullValue()));
assertThat(result.getMessage(), containsString(writeIndexName));
}

public void testCheckBlockSkipsBlockOnNonWriteDataStreamBacking() {
// Given: a data stream with multiple backing indices; an older backing
// (not the write backing) has a METADATA_WRITE block
String dataStreamName = "logs-ds";
int generations = 2;

ClusterState baseState = DataStreamTestHelper.getClusterStateWithDataStreams(
List.of(new Tuple<>(dataStreamName, generations)),
List.of()
);

// The write backing is the highest-generation index; older backings come first.
String olderBacking = baseState.metadata().dataStreams().get(dataStreamName).getIndices().getFirst().getName();

ClusterState state = ClusterState.builder(baseState)
// INDEX_METADATA_BLOCK includes METADATA_WRITE level (see sibling test for rationale).
.blocks(ClusterBlocks.builder().addIndexBlock(olderBacking, IndexMetadata.INDEX_METADATA_BLOCK).build())
.build();

TransportRolloverAction action = newRolloverActionForCheckBlock(state);

// When: checkBlock runs for a rollover targeting the data stream
ClusterBlockException result = action.checkBlock(new RolloverRequest(dataStreamName, null), state);

// Then: returns null (data-stream parity with the alias case)
assertThat(result, is(nullValue()));
}

public void testCheckBlockSkipsRemoteSnapshotWriteIndexWithoutUserReadOnlyBlock() {
// Given: an alias whose write index is a remote_snapshot. Such indices come with
// an implicit METADATA_WRITE block; without a user-set read-only setting, the
// precheck should defer (parity with ClusterBlocks#indicesWithRemoteSnapshotBlockedException).
String alias = "logs-alias";
String writeIndexName = "logs-000002";

Settings remoteSnapshotSettings = Settings.builder()
.put(settings(Version.CURRENT).build())
.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey())
.build();

IndexMetadata writeIndex = IndexMetadata.builder(writeIndexName)
.settings(remoteSnapshotSettings)
.putAlias(AliasMetadata.builder(alias).writeIndex(true).build())
.numberOfShards(1)
.numberOfReplicas(1)
.build();

ClusterState state = ClusterState.builder(ClusterName.DEFAULT)
.metadata(Metadata.builder().put(writeIndex, false))
.blocks(ClusterBlocks.builder().addIndexBlock(writeIndexName, IndexMetadata.INDEX_METADATA_BLOCK).build())
.build();

TransportRolloverAction action = newRolloverActionForCheckBlock(state);

// When: checkBlock runs
ClusterBlockException result = action.checkBlock(new RolloverRequest(alias, null), state);

// Then: returns null (the implicit remote_snapshot block is exempted)
assertThat(result, is(nullValue()));
}

public void testCheckBlockAbortsWhenRemoteSnapshotHasUserReadOnlyBlock() {
// Given: a remote_snapshot write index with a user-set INDEX_READ_ONLY_SETTING.
// That's an intentional read-only block, so the precheck must still abort.
String alias = "logs-alias";
String writeIndexName = "logs-000002";

Settings remoteSnapshotSettings = Settings.builder()
.put(settings(Version.CURRENT).build())
.put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.REMOTE_SNAPSHOT.getSettingsKey())
.put(IndexMetadata.SETTING_READ_ONLY, true)
.build();

IndexMetadata writeIndex = IndexMetadata.builder(writeIndexName)
.settings(remoteSnapshotSettings)
.putAlias(AliasMetadata.builder(alias).writeIndex(true).build())
.numberOfShards(1)
.numberOfReplicas(1)
.build();

ClusterState state = ClusterState.builder(ClusterName.DEFAULT)
.metadata(Metadata.builder().put(writeIndex, false))
.blocks(ClusterBlocks.builder().addIndexBlock(writeIndexName, IndexMetadata.INDEX_METADATA_BLOCK).build())
.build();

TransportRolloverAction action = newRolloverActionForCheckBlock(state);

// When: checkBlock runs
ClusterBlockException result = action.checkBlock(new RolloverRequest(alias, null), state);

// Then: aborts naming the write index (user opted in to read-only)
assertThat(result, is(notNullValue()));
assertThat(result.getMessage(), containsString(writeIndexName));
}

public void testCheckBlockReturnsNullWhenRolloverTargetUnresolvable() {
// Given: cluster state with no abstraction matching the rollover target
ClusterState state = ClusterState.builder(ClusterName.DEFAULT).metadata(Metadata.builder()).build();

TransportRolloverAction action = newRolloverActionForCheckBlock(state);

// When: checkBlock runs
ClusterBlockException result = action.checkBlock(new RolloverRequest("does-not-exist", null), state);

// Then: returns null (defers to main path's canonical "not found" error)
assertThat(result, is(nullValue()));
}

public void testCheckBlockReturnsNullWhenAliasHasNoWriteMember() {
// Given: an alias with members but none marked is_write_index=true.
// (Multiple non-write members with no write member is the only shape that makes
// Alias#getWriteIndex() return null; a single non-write member is implicitly the write index.)
String alias = "logs-alias";
String memberA = "logs-000001";
String memberB = "logs-000002";

IndexMetadata indexA = IndexMetadata.builder(memberA)
.settings(settings(Version.CURRENT))
.putAlias(AliasMetadata.builder(alias).writeIndex(false).build())
.numberOfShards(1)
.numberOfReplicas(1)
.build();
IndexMetadata indexB = IndexMetadata.builder(memberB)
.settings(settings(Version.CURRENT))
.putAlias(AliasMetadata.builder(alias).writeIndex(false).build())
.numberOfShards(1)
.numberOfReplicas(1)
.build();

ClusterState state = ClusterState.builder(ClusterName.DEFAULT)
.metadata(Metadata.builder().put(indexA, false).put(indexB, false))
.build();

TransportRolloverAction action = newRolloverActionForCheckBlock(state);

// When: checkBlock runs
ClusterBlockException result = action.checkBlock(new RolloverRequest(alias, null), state);

// Then: returns null — defers to MetadataRolloverService's canonical
// "rollover target [...] does not point to a write index" error.
assertThat(result, is(nullValue()));
}

public void testCheckBlockAbortsWhenDataStreamWriteBackingBlocked() {
// Given: a data stream where the write (highest-generation) backing carries a METADATA_WRITE block
String dataStreamName = "logs-ds";
int generations = 2;

ClusterState baseState = DataStreamTestHelper.getClusterStateWithDataStreams(
List.of(new Tuple<>(dataStreamName, generations)),
List.of()
);

// Write backing is the highest-generation index — the last in the indices list.
List<org.opensearch.core.index.Index> backings = baseState.metadata().dataStreams().get(dataStreamName).getIndices();
String writeBacking = backings.getLast().getName();

ClusterState state = ClusterState.builder(baseState)
.blocks(ClusterBlocks.builder().addIndexBlock(writeBacking, IndexMetadata.INDEX_METADATA_BLOCK).build())
.build();

TransportRolloverAction action = newRolloverActionForCheckBlock(state);

// When: checkBlock runs for a rollover targeting the data stream
ClusterBlockException result = action.checkBlock(new RolloverRequest(dataStreamName, null), state);

// Then: aborts naming the write backing — symmetry with the alias write-index-blocked case.
assertThat(result, is(notNullValue()));
assertThat(result.getMessage(), containsString(writeBacking));
}

public static IndicesStatsResponse randomIndicesStatsResponse(final IndexMetadata[] indices) {
List<ShardStats> shardStats = new ArrayList<>();
for (final IndexMetadata index : indices) {
Expand Down
Loading