Fix corrupted Metadata from index and alias having the same name by original-brownbear · Pull Request #91456 · elastic/elasticsearch

original-brownbear · 2022-11-09T12:19:57Z

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.

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.

elasticsearchmachine · 2022-11-09T12:20:21Z

Pinging @elastic/es-distributed (Team:Distributed)

elasticsearchmachine · 2022-11-09T12:20:21Z

Hi @original-brownbear, I've created a changelog YAML for you.

…index-collision

…ion' into hunt-down-bug-alias-index-collision

henningandersen

LGTM.

henningandersen · 2022-11-09T13:45:12Z

server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java

+            var aliasesMap = aliases.build();
+            aliasesMap.forEach((key, value) -> {
+                if (value.alias().equals(index)) {
+                    throw new IllegalArgumentException("alias name [" + index + "] and index name may not be the same");


Can we make this a strong message about "conflict":

Suggested change

throw new IllegalArgumentException("alias name [" + index + "] and index name may not be the same");

throw new IllegalArgumentException("alias name [" + index + "] self-conflicts with index name");

henningandersen · 2022-11-09T13:55:37Z

server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java

        assertThat(aliases.get(1).alias(), equalTo("bb"));
    }

-    public void testIndexAndAliasWithSameName() {


I was trying to find a case where we'd build IndexMetadata instances that temporarily breaks this. The one case I could think of was the aliases action where we can add and remove aliases/indices. AFAICS, it does try to do things in order. I think this is OK, but wanted to mention in case you have a case yourself you also want to check.

Obviously we should have tests failling then, but this is pretty old so might not necessarily be covered well.

Yea I checked for that and at least couldn't find any broken spots (all of the spots that may throw seem to have safe listeners). If anything this should at least cause a better exception here and there.

arteam · 2022-11-09T14:11:06Z

server/src/test/java/org/elasticsearch/cluster/metadata/MetadataTests.java

-            aliasToIndices.put(alias, new HashSet<>(randomSubsetOf(randomIntBetween(1, 3), indices)));
+            Set<String> indicesInAlias;
+            do {
+                indicesInAlias = new HashSet<>(randomSubsetOf(randomIntBetween(1, 3), indices));


WDYT about initializing indicesInAlias in one go?

indicesInAlias = randomSubsetOf(randomIntBetween(1, 3), indices).stream() .filter(e -> e.equals(alias) == false) .collect(Collectors.toSet())

server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java

arteam

LGTM! I've left a few small comments

original-brownbear · 2022-11-09T15:12:34Z

Thanks both!

elasticsearchmachine · 2022-11-09T15:13:33Z

💔 Backport failed

Status	Branch	Result
❌	8.5	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 91456

) (#91469) 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.

…#91887) Follow up to #91456 implementing an automated fix for indices corrupted in 8.5.

…#91887) (#91988) Follow up to #91456 implementing an automated fix for indices corrupted in 8.5.

…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.

Relates elastic#91456 Relates elastic#91887

Relates #91456 Relates #91887

original-brownbear added >bug :Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. v8.5.1 v8.6.0 labels Nov 9, 2022

elasticsearchmachine added the Team:Distributed Meta label for distributed team. label Nov 9, 2022

original-brownbear added 3 commits November 9, 2022 13:20

Update docs/changelog/91456.yaml

2d973a8

Merge remote-tracking branch 'elastic/main' into hunt-down-bug-alias-…

94eb3e2

…index-collision

Merge remote-tracking branch 'origin/hunt-down-bug-alias-index-collis…

9692e0d

…ion' into hunt-down-bug-alias-index-collision

arteam self-requested a review November 9, 2022 13:25

henningandersen reviewed Nov 9, 2022

View reviewed changes

arteam reviewed Nov 9, 2022

View reviewed changes

server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java Outdated Show resolved Hide resolved

arteam approved these changes Nov 9, 2022

View reviewed changes

reword'

209a3e9

original-brownbear added the auto-backport-and-merge label Nov 9, 2022

original-brownbear merged commit a867ba1 into elastic:main Nov 9, 2022

original-brownbear deleted the hunt-down-bug-alias-index-collision branch November 9, 2022 15:12

original-brownbear mentioned this pull request Nov 9, 2022

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

Merged

fdartayre mentioned this pull request Nov 15, 2022

Can create an index and an alias with the same name #91580

Closed

original-brownbear mentioned this pull request Nov 24, 2022

Implement repair functionality for aliases colliding with indices bug #91887

Merged

original-brownbear added a commit that referenced this pull request Nov 29, 2022

Implement repair functionality for aliases colliding with indices bug (…

046592e

…#91887) Follow up to #91456 implementing an automated fix for indices corrupted in 8.5.

elasticsearchmachine pushed a commit that referenced this pull request Nov 29, 2022

Implement repair functionality for aliases colliding with indices bug (…

14daacf

…#91887) (#91988) Follow up to #91456 implementing an automated fix for indices corrupted in 8.5.

original-brownbear mentioned this pull request Nov 29, 2022

[8.5] Implement repair functionality for aliases colliding with indices bug #91993

Merged

DaveCTurner mentioned this pull request Dec 4, 2022

Create an index with the same name as an alias results in illegal_argument_exception for any action #92078

Closed

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Dec 12, 2022

Add known-issue docs for index/alias name clash bug

b0e4ece

Relates elastic#91456 Relates elastic#91887

DaveCTurner mentioned this pull request Dec 12, 2022

Add known-issue docs for index/alias name clash bug #92262

Merged

DaveCTurner added a commit that referenced this pull request Dec 12, 2022

Add known-issue docs for index/alias name clash bug (#92262)

b0af79d

Relates #91456 Relates #91887

DaveCTurner added a commit that referenced this pull request Dec 12, 2022

Add known-issue docs for index/alias name clash bug (#92262)

ab602ae

Relates #91456 Relates #91887

DaveCTurner added a commit that referenced this pull request Dec 12, 2022

Add known-issue docs for index/alias name clash bug (#92262)

cdb32ef

Relates #91456 Relates #91887

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

Fix corrupted Metadata from index and alias having the same name#91456
original-brownbear merged 5 commits intoelastic:mainfrom
original-brownbear:hunt-down-bug-alias-index-collision

original-brownbear commented Nov 9, 2022

Uh oh!

elasticsearchmachine commented Nov 9, 2022

Uh oh!

elasticsearchmachine commented Nov 9, 2022

Uh oh!

henningandersen left a comment

Uh oh!

henningandersen Nov 9, 2022

Uh oh!

henningandersen Nov 9, 2022

Uh oh!

original-brownbear Nov 9, 2022

Uh oh!

arteam Nov 9, 2022

Uh oh!

Uh oh!

arteam left a comment

Uh oh!

original-brownbear commented Nov 9, 2022

Uh oh!

elasticsearchmachine commented Nov 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	throw new IllegalArgumentException("alias name [" + index + "] and index name may not be the same");
	throw new IllegalArgumentException("alias name [" + index + "] self-conflicts with index name");

Conversation

original-brownbear commented Nov 9, 2022

Uh oh!

elasticsearchmachine commented Nov 9, 2022

Uh oh!

elasticsearchmachine commented Nov 9, 2022

Uh oh!

henningandersen left a comment

Choose a reason for hiding this comment

Uh oh!

henningandersen Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

henningandersen Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

original-brownbear Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

arteam Nov 9, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arteam left a comment

Choose a reason for hiding this comment

Uh oh!

original-brownbear commented Nov 9, 2022

Uh oh!

elasticsearchmachine commented Nov 9, 2022

💔 Backport failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants