[8.x] Add a cluster listener to fix missing system index mappings after upgrade#115771
Conversation
…rsions-fixup-listener
|
Removing auto-backport as this includes a tv change. |
|
I think we should backport it to 8.16.1, given it fixes some erroneous behaviour users have seen. We can use a patch TV to do this change on 8.16.1. |
…rsions-fixup-listener
Will do that, thanks! |
server/src/main/java/org/elasticsearch/cluster/service/TransportVersionsFixupListener.java
Outdated
Show resolved
Hide resolved
...ng-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/SystemIndexMappingUpgradeIT.java
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| private static List<String> getUpgradedNodesAddresses() throws IOException { |
There was a problem hiding this comment.
There must be a helper method somewhere that does this already?
There was a problem hiding this comment.
Couldn't find one, but I'll look again
| public void clusterChanged(ClusterChangedEvent event) { | ||
| if (event.localNodeMaster() == false) return; // only if we're master | ||
|
|
||
| List<Stream<String>> queries = new ArrayList<>(); |
There was a problem hiding this comment.
nit - Maybe just have a Stream<String>, you can then add another stream using queries = Stream.concat(queries, ...)
There was a problem hiding this comment.
Or why not just have a Set<String> and you can do a forEach(addAll)?
thecoop
left a comment
There was a problem hiding this comment.
Principal looks good, just a few things to tweak
|
If this is being backported to 8.16, the backport PR should be in place too, and the backport transport version referenced in this PR |
Yes, definitely, I wanted to have this in the final form before backporting. |
Extracting the Transport protocol related changes from #115771 to make backport easier.
…rsions-fixup-listener
💔 Backport failedThe backport operation could not be completed due to the following error: You can use sqren/backport to manually backport by running |
Extracting the Transport protocol related changes from #115771 to make backport easier.
Extracting the Transport protocol related changes from elastic#115771 to make backport easier.
Extracting the Transport protocol related changes from elastic#115771 to make backport easier.
This PR modifies
TransportVersionsFixupListenerto include all of compatibility versions (not only TransportVersion) in the fixup.TransportVersionsFixupListenerspots 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