Guard file settings readiness on file settings support (#109500)

Consistency of file settings is an important invariant. However, when
upgrading from Elasticsearch versions before file settings existed,
cluster state will not yet have the file settings metadata. If the first
node upgraded is not the master node, new nodes will never become ready
while they wait for file settings metadata to exist.

This commit adds a node feature for file settings to guard waiting on
file settings for readiness. Although file settings has existed since
8.4, the feature is not a historical feature because historical features
are not applied to cluster state that readiness checks. In this case it
is not needed since upgrading from 8.4+ will already contain file
settings metadata.
This commit is contained in:
Ryan Ernst 2024-06-10 13:55:53 -07:00 committed by GitHub
parent e4c10d82bb
commit 0be3c741df
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 99 additions and 25 deletions

View file

@ -0,0 +1,5 @@
pr: 109500
summary: Guard file settings readiness on file settings support
area: Infra/Settings
type: bug
issues: []

View file

@ -430,7 +430,8 @@ module org.elasticsearch.server {
org.elasticsearch.indices.IndicesFeatures,
org.elasticsearch.action.admin.cluster.allocation.AllocationStatsFeatures,
org.elasticsearch.index.mapper.MapperFeatures,
org.elasticsearch.search.retriever.RetrieversFeatures;
org.elasticsearch.search.retriever.RetrieversFeatures,
org.elasticsearch.reservedstate.service.FileSettingsFeatures;
uses org.elasticsearch.plugins.internal.SettingsExtension;
uses RestExtension;

View file

@ -21,7 +21,9 @@ import org.elasticsearch.common.network.NetworkAddress;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.transport.BoundTransportAddress;
import org.elasticsearch.common.transport.TransportAddress;
import org.elasticsearch.core.SuppressForbidden;
import org.elasticsearch.env.Environment;
import org.elasticsearch.reservedstate.service.FileSettingsFeatures;
import org.elasticsearch.reservedstate.service.FileSettingsService;
import org.elasticsearch.shutdown.PluginShutdownService;
import org.elasticsearch.transport.BindTransportException;
@ -277,7 +279,22 @@ public class ReadinessService extends AbstractLifecycleComponent implements Clus
// protected to allow mock service to override
protected boolean areFileSettingsApplied(ClusterState clusterState) {
ReservedStateMetadata fileSettingsMetadata = clusterState.metadata().reservedStateMetadata().get(FileSettingsService.NAMESPACE);
return fileSettingsMetadata != null && fileSettingsMetadata.version().equals(ReservedStateMetadata.NO_VERSION) == false;
if (fileSettingsMetadata == null) {
// In order to block readiness on file settings being applied, we need to know that the master node has written an initial
// version, or a marker that file settings don't exist. When upgrading from a version that did not have file settings, the
// current master node may not be the first node upgraded. To be safe, we wait to consider file settings application for
// readiness until the whole cluster supports file settings. Note that this only applies when no reserved state metadata
// exists, so either we are starting up a current cluster (and the feature will be found) or we are upgrading from
// a version before file settings existed (before 8.4).
return supportsFileSettings(clusterState) == false;
} else {
return fileSettingsMetadata.version().equals(ReservedStateMetadata.NO_VERSION) == false;
}
}
@SuppressForbidden(reason = "need to check file settings support on exact cluster state")
private static boolean supportsFileSettings(ClusterState clusterState) {
return clusterState.clusterFeatures().clusterHasFeature(FileSettingsFeatures.FILE_SETTINGS_SUPPORTED);
}
private void setReady(boolean ready) {

View file

@ -0,0 +1,26 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
package org.elasticsearch.reservedstate.service;
import org.elasticsearch.features.FeatureSpecification;
import org.elasticsearch.features.NodeFeature;
import java.util.Set;
public class FileSettingsFeatures implements FeatureSpecification {
// Although file settings were supported starting in 8.4.0, this is really about whether file settings
// are used in readiness.
public static final NodeFeature FILE_SETTINGS_SUPPORTED = new NodeFeature("file_settings");
@Override
public Set<NodeFeature> getFeatures() {
return Set.of(FILE_SETTINGS_SUPPORTED);
}
}

View file

@ -15,3 +15,4 @@ org.elasticsearch.indices.IndicesFeatures
org.elasticsearch.action.admin.cluster.allocation.AllocationStatsFeatures
org.elasticsearch.index.mapper.MapperFeatures
org.elasticsearch.search.retriever.RetrieversFeatures
org.elasticsearch.reservedstate.service.FileSettingsFeatures

View file

@ -31,6 +31,7 @@ import org.elasticsearch.env.Environment;
import org.elasticsearch.http.HttpInfo;
import org.elasticsearch.http.HttpServerTransport;
import org.elasticsearch.http.HttpStats;
import org.elasticsearch.reservedstate.service.FileSettingsFeatures;
import org.elasticsearch.reservedstate.service.FileSettingsService;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.MockLog;
@ -46,6 +47,7 @@ import java.net.UnknownHostException;
import java.nio.channels.ServerSocketChannel;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import static org.elasticsearch.cluster.metadata.ReservedStateErrorMetadata.ErrorKind.TRANSIENT;
@ -57,6 +59,7 @@ public class ReadinessServiceTests extends ESTestCase implements ReadinessClient
private ThreadPool threadpool;
private Environment env;
private FakeHttpTransport httpTransport;
private static final Set<String> nodeFeatures = Set.of(FileSettingsFeatures.FILE_SETTINGS_SUPPORTED.id());
private static Metadata emptyReservedStateMetadata;
static {
@ -205,21 +208,8 @@ public class ReadinessServiceTests extends ESTestCase implements ReadinessClient
// initially the service isn't ready
assertFalse(readinessService.ready());
ClusterState emptyState = ClusterState.builder(new ClusterName("cluster"))
.nodes(
DiscoveryNodes.builder().add(DiscoveryNodeUtils.create("node2", new TransportAddress(TransportAddress.META_ADDRESS, 9201)))
)
.build();
ClusterState noFileSettingsState = ClusterState.builder(emptyState)
.nodes(
DiscoveryNodes.builder(emptyState.nodes())
.add(httpTransport.node)
.masterNodeId(httpTransport.node.getId())
.localNodeId(httpTransport.node.getId())
)
.build();
ClusterChangedEvent event = new ClusterChangedEvent("test", noFileSettingsState, emptyState);
ClusterState noFileSettingsState = noFileSettingsState();
ClusterChangedEvent event = new ClusterChangedEvent("test", noFileSettingsState, emptyState());
readinessService.clusterChanged(event);
// sending a cluster state with active master should not yet bring up the service, file settings still are not applied
@ -306,14 +296,7 @@ public class ReadinessServiceTests extends ESTestCase implements ReadinessClient
var fileSettingsState = new ReservedStateMetadata.Builder(FileSettingsService.NAMESPACE).version(21L)
.errorMetadata(new ReservedStateErrorMetadata(22L, TRANSIENT, List.of("dummy error")));
ClusterState state = ClusterState.builder(new ClusterName("cluster"))
.nodes(
DiscoveryNodes.builder()
.add(DiscoveryNodeUtils.create("node2", new TransportAddress(TransportAddress.META_ADDRESS, 9201)))
.add(httpTransport.node)
.masterNodeId(httpTransport.node.getId())
.localNodeId(httpTransport.node.getId())
)
ClusterState state = ClusterState.builder(noFileSettingsState())
.metadata(new Metadata.Builder().put(fileSettingsState.build()))
.build();
@ -324,4 +307,45 @@ public class ReadinessServiceTests extends ESTestCase implements ReadinessClient
readinessService.stop();
readinessService.close();
}
public void testFileSettingsMixedCluster() throws Exception {
readinessService.start();
// initially the service isn't ready because initial cluster state has not been applied yet
assertFalse(readinessService.ready());
ClusterState noFileSettingsState = ClusterState.builder(noFileSettingsState())
// the master node is upgraded to support file settings, but existing node2 is not
.nodeFeatures(Map.of(httpTransport.node.getId(), nodeFeatures))
.build();
ClusterChangedEvent event = new ClusterChangedEvent("test", noFileSettingsState, emptyState());
readinessService.clusterChanged(event);
// when upgrading from nodes before file settings exist, readiness should return true once a master is elected
assertTrue(readinessService.ready());
readinessService.stop();
readinessService.close();
}
private ClusterState emptyState() {
return ClusterState.builder(new ClusterName("cluster"))
.nodes(
DiscoveryNodes.builder().add(DiscoveryNodeUtils.create("node2", new TransportAddress(TransportAddress.META_ADDRESS, 9201)))
)
.build();
}
private ClusterState noFileSettingsState() {
ClusterState emptyState = emptyState();
return ClusterState.builder(emptyState)
.nodes(
DiscoveryNodes.builder(emptyState.nodes())
.add(httpTransport.node)
.masterNodeId(httpTransport.node.getId())
.localNodeId(httpTransport.node.getId())
)
.nodeFeatures(Map.of(httpTransport.node.getId(), nodeFeatures, "node2", nodeFeatures))
.build();
}
}