From 658c01401696c4685e8f7acf18865e2061fa534e Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 26 Apr 2024 08:03:30 -0700 Subject: [PATCH] Block readiness on bad initial file settings (#107775) If file settings have an update that fails, existing applied file settings continue to work. But if the initial file settings fail to process, readiness should be blocked. This commit adjusts readiness to look for this special initialization case. relates #107738 --- .../java/org/elasticsearch/readiness/ReadinessClusterIT.java | 1 - .../elasticsearch/cluster/metadata/ReservedStateMetadata.java | 4 +++- .../java/org/elasticsearch/readiness/ReadinessService.java | 2 +- .../cluster/metadata/ReservedStateMetadataTests.java | 3 ++- .../cluster/metadata/ToAndFromJsonMetadataTests.java | 4 ++-- 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/readiness/ReadinessClusterIT.java b/server/src/internalClusterTest/java/org/elasticsearch/readiness/ReadinessClusterIT.java index 2ecdd06f379d2..1f8d55516d508 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/readiness/ReadinessClusterIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/readiness/ReadinessClusterIT.java @@ -251,7 +251,6 @@ private void writeFileSettings(String json) throws Exception { logger.info("--> New file settings: [{}]", Strings.format(json, version)); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/107744") public void testNotReadyOnBadFileSettings() throws Exception { internalCluster().setBootstrapMasterNodeIndex(0); logger.info("--> start data node / non master node"); diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/ReservedStateMetadata.java b/server/src/main/java/org/elasticsearch/cluster/metadata/ReservedStateMetadata.java index dd5ca03cf759a..ec8200bf2d701 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/ReservedStateMetadata.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/ReservedStateMetadata.java @@ -46,6 +46,8 @@ public record ReservedStateMetadata( ReservedStateErrorMetadata errorMetadata ) implements SimpleDiffable, ToXContentFragment { + public static final Long NO_VERSION = Long.MIN_VALUE; // use min long as sentinel for uninitialized version + private static final ParseField VERSION = new ParseField("version"); private static final ParseField HANDLERS = new ParseField("handlers"); private static final ParseField ERRORS_METADATA = new ParseField("errors"); @@ -209,7 +211,7 @@ public static class Builder { */ public Builder(String namespace) { this.namespace = namespace; - this.version = -1L; + this.version = NO_VERSION; this.handlers = new HashMap<>(); this.errorMetadata = null; } diff --git a/server/src/main/java/org/elasticsearch/readiness/ReadinessService.java b/server/src/main/java/org/elasticsearch/readiness/ReadinessService.java index 1cac133106403..61425250c19b4 100644 --- a/server/src/main/java/org/elasticsearch/readiness/ReadinessService.java +++ b/server/src/main/java/org/elasticsearch/readiness/ReadinessService.java @@ -254,7 +254,7 @@ public void clusterChanged(ClusterChangedEvent event) { // protected to allow mock service to override protected boolean areFileSettingsApplied(ClusterState clusterState) { ReservedStateMetadata fileSettingsMetadata = clusterState.metadata().reservedStateMetadata().get(FileSettingsService.NAMESPACE); - return fileSettingsMetadata != null; + return fileSettingsMetadata != null && fileSettingsMetadata.version().equals(ReservedStateMetadata.NO_VERSION) == false; } private void setReady(boolean ready) { diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/ReservedStateMetadataTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/ReservedStateMetadataTests.java index 46be49ad7111f..5086813cc5c13 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/ReservedStateMetadataTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/ReservedStateMetadataTests.java @@ -20,6 +20,7 @@ import java.util.Collections; import java.util.List; +import static org.elasticsearch.cluster.metadata.ReservedStateMetadata.NO_VERSION; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; @@ -78,7 +79,7 @@ public void testXContent() throws IOException { public void testReservedStateVersionWithError() { final ReservedStateMetadata meta = createRandom(false, true); - assertEquals(-1L, meta.version().longValue()); + assertEquals(NO_VERSION.longValue(), meta.version().longValue()); } private static ReservedStateMetadata createRandom(boolean addHandlers, boolean addErrors) { diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/ToAndFromJsonMetadataTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/ToAndFromJsonMetadataTests.java index aa9d0b9368fa6..3a522f3f5c06c 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/ToAndFromJsonMetadataTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/ToAndFromJsonMetadataTests.java @@ -776,7 +776,7 @@ public void testToXContentAPIReservedMetadata() throws IOException { }, "reserved_state" : { "namespace_one" : { - "version" : -1, + "version" : -9223372036854775808, "handlers" : { "one" : { "keys" : [ @@ -801,7 +801,7 @@ public void testToXContentAPIReservedMetadata() throws IOException { } }, "namespace_two" : { - "version" : -1, + "version" : -9223372036854775808, "handlers" : { "three" : { "keys" : [