Skip to content

Commit d698e72

Browse files
authored
[8.x] Add a cluster listener to fix missing system index mappings after upgrade (#115771)
This PR modifies `TransportVersionsFixupListener` to include all of compatibility versions (not only TransportVersion) in the fixup. `TransportVersionsFixupListener` spots the instances when the master has been upgraded to the most recent code version, along with non-master nodes, but some nodes are missing a "proper" (non-inferred) Transport version. This PR adds another check to also ensure that we have real (non-empty) system index mapping versions. To do so, it modifies NodeInfo so it carries all of CompatibilityVersions (TransportVersion + SystemIndexDescriptor.MappingVersions). This was initially done via a separate fixup listener + ad-hoc transport action, but the 2 listeners "raced" to update ClusterState on the same CompatibilityVersions structure; it just made sense to do it at the same time. The fixup is very similar to #110710, which does the same for cluster features; plus, it adds a CI test to cover the bug raised in #112694 Closes #112694
1 parent a9003c9 commit d698e72

File tree

5 files changed

+428
-49
lines changed

5 files changed

+428
-49
lines changed
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.upgrades;
11+
12+
import org.elasticsearch.Build;
13+
import org.elasticsearch.client.Request;
14+
import org.elasticsearch.client.Response;
15+
import org.elasticsearch.common.settings.Settings;
16+
import org.elasticsearch.core.SuppressForbidden;
17+
import org.elasticsearch.core.UpdateForV9;
18+
import org.elasticsearch.test.cluster.ElasticsearchCluster;
19+
import org.elasticsearch.test.cluster.local.distribution.DistributionType;
20+
import org.elasticsearch.test.cluster.util.Version;
21+
import org.elasticsearch.test.rest.ESRestTestCase;
22+
import org.elasticsearch.test.rest.ObjectPath;
23+
import org.junit.ClassRule;
24+
import org.junit.rules.RuleChain;
25+
import org.junit.rules.TemporaryFolder;
26+
import org.junit.rules.TestRule;
27+
28+
import java.io.IOException;
29+
import java.util.List;
30+
import java.util.Map;
31+
import java.util.function.Supplier;
32+
33+
import static org.elasticsearch.test.LambdaMatchers.transformedMatch;
34+
import static org.hamcrest.Matchers.allOf;
35+
import static org.hamcrest.Matchers.anEmptyMap;
36+
import static org.hamcrest.Matchers.anyOf;
37+
import static org.hamcrest.Matchers.everyItem;
38+
import static org.hamcrest.Matchers.hasKey;
39+
import static org.hamcrest.Matchers.not;
40+
41+
public class SystemIndexMappingUpgradeIT extends ESRestTestCase {
42+
43+
private static final String OLD_CLUSTER_VERSION = System.getProperty("tests.old_cluster_version");
44+
private static final TemporaryFolder repoDirectory = new TemporaryFolder();
45+
46+
private static final ElasticsearchCluster cluster = ElasticsearchCluster.local()
47+
.distribution(DistributionType.DEFAULT)
48+
.version(Version.fromString(OLD_CLUSTER_VERSION))
49+
.nodes(3)
50+
.setting("path.repo", new Supplier<>() {
51+
@Override
52+
@SuppressForbidden(reason = "TemporaryFolder only has io.File methods, not nio.File")
53+
public String get() {
54+
return repoDirectory.getRoot().getPath();
55+
}
56+
})
57+
.setting("xpack.security.enabled", "false")
58+
.build();
59+
60+
@ClassRule
61+
public static TestRule ruleChain = RuleChain.outerRule(repoDirectory).around(cluster);
62+
63+
private String testRestCluster = cluster.getHttpAddresses();
64+
65+
@Override
66+
protected String getTestRestCluster() {
67+
return testRestCluster;
68+
}
69+
70+
@Override
71+
protected final Settings restClientSettings() {
72+
return Settings.builder()
73+
.put(super.restClientSettings())
74+
// increase the timeout here to 90 seconds to handle long waits for a green
75+
// cluster health. the waits for green need to be longer than a minute to
76+
// account for delayed shards
77+
.put(ESRestTestCase.CLIENT_SOCKET_TIMEOUT, "90s")
78+
.build();
79+
}
80+
81+
private static String getNodesAttribute(ObjectPath objectPath, String nodeId, String attribute) {
82+
try {
83+
return objectPath.evaluate("nodes." + nodeId + "." + attribute);
84+
} catch (IOException e) {
85+
throw new RuntimeException(e);
86+
}
87+
}
88+
89+
private static List<String> getUpgradedNodesAddresses() throws IOException {
90+
Response response = client().performRequest(new Request("GET", "_nodes"));
91+
ObjectPath objectPath = ObjectPath.createFromResponse(response);
92+
Map<String, Object> nodesAsMap = objectPath.evaluate("nodes");
93+
return nodesAsMap.keySet()
94+
.stream()
95+
.filter(id -> getNodesAttribute(objectPath, id, "version").equals(Build.current().version())) // nodes on current version
96+
.map(id -> getNodesAttribute(objectPath, id, "http.publish_address"))
97+
.toList();
98+
}
99+
100+
@SuppressWarnings("unchecked")
101+
private List<Map<String, Object>> fieldAsObjectList(Map<? extends String, ?> objectMap, String field) {
102+
return (List<Map<String, Object>>) (objectMap.get(field));
103+
}
104+
105+
@SuppressWarnings("unchecked")
106+
private Map<String, Object> fieldAsObject(Map<? extends String, ?> objectMap, String field) {
107+
return (Map<String, Object>) (objectMap.get(field));
108+
}
109+
110+
@UpdateForV9()
111+
public void testGrowShrinkUpgradeUpdatesSystemIndexMapping() throws Exception {
112+
/*
113+
* From 8.11, CompatibilityVersions holds a map of system index names to their mappings versions, alongside the transport version.
114+
* See https://github.com/elastic/elasticsearch/pull/99307
115+
* The problem was fixed in 8.16.1/8.17, so we want to test upgrade from a cluster pre-8.11 to a cluster post-8.16.0
116+
* For testing the first condition, we use a synthetic cluster feature. The second condition is implied, as the fix and this test
117+
* are applied only to post-8.16.0
118+
*/
119+
assumeFalse(
120+
"Testing upgrades from before CompatibilityVersions held mapping versions in cluster state",
121+
testFeatureService.clusterHasFeature("gte_v8.11.0")
122+
);
123+
124+
// Upgrade node 0 and 1 to the current version, leave node 2 to the BwC version
125+
logger.info("Upgrading node 0 to version {}", Version.CURRENT);
126+
cluster.upgradeNodeToVersion(0, Version.CURRENT);
127+
128+
logger.info("Upgrading node 1 to version {}", Version.CURRENT);
129+
cluster.upgradeNodeToVersion(1, Version.CURRENT);
130+
131+
// Query the nodes, ensure we do _not_ have node versions in the answer, or if we do, mappings are empty
132+
Map<String, Object> nodesVersions = entityAsMap(client().performRequest(new Request("GET", "/_cluster/state")));
133+
assertThat(
134+
nodesVersions,
135+
anyOf(
136+
not(hasKey("nodes_versions")),
137+
transformedMatch(
138+
x -> fieldAsObjectList(x, "nodes_versions"),
139+
everyItem(transformedMatch(item -> fieldAsObject(item, "mappings_versions"), anEmptyMap()))
140+
)
141+
)
142+
);
143+
144+
var upgradedNodes = getUpgradedNodesAddresses();
145+
146+
// Stop the last "old" node
147+
cluster.stopNode(2, false);
148+
149+
// Ensure we talk only to the 2 live, upgraded nodes
150+
closeClients();
151+
testRestCluster = String.join(",", upgradedNodes);
152+
initClient();
153+
154+
assertBusy(() -> {
155+
var newNodesVersions = entityAsMap(client().performRequest(new Request("GET", "/_cluster/state")));
156+
assertThat(
157+
newNodesVersions,
158+
allOf(
159+
hasKey("nodes_versions"),
160+
transformedMatch(
161+
x -> fieldAsObjectList(x, "nodes_versions"),
162+
everyItem(transformedMatch(item -> fieldAsObject(item, "mappings_versions"), not(anEmptyMap())))
163+
)
164+
)
165+
);
166+
});
167+
}
168+
}

0 commit comments

Comments
 (0)