Skip to content

Commit

Permalink
[8.5] Implement repair functionality for aliases colliding with indic…
Browse files Browse the repository at this point in the history
…es bug (#91993)

* Fix corrupted Metadata from index and alias having the same name (#91456)

This fixes a bug introduced in #87863
which added a Metadata copy constructor with separate name collision checks that
assumed index name and alias names were already validated in `IndexMetada`.
=> fixed corrupted states by actually adding the validation to `IndexMetadata`
to make it impossible to instantiate a broken `IndexMetadata` in the first place.

* Implement repair functionality for aliases colliding with indices bug (#91887)

Follow up to #91456 implementing an automated fix for indices corrupted in 8.5.
  • Loading branch information
original-brownbear committed Nov 29, 2022
1 parent 2673202 commit 8039973
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 15 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/91456.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 91456
summary: Fix corrupted Metadata from index and alias having the same name
area: Cluster Coordination
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

package org.elasticsearch.cluster.metadata;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.Version;
import org.elasticsearch.action.admin.indices.rollover.RolloverInfo;
import org.elasticsearch.action.support.ActiveShardCount;
Expand Down Expand Up @@ -82,6 +84,8 @@

public class IndexMetadata implements Diffable<IndexMetadata>, ToXContentFragment {

private static final Logger logger = LogManager.getLogger(IndexMetadata.class);

public static final ClusterBlock INDEX_READ_ONLY_BLOCK = new ClusterBlock(
5,
"index read-only (api)",
Expand Down Expand Up @@ -1512,7 +1516,7 @@ public IndexMetadata apply(IndexMetadata part) {
builder.rolloverInfos.putAllFromMap(rolloverInfos.apply(part.rolloverInfos));
builder.system(isSystem);
builder.timestampRange(timestampRange);
return builder.build();
return builder.build(true);
}
}

Expand Down Expand Up @@ -1573,7 +1577,7 @@ public static IndexMetadata readFrom(StreamInput in, @Nullable Function<String,
builder.system(in.readBoolean());
}
builder.timestampRange(IndexLongFieldRange.readFrom(in));
return builder.build();
return builder.build(true);
}

/**
Expand Down Expand Up @@ -1903,6 +1907,11 @@ public Builder timestampRange(IndexLongFieldRange timestampRange) {
}

public IndexMetadata build() {
return build(false);
}

// package private for testing
IndexMetadata build(boolean repair) {
/*
* We expect that the metadata has been properly built to set the number of shards and the number of replicas, and do not rely
* on the default values here. Those must have been set upstream.
Expand Down Expand Up @@ -2020,7 +2029,16 @@ public IndexMetadata build() {
var aliasesMap = aliases.build();
for (AliasMetadata alias : aliasesMap.values()) {
if (alias.alias().equals(index)) {
throw new IllegalArgumentException("alias name [" + index + "] self-conflicts with index name");
if (repair && indexCreatedVersion.equals(Version.V_8_5_0)) {
var updatedBuilder = ImmutableOpenMap.builder(aliasesMap);
final var brokenAlias = updatedBuilder.remove(index);
final var fixedAlias = AliasMetadata.newAliasMetadata(brokenAlias, index + "-alias-corrupted-by-8-5");
aliasesMap = updatedBuilder.fPut(fixedAlias.getAlias(), fixedAlias).build();
logger.warn("Repaired corrupted alias with the same name as its index for [{}]", index);
break;
} else {
throw new IllegalArgumentException("alias name [" + index + "] self-conflicts with index name");
}
}
}

Expand Down Expand Up @@ -2321,7 +2339,7 @@ public static IndexMetadata fromXContent(XContentParser parser, Map<String, Mapp
assert settingsVersion : "settings version should be present for indices created on or after 6.5.0";
assert indexCreatedVersion(builder.settings).before(Version.V_7_2_0) || aliasesVersion
: "aliases version should be present for indices created on or after 7.2.0";
return builder.build();
return builder.build(true);
}

/**
Expand Down Expand Up @@ -2433,7 +2451,7 @@ public static IndexMetadata legacyFromXContent(XContentParser parser) throws IOE
builder.putMapping(MappingMetadata.EMPTY_MAPPINGS); // just make sure it's not empty so that _source can be read
}

IndexMetadata indexMetadata = builder.build();
IndexMetadata indexMetadata = builder.build(true);
assert indexMetadata.getCreationVersion().isLegacyIndexVersion();
assert indexMetadata.getCompatibilityVersion().isLegacyIndexVersion();
return indexMetadata;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import static org.elasticsearch.cluster.metadata.IndexMetadata.parseIndexNameCounter;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.is;

public class IndexMetadataTests extends ESTestCase {
Expand Down Expand Up @@ -483,16 +484,40 @@ public void testLifeCyclePolicyName() {
}

public void testIndexAndAliasWithSameName() {
final IllegalArgumentException iae = expectThrows(
IllegalArgumentException.class,
() -> IndexMetadata.builder("index")
.settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT))
.numberOfShards(1)
.numberOfReplicas(0)
.putAlias(AliasMetadata.builder("index").build())
.build()
);
assertEquals("alias name [index] self-conflicts with index name", iae.getMessage());
{
final IllegalArgumentException iae = expectThrows(
IllegalArgumentException.class,
() -> IndexMetadata.builder("index")
.settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT))
.numberOfShards(1)
.numberOfReplicas(0)
.putAlias(AliasMetadata.builder("index").build())
.build(randomBoolean())
);
assertEquals("alias name [index] self-conflicts with index name", iae.getMessage());
}
{
final IllegalArgumentException iae = expectThrows(
IllegalArgumentException.class,
() -> IndexMetadata.builder("index")
.settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.V_8_5_0))
.numberOfShards(1)
.numberOfReplicas(0)
.putAlias(AliasMetadata.builder("index").build())
.build(false)
);
assertEquals("alias name [index] self-conflicts with index name", iae.getMessage());
}
}

public void testRepairIndexAndAliasWithSameName() {
final IndexMetadata indexMetadata = IndexMetadata.builder("index")
.settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.V_8_5_0))
.numberOfShards(1)
.numberOfReplicas(0)
.putAlias(AliasMetadata.builder("index").build())
.build(true);
assertThat(indexMetadata.getAliases(), hasKey("index-alias-corrupted-by-8-5"));
}

private static Settings indexSettingsWithDataTier(String dataTier) {
Expand Down

0 comments on commit 8039973

Please sign in to comment.