Refactor indices aliases #2

joegallo · 2021-11-15T21:29:55Z

Please don't actually merge this. I'd rather we use git trickery to update your branch without merging this PR. But I did want some easy way to say "look at what I've done here since we last spoke" and this is a half decent way of doing that.

Annoyingly there's some (IMHO) unnecessary noise because I merged master in.

In conclusion, reiterating: please don't actually merge this.

❤️

* Reorganizes the 8.0.0 and 8.1.0 breaking changes and deprecations into component-based categories. * Adds an ESS icon to cluster settings on the ESS user settings allowlist. * Adds tips for sections that aren't relevant to Cloud users. * Updates the labels for some items to provide better context. Co-authored-by: debadair <debadair@elastic.co>

…80696) We currently throw a message that always refers to the `_parent` field, even though it could be throw from any metadata field, and indeed the `_parent` field hasn't existed for several versions now.

Includes command line tool deprecations in the notable changes tag.

getIndices() and getAliases() go through indices() and aliases(), so do that here, too.

Makes several changes to consolidate snapshot and backup-related docs. Highlights: * Adds info about supported ESS snapshot repository types * Adds docs for Kibana's Snapshot and Restore feature * Combines tutorial pages related to taking and managing snapshots * Consolidates explanations of the snapshot process * Incorporates SLM into the snapshot tutorial * Removes duplicate "back up a cluster" pages

Update the notes in TESTING.asciidoc about how to build a distribution of Elasticsearch.

martijnvg · 2021-11-16T09:33:00Z

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

    public ImmutableOpenMap<String, IndexTemplateMetadata> templates() {
        return this.templates;
    }

    public ImmutableOpenMap<String, IndexTemplateMetadata> getTemplates() {
-        return this.templates;
+        return templates();


unrelated change?

Not entirely unrelated, see 7f25f49.

martijnvg · 2021-11-16T09:35:09Z

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

+            previousIndicesLookup = null;
+
+            List<Index> indices = new ArrayList<>(aliases.getOrDefault(alias, List.of()));
+            indices.remove(index); // TODO what if it wasn't in the list already?


maybe:

if (indices.remove(index) == false) { return this; }

Which would make this a no-op.

martijnvg · 2021-11-16T09:37:27Z

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

+            previousIndicesLookup = null;
+
+            List<Index> indices = new ArrayList<>(aliases.getOrDefault(alias, List.of()));
+            indices.add(index); // TODO what if it's a duplicate? maybe use a Set<Index> instead?


I think it easier if we make we change the aliases map to contain a Set instead of if List.

and also maybe:

if (indices.add(index) == false) { return this; }

to make this a no-op in case .

++, I'll make the List<Index> --> Set<Index> change throughout, and let's see what that gets us.

Oh, actually, I just remembered -- https://github.com/elastic/elasticsearch/blob/fed20aa74b48b2220870c782027a69de305bf56c/server/src/main/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexService.java#L1147-L1150 makes me think there's existing semantics around the order of the aliases for an index, might we want to preserve something that like here? That is, is there value in having List semantics rather than Set semantics or no?

I don't think that is the case, once an index and its aliases are created.

martijnvg · 2021-11-16T10:09:38Z

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

+            .blocks(ClusterBlocks.builder().addBlocks(idxMetadata))
+            .build();
+
+        ClusterState after = service.deleteIndices(before, Set.of(before.metadata().getIndices().get(index).getIndex()));


maybe check that that before the delete call an alias entry does exist?

martijnvg · 2021-11-16T10:13:10Z

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

@@ -124,6 +124,13 @@ public ClusterState deleteIndices(ClusterState currentState, Set<Index> indices)
                metadataBuilder.put(parent.removeBackingIndex(index));
            }
        }
+        // delete associated aliases from the cluster state


Maybe inline this into Metadata.Builder#remove(...) method? If an IndexMetadata has aliases then we always need to invoke the remobeAlias() method (and we don't need to check whether aliases have changed).

…c#100592) * Don't print synthetic source in mapping for bwc tests * Move comment. * Don't print synthetic source in mapping for bwc tests #2 * Don't print synthetic source in mapping for bwc tests #2 * Revert "Don't print synthetic source in mapping for bwc tests #2" This reverts commit 034262c. * Revert "Don't print synthetic source in mapping for bwc tests #2" This reverts commit 44e8156. * Revert "Don't print synthetic source in mapping for bwc tests (elastic#100572)" This reverts commit 9322ab9. * Exclude synthetic source test from mixedClusterTests * Update comment.

….10 (elastic#100805) * Don't print synthetic source in mapping for bwc tests * Move comment. * Don't print synthetic source in mapping for bwc tests #2 * Don't print synthetic source in mapping for bwc tests #2 * Revert "Don't print synthetic source in mapping for bwc tests #2" This reverts commit 034262c. * Revert "Don't print synthetic source in mapping for bwc tests #2" This reverts commit 44e8156. * Revert "Don't print synthetic source in mapping for bwc tests (elastic#100572)" This reverts commit 9322ab9. * Exclude synthetic source test from mixedClusterTests * Update comment. * Mute all tsdb tests in mixedClusterTests This is an interim step to stop sporadic test failures, while we try to fix version skip for mixed cluster tests. * Remove old exclusion * Add aggregations too * Mute tests for versions between 8.7-8.10 * Remove mute

* Don't print synthetic source in mapping for bwc tests * Move comment. * Don't print synthetic source in mapping for bwc tests #2 * Don't print synthetic source in mapping for bwc tests #2 * Revert "Don't print synthetic source in mapping for bwc tests #2" This reverts commit 034262c. * Revert "Don't print synthetic source in mapping for bwc tests #2" This reverts commit 44e8156. * Revert "Don't print synthetic source in mapping for bwc tests (elastic#100572)" This reverts commit 9322ab9. * Exclude synthetic source test from mixedClusterTests * Update comment. * Mute all tsdb tests in mixedClusterTests This is an interim step to stop sporadic test failures, while we try to fix version skip for mixed cluster tests. * Remove old exclusion * Add aggregations too * Mute tests for versions between 8.7-8.10 * Remove mute * Restore version skipping for position fields * Restore version skip for synthetic source

….10 (elastic#100805) (elastic#100814) * Don't print synthetic source in mapping for bwc tests * Move comment. * Don't print synthetic source in mapping for bwc tests #2 * Don't print synthetic source in mapping for bwc tests #2 * Revert "Don't print synthetic source in mapping for bwc tests #2" This reverts commit 034262c. * Revert "Don't print synthetic source in mapping for bwc tests #2" This reverts commit 44e8156. * Revert "Don't print synthetic source in mapping for bwc tests (elastic#100572)" This reverts commit 9322ab9. * Exclude synthetic source test from mixedClusterTests * Update comment. * Mute all tsdb tests in mixedClusterTests This is an interim step to stop sporadic test failures, while we try to fix version skip for mixed cluster tests. * Remove old exclusion * Add aggregations too * Mute tests for versions between 8.7-8.10 * Remove mute

…c#100823) * Don't print synthetic source in mapping for bwc tests * Move comment. * Don't print synthetic source in mapping for bwc tests #2 * Don't print synthetic source in mapping for bwc tests #2 * Revert "Don't print synthetic source in mapping for bwc tests #2" This reverts commit 034262c. * Revert "Don't print synthetic source in mapping for bwc tests #2" This reverts commit 44e8156. * Revert "Don't print synthetic source in mapping for bwc tests (elastic#100572)" This reverts commit 9322ab9. * Exclude synthetic source test from mixedClusterTests * Update comment. * Mute all tsdb tests in mixedClusterTests This is an interim step to stop sporadic test failures, while we try to fix version skip for mixed cluster tests. * Remove old exclusion * Add aggregations too * Mute tests for versions between 8.7-8.10 * Remove mute * Restore version skipping for position fields * Restore version skip for synthetic source

When we run the csv-spec tests for ESQL against a real http endpoint we actually run them twice - once async and once sync. But the names of the tests didn't reflect that - they just looked like they were accidentally duplicated. This updates the format. So this: ``` test {string.Trim} test {string.Trim #2} ``` becomes: ``` test {string.Trim ASYNC} test {string.Trim SYNC} ```

…8.7 - 8.10 (elastic#100805) (elastic#100815) * [TEST] Mute all tsdb tests in mixedClusterTests, for versions 8.7 - 8.10 (elastic#100805) * Don't print synthetic source in mapping for bwc tests * Move comment. * Don't print synthetic source in mapping for bwc tests #2 * Don't print synthetic source in mapping for bwc tests #2 * Revert "Don't print synthetic source in mapping for bwc tests #2" This reverts commit 034262c. * Revert "Don't print synthetic source in mapping for bwc tests #2" This reverts commit 44e8156. * Revert "Don't print synthetic source in mapping for bwc tests (elastic#100572)" This reverts commit 9322ab9. * Exclude synthetic source test from mixedClusterTests * Update comment. * Mute all tsdb tests in mixedClusterTests This is an interim step to stop sporadic test failures, while we try to fix version skip for mixed cluster tests. * Remove old exclusion * Add aggregations too * Mute tests for versions between 8.7-8.10 * Remove mute * Fix skip for position fields

…rce (elastic#100827) * [TEST] Mute all tsdb tests in mixedClusterTests, for versions 8.7 - 8.10 (elastic#100805) * Don't print synthetic source in mapping for bwc tests * Move comment. * Don't print synthetic source in mapping for bwc tests #2 * Don't print synthetic source in mapping for bwc tests #2 * Revert "Don't print synthetic source in mapping for bwc tests #2" This reverts commit 034262c. * Revert "Don't print synthetic source in mapping for bwc tests #2" This reverts commit 44e8156. * Revert "Don't print synthetic source in mapping for bwc tests (elastic#100572)" This reverts commit 9322ab9. * Exclude synthetic source test from mixedClusterTests * Update comment. * Mute all tsdb tests in mixedClusterTests This is an interim step to stop sporadic test failures, while we try to fix version skip for mixed cluster tests. * Remove old exclusion * Add aggregations too * Mute tests for versions between 8.7-8.10 * Remove mute * Fix skip for position fields * Restore version skip for synthetic source

…t {stats.Count_or_null SYNC #2} elastic#110950

…alCentroidTests testAggregateIntermediate {TestCase=<geo_point> #2} elastic#112461

…sts testFold {TestCase=<double> #2} elastic#113225

…uginFuncTest builds distribution from branches via archives extractedAssemble [bwcDistVersion: 8.2.1, bwcProject: bugfix, expectedAssembleTaskName: extractedAssemble, #2] elastic#119871

jrodewig and others added 19 commits November 15, 2021 10:42

Improve doc parsing errors for badly formed metadata fields (elastic#…

3ca033f

…80696) We currently throw a message that always refers to the `_parent` field, even though it could be throw from any metadata field, and indeed the `_parent` field hasn't existed for several versions now.

[DOCS] Fix 8.0 breaking change tags (elastic#80725)

987010d

Includes command line tool deprecations in the notable changes tag.

Add getters for aliases

43184e2

Increase consistency

7f25f49

getIndices() and getAliases() go through indices() and aliases(), so do that here, too.

Revise notes in TESTING.asciidoc about building a distro (elastic#80727)

4d18ba6

Update the notes in TESTING.asciidoc about how to build a distribution of Elasticsearch.

Add example plugins pull request check

4ba14c4

Failing unit test

8fdb5de

And make it pass

4eeb57a

Test the newly created alias, too

1240353

Failing unit test

e036812

And make it pass

7ca1c0b

Fix references to moved classes in plugin examples (elastic#80736)

c6cdbf8

Clean up imports / use of liberally

110867e

Test for deleting an index with an alias

55175c3

Failing unit test

9e0d266

And make it pass

9703cd8

Merge branch 'master' into refactor_indices_aliases

41dd231

martijnvg reviewed Nov 16, 2021

View reviewed changes

joegallo closed this Nov 16, 2021

martijnvg pushed a commit that referenced this pull request Aug 14, 2024

Mute org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT tes…

d990664

…t {stats.Count_or_null SYNC #2} elastic#110950

martijnvg pushed a commit that referenced this pull request Sep 3, 2024

Mute org.elasticsearch.xpack.esql.expression.function.aggregate.Spati…

301f3fb

…alCentroidTests testAggregateIntermediate {TestCase=<geo_point> #2} elastic#112461

martijnvg pushed a commit that referenced this pull request Sep 20, 2024

Mute org.elasticsearch.xpack.esql.expression.function.aggregate.AvgTe…

4b8671b

…sts testFold {TestCase=<double> #2} elastic#113225

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor indices aliases #2

Refactor indices aliases #2

joegallo commented Nov 15, 2021

martijnvg Nov 16, 2021

joegallo Nov 16, 2021

martijnvg Nov 16, 2021

martijnvg Nov 16, 2021

joegallo Nov 16, 2021

joegallo Nov 16, 2021

martijnvg Nov 16, 2021

martijnvg Nov 16, 2021

martijnvg Nov 16, 2021

Refactor indices aliases #2

Refactor indices aliases #2

Conversation

joegallo commented Nov 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment