Skip to content

Commit 0b4620e

Browse files
rjernstmosche
andauthored
Consider node ready on any file settings applied (#107738)
Readiness is blocked until file based settings having been applied. Yet in the case that an error occurs in a subsequent file based settings update, readiness will stop working. This was unintentional; if file based settings have already been applied once, subsequent failures to apply file based settings should not affect the running system. This commit adjusts the readiness check to consider any file based settings having been applied as allowing the node to be ready. In a follow up (#107744) we are going to prevent readiness in case initial file settings of a new cluster cannot be applied. --------- Co-authored-by: Moritz Mack <[email protected]>
1 parent 014fea8 commit 0b4620e

File tree

3 files changed

+35
-1
lines changed

3 files changed

+35
-1
lines changed

server/src/internalClusterTest/java/org/elasticsearch/readiness/ReadinessClusterIT.java

+1
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ private void writeFileSettings(String json) throws Exception {
247247
logger.info("--> New file settings: [{}]", Strings.format(json, version));
248248
}
249249

250+
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/107744")
250251
public void testNotReadyOnBadFileSettings() throws Exception {
251252
internalCluster().setBootstrapMasterNodeIndex(0);
252253
logger.info("--> start data node / non master node");

server/src/main/java/org/elasticsearch/readiness/ReadinessService.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ public void clusterChanged(ClusterChangedEvent event) {
254254
// protected to allow mock service to override
255255
protected boolean areFileSettingsApplied(ClusterState clusterState) {
256256
ReservedStateMetadata fileSettingsMetadata = clusterState.metadata().reservedStateMetadata().get(FileSettingsService.NAMESPACE);
257-
return fileSettingsMetadata != null && fileSettingsMetadata.errorMetadata() == null;
257+
return fileSettingsMetadata != null;
258258
}
259259

260260
private void setReady(boolean ready) {

server/src/test/java/org/elasticsearch/readiness/ReadinessServiceTests.java

+33
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.elasticsearch.cluster.ClusterState;
1515
import org.elasticsearch.cluster.metadata.Metadata;
1616
import org.elasticsearch.cluster.metadata.NodesShutdownMetadata;
17+
import org.elasticsearch.cluster.metadata.ReservedStateErrorMetadata;
1718
import org.elasticsearch.cluster.metadata.ReservedStateMetadata;
1819
import org.elasticsearch.cluster.metadata.SingleNodeShutdownMetadata;
1920
import org.elasticsearch.cluster.node.DiscoveryNode;
@@ -44,8 +45,11 @@
4445
import java.net.UnknownHostException;
4546
import java.nio.channels.ServerSocketChannel;
4647
import java.util.Collections;
48+
import java.util.List;
4749
import java.util.Set;
4850

51+
import static org.elasticsearch.cluster.metadata.ReservedStateErrorMetadata.ErrorKind.TRANSIENT;
52+
4953
public class ReadinessServiceTests extends ESTestCase implements ReadinessClientProbe {
5054
private ClusterService clusterService;
5155
private ReadinessService readinessService;
@@ -291,4 +295,33 @@ public void testStatusChange() throws Exception {
291295
readinessService.stop();
292296
readinessService.close();
293297
}
298+
299+
public void testFileSettingsUpdateError() throws Exception {
300+
// ensure an update to file settings that results in an error state doesn't cause readiness to fail
301+
// since the current file settings already applied cleanly
302+
readinessService.start();
303+
304+
// initially the service isn't ready because initial cluster state has not been applied yet
305+
assertFalse(readinessService.ready());
306+
307+
var fileSettingsState = new ReservedStateMetadata.Builder(FileSettingsService.NAMESPACE).version(21L)
308+
.errorMetadata(new ReservedStateErrorMetadata(22L, TRANSIENT, List.of("dummy error")));
309+
ClusterState state = ClusterState.builder(new ClusterName("cluster"))
310+
.nodes(
311+
DiscoveryNodes.builder()
312+
.add(DiscoveryNodeUtils.create("node2", new TransportAddress(TransportAddress.META_ADDRESS, 9201)))
313+
.add(httpTransport.node)
314+
.masterNodeId(httpTransport.node.getId())
315+
.localNodeId(httpTransport.node.getId())
316+
)
317+
.metadata(new Metadata.Builder().put(fileSettingsState.build()))
318+
.build();
319+
320+
ClusterChangedEvent event = new ClusterChangedEvent("test", state, ClusterState.EMPTY_STATE);
321+
readinessService.clusterChanged(event);
322+
assertTrue(readinessService.ready());
323+
324+
readinessService.stop();
325+
readinessService.close();
326+
}
294327
}

0 commit comments

Comments
 (0)