From 84a3aec285fa4748eb1f134e8cdb494410aa2426 Mon Sep 17 00:00:00 2001 From: James Baiera Date: Fri, 8 Jul 2022 14:41:30 -0400 Subject: [PATCH 1/8] Add a count of invocations since the last success on snapshot lifecycle policy metadata. --- .../slm/SnapshotLifecyclePolicyMetadata.java | 38 ++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java index d42d7f62d6a1c..3699ca7970284 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.core.slm; +import org.elasticsearch.Version; import org.elasticsearch.cluster.SimpleDiffable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -40,6 +41,7 @@ public class SnapshotLifecyclePolicyMetadata implements SimpleDiffable PARSER = new ConstructingObjectParser<>( @@ -66,6 +70,7 @@ public class SnapshotLifecyclePolicyMetadata implements SimpleDiffable getHeaders() { @@ -165,6 +178,10 @@ public SnapshotInvocationRecord getLastFailure() { return lastFailure; } + public Long getInvocationsSinceLastSuccess() { + return invocationsSinceLastSuccess; + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); @@ -178,13 +195,16 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (Objects.nonNull(lastFailure)) { builder.field(LAST_FAILURE.getPreferredName(), lastFailure); } + if (Objects.nonNull(invocationsSinceLastSuccess)) { + builder.field(INVOCATIONS_SINCE_LAST_SUCCESS.getPreferredName(), invocationsSinceLastSuccess); + } builder.endObject(); return builder; } @Override public int hashCode() { - return Objects.hash(policy, headers, version, modifiedDate, lastSuccess, lastFailure); + return Objects.hash(policy, headers, version, modifiedDate, lastSuccess, lastFailure, invocationsSinceLastSuccess); } @Override @@ -201,7 +221,8 @@ public boolean equals(Object obj) { && Objects.equals(version, other.version) && Objects.equals(modifiedDate, other.modifiedDate) && Objects.equals(lastSuccess, other.lastSuccess) - && Objects.equals(lastFailure, other.lastFailure); + && Objects.equals(lastFailure, other.lastFailure) + && Objects.equals(invocationsSinceLastSuccess, other.invocationsSinceLastSuccess); } @Override @@ -222,6 +243,7 @@ private Builder() {} private Long modifiedDate; private SnapshotInvocationRecord lastSuccessDate; private SnapshotInvocationRecord lastFailureDate; + private Long invocationsSinceLastSuccess; public Builder setPolicy(SnapshotLifecyclePolicy policy) { this.policy = policy; @@ -253,6 +275,11 @@ public Builder setLastFailure(SnapshotInvocationRecord lastFailure) { return this; } + public Builder setInvocationsSinceLastSuccess(Long invocationsSinceLastSuccess) { + this.invocationsSinceLastSuccess = invocationsSinceLastSuccess; + return this; + } + public SnapshotLifecyclePolicyMetadata build() { return new SnapshotLifecyclePolicyMetadata( Objects.requireNonNull(policy), @@ -260,7 +287,8 @@ public SnapshotLifecyclePolicyMetadata build() { version, Objects.requireNonNull(modifiedDate, "modifiedDate must be set"), lastSuccessDate, - lastFailureDate + lastFailureDate, + invocationsSinceLastSuccess ); } } From 8858efb86a9a00f94903e42c6ca729b6b0cd70a4 Mon Sep 17 00:00:00 2001 From: James Baiera Date: Fri, 8 Jul 2022 15:57:41 -0400 Subject: [PATCH 2/8] Add invocations since last success logic to lifecycle task --- .../org/elasticsearch/xpack/slm/SnapshotLifecycleTask.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTask.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTask.java index 1f7f5faf45d74..125b58f5861ae 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTask.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTask.java @@ -276,9 +276,13 @@ public ClusterState execute(ClusterState currentState) throws Exception { exception.map(SnapshotLifecycleTask::exceptionToString).orElse(null) ) ); + Long invocations = policyMetadata.getInvocationsSinceLastSuccess(); + invocations = invocations == null ? 0L : invocations; + newPolicyMetadata.setInvocationsSinceLastSuccess(invocations + 1); } else { stats.snapshotTaken(policyName); newPolicyMetadata.setLastSuccess(new SnapshotInvocationRecord(snapshotName, snapshotStartTime, snapshotFinishTime, null)); + newPolicyMetadata.setInvocationsSinceLastSuccess(0L); } snapLifecycles.put(policyName, newPolicyMetadata.build()); From 145bc348277e12d9b4e46ba076b2ff029ac9cb35 Mon Sep 17 00:00:00 2001 From: James Baiera Date: Fri, 8 Jul 2022 15:58:11 -0400 Subject: [PATCH 3/8] Assert invocationsSinceLastSuccess needs a success present. --- .../xpack/core/slm/SnapshotLifecyclePolicyMetadata.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java index 3699ca7970284..1445da0eeb3b6 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java @@ -281,6 +281,9 @@ public Builder setInvocationsSinceLastSuccess(Long invocationsSinceLastSuccess) } public SnapshotLifecyclePolicyMetadata build() { + assert lastSuccessDate != null || invocationsSinceLastSuccess == null + : "Cannot have invocations since last success with no recorded success date"; + return new SnapshotLifecyclePolicyMetadata( Objects.requireNonNull(policy), Optional.ofNullable(headers).orElse(new HashMap<>()), From be0e6f387bf9aa1ab215b9ac15d5788a9d51fdc5 Mon Sep 17 00:00:00 2001 From: James Baiera Date: Fri, 8 Jul 2022 15:58:32 -0400 Subject: [PATCH 4/8] Include new field in serialization tests. --- .../core/slm/SnapshotLifecyclePolicyMetadataTests.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadataTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadataTests.java index 59c703c5418c0..58ed29e30d79d 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadataTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadataTests.java @@ -84,11 +84,16 @@ public static SnapshotLifecyclePolicyMetadata createRandomPolicyMetadata(String if (randomBoolean()) { builder.setHeaders(randomHeaders()); } - if (randomBoolean()) { + boolean hasSuccess = randomBoolean(); + if (hasSuccess) { builder.setLastSuccess(randomSnapshotInvocationRecord()); + builder.setInvocationsSinceLastSuccess(0L); } if (randomBoolean()) { builder.setLastFailure(randomSnapshotInvocationRecord()); + if (hasSuccess) { + builder.setInvocationsSinceLastSuccess(randomLongBetween(1, Long.MAX_VALUE)); + } } return builder.build(); } From f93b85b7ace6e0e909d61934f5dc39c279753982 Mon Sep 17 00:00:00 2001 From: James Baiera Date: Fri, 8 Jul 2022 16:19:40 -0400 Subject: [PATCH 5/8] Update docs/changelog/88398.yaml --- docs/changelog/88398.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/88398.yaml diff --git a/docs/changelog/88398.yaml b/docs/changelog/88398.yaml new file mode 100644 index 0000000000000..e694b560e9407 --- /dev/null +++ b/docs/changelog/88398.yaml @@ -0,0 +1,5 @@ +pr: 88398 +summary: Track the count of failed invocations since last successful policy snapshot +area: ILM+SLM +type: enhancement +issues: [] From f72a75b7d7980501adfd8d74bd145acf1b37ca09 Mon Sep 17 00:00:00 2001 From: James Baiera Date: Mon, 11 Jul 2022 13:05:35 -0400 Subject: [PATCH 6/8] fix setting bug --- .../elasticsearch/xpack/slm/SnapshotLifecycleTask.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTask.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTask.java index 125b58f5861ae..54f9bed4cf59e 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTask.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTask.java @@ -276,9 +276,11 @@ public ClusterState execute(ClusterState currentState) throws Exception { exception.map(SnapshotLifecycleTask::exceptionToString).orElse(null) ) ); - Long invocations = policyMetadata.getInvocationsSinceLastSuccess(); - invocations = invocations == null ? 0L : invocations; - newPolicyMetadata.setInvocationsSinceLastSuccess(invocations + 1); + if (policyMetadata.getLastSuccess() != null) { + Long invocations = policyMetadata.getInvocationsSinceLastSuccess(); + invocations = invocations == null ? 1L : invocations + 1L; + newPolicyMetadata.setInvocationsSinceLastSuccess(invocations); + } } else { stats.snapshotTaken(policyName); newPolicyMetadata.setLastSuccess(new SnapshotInvocationRecord(snapshotName, snapshotStartTime, snapshotFinishTime, null)); From 0503839e2ba4872ea604d91643aac75c0fc26277 Mon Sep 17 00:00:00 2001 From: James Baiera Date: Mon, 11 Jul 2022 14:37:43 -0400 Subject: [PATCH 7/8] Use non nullable long value --- .../slm/SnapshotLifecyclePolicyMetadata.java | 22 +++++++------------ .../xpack/slm/SnapshotLifecycleTask.java | 6 +---- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java index 1445da0eeb3b6..08c4d94706554 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java @@ -53,8 +53,7 @@ public class SnapshotLifecyclePolicyMetadata implements SimpleDiffable PARSER = new ConstructingObjectParser<>( @@ -96,7 +95,7 @@ public static SnapshotLifecyclePolicyMetadata parse(XContentParser parser, Strin long modifiedDate, SnapshotInvocationRecord lastSuccess, SnapshotInvocationRecord lastFailure, - Long invocationsSinceLastSuccess + long invocationsSinceLastSuccess ) { this.policy = policy; this.headers = headers; @@ -117,7 +116,7 @@ public static SnapshotLifecyclePolicyMetadata parse(XContentParser parser, Strin this.modifiedDate = in.readVLong(); this.lastSuccess = in.readOptionalWriteable(SnapshotInvocationRecord::new); this.lastFailure = in.readOptionalWriteable(SnapshotInvocationRecord::new); - this.invocationsSinceLastSuccess = in.getVersion().onOrAfter(Version.V_8_4_0) ? in.readOptionalVLong() : null; + this.invocationsSinceLastSuccess = in.getVersion().onOrAfter(Version.V_8_4_0) ? in.readVLong() : 0L; } @Override @@ -129,7 +128,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalWriteable(this.lastSuccess); out.writeOptionalWriteable(this.lastFailure); if (out.getVersion().onOrAfter(Version.V_8_4_0)) { - out.writeOptionalVLong(this.invocationsSinceLastSuccess); + out.writeVLong(this.invocationsSinceLastSuccess); } } @@ -178,7 +177,7 @@ public SnapshotInvocationRecord getLastFailure() { return lastFailure; } - public Long getInvocationsSinceLastSuccess() { + public long getInvocationsSinceLastSuccess() { return invocationsSinceLastSuccess; } @@ -195,9 +194,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (Objects.nonNull(lastFailure)) { builder.field(LAST_FAILURE.getPreferredName(), lastFailure); } - if (Objects.nonNull(invocationsSinceLastSuccess)) { - builder.field(INVOCATIONS_SINCE_LAST_SUCCESS.getPreferredName(), invocationsSinceLastSuccess); - } + builder.field(INVOCATIONS_SINCE_LAST_SUCCESS.getPreferredName(), invocationsSinceLastSuccess); builder.endObject(); return builder; } @@ -243,7 +240,7 @@ private Builder() {} private Long modifiedDate; private SnapshotInvocationRecord lastSuccessDate; private SnapshotInvocationRecord lastFailureDate; - private Long invocationsSinceLastSuccess; + private long invocationsSinceLastSuccess = 0L; public Builder setPolicy(SnapshotLifecyclePolicy policy) { this.policy = policy; @@ -275,15 +272,12 @@ public Builder setLastFailure(SnapshotInvocationRecord lastFailure) { return this; } - public Builder setInvocationsSinceLastSuccess(Long invocationsSinceLastSuccess) { + public Builder setInvocationsSinceLastSuccess(long invocationsSinceLastSuccess) { this.invocationsSinceLastSuccess = invocationsSinceLastSuccess; return this; } public SnapshotLifecyclePolicyMetadata build() { - assert lastSuccessDate != null || invocationsSinceLastSuccess == null - : "Cannot have invocations since last success with no recorded success date"; - return new SnapshotLifecyclePolicyMetadata( Objects.requireNonNull(policy), Optional.ofNullable(headers).orElse(new HashMap<>()), diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTask.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTask.java index 54f9bed4cf59e..bd06e35a5b217 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTask.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleTask.java @@ -276,11 +276,7 @@ public ClusterState execute(ClusterState currentState) throws Exception { exception.map(SnapshotLifecycleTask::exceptionToString).orElse(null) ) ); - if (policyMetadata.getLastSuccess() != null) { - Long invocations = policyMetadata.getInvocationsSinceLastSuccess(); - invocations = invocations == null ? 1L : invocations + 1L; - newPolicyMetadata.setInvocationsSinceLastSuccess(invocations); - } + newPolicyMetadata.setInvocationsSinceLastSuccess(policyMetadata.getInvocationsSinceLastSuccess() + 1L); } else { stats.snapshotTaken(policyName); newPolicyMetadata.setLastSuccess(new SnapshotInvocationRecord(snapshotName, snapshotStartTime, snapshotFinishTime, null)); From f8975bbf05ebd0b6fcab135315702d4e582991e8 Mon Sep 17 00:00:00 2001 From: James Baiera Date: Mon, 11 Jul 2022 16:20:52 -0400 Subject: [PATCH 8/8] Fix xcontent nullity logic --- .../xpack/core/slm/SnapshotLifecyclePolicyMetadata.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java index 08c4d94706554..59c65da8cfea0 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadata.java @@ -69,7 +69,7 @@ public class SnapshotLifecyclePolicyMetadata implements SimpleDiffable