From 24c435cec61f6f2025e2c0a04575b1ae7f298ee0 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Tue, 26 Mar 2019 17:14:19 -0600 Subject: [PATCH 1/5] Record most recent snapshot policy success/failure Keeping a record of the results of the successes and failures will aid troubleshooting of policies and make users more confident that their snapshots are being taken as expected. This is the first step toward writing history in a more permanent fashion. --- .../SnapshotLifecyclePolicyMetadata.java | 146 +++++++++++++++++- .../action/GetSnapshotLifecycleAction.java | 49 +++++- .../SnapshotLifecycleIT.java | 122 +++++++++++---- .../SnapshotLifecycleTask.java | 96 +++++++++++- .../TransportGetSnapshotLifecycleAction.java | 4 +- .../TransportPutSnapshotLifecycleAction.java | 15 +- .../SnapshotLifecycleServiceTests.java | 50 ++++-- .../SnapshotLifecycleTaskTests.java | 10 +- 8 files changed, 423 insertions(+), 69 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadata.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadata.java index 1c2e1956707f2..1fc5366cd235e 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadata.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadata.java @@ -8,6 +8,7 @@ import org.elasticsearch.cluster.AbstractDiffable; import org.elasticsearch.cluster.Diffable; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -37,18 +38,29 @@ public class SnapshotLifecyclePolicyMetadata extends AbstractDiffable headers; private final long version; private final long modifiedDate; + @Nullable + private final Long lastSuccessDate; + @Nullable + private final Long lastFailureDate; + @Nullable + private final String lastFailureInfo; @SuppressWarnings("unchecked") public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("snapshot_policy_metadata", a -> { SnapshotLifecyclePolicy policy = (SnapshotLifecyclePolicy) a[0]; - return new SnapshotLifecyclePolicyMetadata(policy, (Map) a[1], (long) a[2], (long) a[3]); + return new Builder().setPolicy(policy).setHeaders((Map) a[1]).setVersion((long) a[2]).setModifiedDate((long) a[3]).setLastSuccessDate((Long) a[4]).setLastFailureDate((Long) a[5]).setLastFailureInfo((String) a[6]).build(); }); static { @@ -57,17 +69,26 @@ public class SnapshotLifecyclePolicyMetadata extends AbstractDiffable headers, long version, long modifiedDate) { - this.policy = policy; - this.headers = headers; + SnapshotLifecyclePolicyMetadata(SnapshotLifecyclePolicy policy, Map headers, long version, long modifiedDate, + Long lastSuccessDate, Long lastFailureDate, String lastFailureInfo) { + this.policy = Objects.requireNonNull(policy); + this.headers = Objects.requireNonNull(headers); this.version = version; this.modifiedDate = modifiedDate; + this.lastSuccessDate = lastSuccessDate; + this.lastFailureDate = lastFailureDate; + this.lastFailureInfo = lastFailureInfo; } @SuppressWarnings("unchecked") @@ -76,6 +97,9 @@ public SnapshotLifecyclePolicyMetadata(SnapshotLifecyclePolicy policy, Map) in.readGenericValue(); this.version = in.readVLong(); this.modifiedDate = in.readVLong(); + this.lastSuccessDate = in.readOptionalLong(); + this.lastFailureDate = in.readOptionalLong(); + this.lastFailureInfo = in.readOptionalString(); } @Override @@ -84,6 +108,27 @@ public void writeTo(StreamOutput out) throws IOException { out.writeGenericValue(this.headers); out.writeVLong(this.version); out.writeVLong(this.modifiedDate); + out.writeOptionalLong(this.lastSuccessDate); + out.writeOptionalLong(this.lastFailureDate); + out.writeOptionalString(this.lastFailureInfo); + } + + public static Builder builder() { + return new Builder(); + } + + public static Builder builder(SnapshotLifecyclePolicyMetadata metadata) { + if (metadata == null) { + return builder(); + } + return new Builder() + .setHeaders(metadata.getHeaders()) + .setPolicy(metadata.getPolicy()) + .setVersion(metadata.getVersion()) + .setModifiedDate(metadata.getModifiedDate()) + .setLastSuccessDate(metadata.getLastSuccessDate()) + .setLastFailureDate(metadata.getLastFailureDate()) + .setLastFailureInfo(metadata.getLastFailureInfo()); } public Map getHeaders() { @@ -106,9 +151,35 @@ public long getModifiedDate() { return modifiedDate; } + private String dateToDateString(Long date) { + if (Objects.isNull(date)) { + return null; + } + ZonedDateTime dateTime = ZonedDateTime.ofInstant(Instant.ofEpochMilli(date), ZoneOffset.UTC); + return dateTime.toString(); + } + public String getModifiedDateString() { - ZonedDateTime modifiedDateTime = ZonedDateTime.ofInstant(Instant.ofEpochMilli(modifiedDate), ZoneOffset.UTC); - return modifiedDateTime.toString(); + return dateToDateString(modifiedDate); + } + + public Long getLastSuccessDate() { + return lastSuccessDate; + } + + public String getLastSuccessDateString() { + return dateToDateString(lastSuccessDate); + } + + public Long getLastFailureDate() { + return lastFailureDate; + } + public String getLastFailureDateString() { + return dateToDateString(lastFailureDate); + } + + public String getLastFailureInfo() { + return lastFailureInfo; } @Override @@ -119,6 +190,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field(VERSION.getPreferredName(), version); builder.field(MODIFIED_DATE.getPreferredName(), modifiedDate); builder.field(MODIFIED_DATE_STRING.getPreferredName(), getModifiedDateString()); + builder.field(LAST_SUCCESS_DATE.getPreferredName(), lastSuccessDate); + builder.field(LAST_SUCCESS_DATE_STRING.getPreferredName(), getLastSuccessDateString()); + builder.field(LAST_FAILURE_DATE.getPreferredName(), lastFailureDate); + builder.field(LAST_FAILURE_DATE_STRING.getPreferredName(), getLastFailureDateString()); + builder.field(LAST_FAILURE_INFO.getPreferredName(), lastFailureInfo); builder.endObject(); return builder; } @@ -140,7 +216,10 @@ public boolean equals(Object obj) { return Objects.equals(policy, other.policy) && Objects.equals(headers, other.headers) && Objects.equals(version, other.version) && - Objects.equals(modifiedDate, other.modifiedDate); + Objects.equals(modifiedDate, other.modifiedDate) && + Objects.equals(lastSuccessDate, other.lastSuccessDate) && + Objects.equals(lastFailureDate, other.lastFailureDate) && + Objects.equals(lastFailureInfo, other.lastFailureInfo); } @Override @@ -150,4 +229,57 @@ public String toString() { // should not emit them in case it accidentally gets logged. return super.toString(); } + + public static class Builder { + + private Builder() {} + + private SnapshotLifecyclePolicy policy; + private Map headers; + private long version = 1L; + private Long modifiedDate; + private Long lastSuccessDate; + private Long lastFailureDate; + private String lastFailureInfo; + + public Builder setPolicy(SnapshotLifecyclePolicy policy) { + this.policy = policy; + return this; + } + + public Builder setHeaders(Map headers) { + this.headers = headers; + return this; + } + + public Builder setVersion(long version) { + this.version = version; + return this; + } + + public Builder setModifiedDate(long modifiedDate) { + this.modifiedDate = modifiedDate; + return this; + } + + public Builder setLastSuccessDate(Long lastSuccessDate) { + this.lastSuccessDate = lastSuccessDate; + return this; + } + + public Builder setLastFailureDate(Long lastFailureDate) { + this.lastFailureDate = lastFailureDate; + return this; + } + + public Builder setLastFailureInfo(String lastFailureInfo) { + this.lastFailureInfo = lastFailureInfo; + return this; + } + + public SnapshotLifecyclePolicyMetadata build() { + Objects.requireNonNull(modifiedDate, "modifiedDate must be set"); + return new SnapshotLifecyclePolicyMetadata(policy, headers, version, modifiedDate, lastSuccessDate, lastFailureDate, lastFailureInfo); + } + } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/action/GetSnapshotLifecycleAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/action/GetSnapshotLifecycleAction.java index 5606bf837e2c4..f13ed6936778b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/action/GetSnapshotLifecycleAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/action/GetSnapshotLifecycleAction.java @@ -10,6 +10,7 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.support.master.AcknowledgedRequest; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -123,6 +124,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeList(lifecycles); + } + @Override public int hashCode() { return Objects.hash(lifecycles); @@ -146,22 +152,34 @@ public boolean equals(Object obj) { * {@link SnapshotLifecyclePolicyMetadata}, however, it elides the headers to ensure that they * are not leaked to the user since they may contain sensitive information. */ - public static class SnapshotLifecyclePolicyItem implements ToXContentFragment { + public static class SnapshotLifecyclePolicyItem implements ToXContentFragment, Writeable { private final SnapshotLifecyclePolicy policy; private final long version; private final long modifiedDate; - - public SnapshotLifecyclePolicyItem(SnapshotLifecyclePolicy policy, long version, long modifiedDate) { - this.policy = policy; - this.version = version; - this.modifiedDate = modifiedDate; + @Nullable + private final Long lastSuccessDate; + @Nullable + private final Long lastFailureDate; + @Nullable + private final String lastFailureInfo; + + public SnapshotLifecyclePolicyItem(SnapshotLifecyclePolicyMetadata policyMetadata) { + this.policy = policyMetadata.getPolicy(); + this.version = policyMetadata.getVersion(); + this.modifiedDate = policyMetadata.getModifiedDate(); + this.lastSuccessDate = policyMetadata.getLastSuccessDate(); + this.lastFailureDate = policyMetadata.getLastFailureDate(); + this.lastFailureInfo = policyMetadata.getLastFailureInfo(); } public SnapshotLifecyclePolicyItem(StreamInput in) throws IOException { this.policy = new SnapshotLifecyclePolicy(in); this.version = in.readVLong(); this.modifiedDate = in.readVLong(); + this.lastSuccessDate = in.readOptionalLong(); + this.lastFailureDate = in.readOptionalLong(); + this.lastFailureInfo = in.readOptionalString(); } public SnapshotLifecyclePolicy getPolicy() { @@ -176,6 +194,16 @@ public long getModifiedDate() { return modifiedDate; } + @Override + public void writeTo(StreamOutput out) throws IOException { + policy.writeTo(out); + out.writeVLong(version); + out.writeVLong(modifiedDate); + out.writeOptionalLong(lastSuccessDate); + out.writeOptionalLong(lastFailureDate); + out.writeOptionalString(lastFailureInfo); + } + @Override public int hashCode() { return Objects.hash(policy, version, modifiedDate); @@ -201,6 +229,15 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field("version", version); builder.field("modified_date", modifiedDate); builder.field("policy", policy); + if (lastSuccessDate != null) { + builder.field("last_success_date", lastSuccessDate); + } + if (lastFailureDate != null) { + builder.field("last_failure_date", lastFailureDate); + } + if (lastFailureInfo != null) { + builder.field("last_failure_info", lastFailureInfo); + } builder.endObject(); return builder; } diff --git a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java index e08b4cfdc41b5..b202b64ebffbc 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java @@ -28,6 +28,7 @@ import java.util.Map; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.startsWith; @@ -36,52 +37,49 @@ public class SnapshotLifecycleIT extends ESRestTestCase { @SuppressWarnings("unchecked") public void testFullPolicySnapshot() throws Exception { - final String IDX = "test"; + final String indexName = "test"; + final String policyName = "test-policy"; + final String repoId = "my-repo"; int docCount = randomIntBetween(10, 50); List indexReqs = new ArrayList<>(); for (int i = 0; i < docCount; i++) { - index(client(), IDX, "" + i, "foo", "bar"); + index(client(), indexName, "" + i, "foo", "bar"); } // Create a snapshot repo - Request request = new Request("PUT", "/_snapshot/my-repo"); - request.setJsonEntity(Strings - .toString(JsonXContent.contentBuilder() - .startObject() - .field("type", "fs") - .startObject("settings") - .field("compress", randomBoolean()) - .field("location", System.getProperty("tests.path.repo")) - .field("max_snapshot_bytes_per_sec", "256b") - .endObject() - .endObject())); - assertOK(client().performRequest(request)); + inializeRepo(repoId); - Map snapConfig = new HashMap<>(); - snapConfig.put("indices", Collections.singletonList(IDX)); - SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy("test-policy", "snap", "*/1 * * * * ?", "my-repo", snapConfig); - - Request putLifecycle = new Request("PUT", "/_ilm/snapshot/test-policy"); - XContentBuilder lifecycleBuilder = JsonXContent.contentBuilder(); - policy.toXContent(lifecycleBuilder, ToXContent.EMPTY_PARAMS); - putLifecycle.setJsonEntity(Strings.toString(lifecycleBuilder)); - assertOK(client().performRequest(putLifecycle)); + createSnapshotPolicy(policyName, "snap", "*/1 * * * * ?", repoId, indexName); // Check that the snapshot was actually taken assertBusy(() -> { - Response response = client().performRequest(new Request("GET", "/_snapshot/my-repo/_all")); - Map responseMap; + Response response = client().performRequest(new Request("GET", "/_snapshot/" + repoId + "/_all")); + Map snapshotResponseMap; try (InputStream is = response.getEntity().getContent()) { - responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); + snapshotResponseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); } - assertThat(responseMap.size(), greaterThan(0)); - assertThat(((List>) responseMap.get("snapshots")).size(), greaterThan(0)); - Map snapResponse = ((List>) responseMap.get("snapshots")).get(0); + assertThat(snapshotResponseMap.size(), greaterThan(0)); + assertThat(((List>) snapshotResponseMap.get("snapshots")).size(), greaterThan(0)); + Map snapResponse = ((List>) snapshotResponseMap.get("snapshots")).get(0); assertThat(snapResponse.get("snapshot").toString(), startsWith("snap-")); - assertThat((List)snapResponse.get("indices"), equalTo(Collections.singletonList(IDX))); + assertThat((List)snapResponse.get("indices"), equalTo(Collections.singletonList(indexName))); + + // Check that the last success date was written to the cluster state + Request getReq = new Request("GET", "/_ilm/snapshot/" + policyName); + Response policyMetadata = client().performRequest(getReq); + Map policyResponseMap; + try (InputStream is = policyMetadata.getEntity().getContent()) { + policyResponseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); + } + Map policyMetadataMap = (Map) policyResponseMap.get(policyName); + Long lastSuccess = (Long) policyMetadataMap.get("last_success_date"); + Long modifiedDate = (Long) policyMetadataMap.get("modified_date"); + assertNotNull(lastSuccess); + assertNotNull(modifiedDate); + assertThat(lastSuccess, greaterThan(modifiedDate)); }); - Request delReq = new Request("DELETE", "/_ilm/snapshot/test-policy"); + Request delReq = new Request("DELETE", "/_ilm/snapshot/" + policyName); assertOK(client().performRequest(delReq)); // It's possible there could have been a snapshot in progress when the @@ -91,6 +89,68 @@ public void testFullPolicySnapshot() throws Exception { }); } + public void testPolicyFailure() throws Exception { + final String indexName = "test"; + final String policyName = "test-policy"; + int docCount = randomIntBetween(10, 50); + List indexReqs = new ArrayList<>(); + for (int i = 0; i < docCount; i++) { + index(client(), indexName, "" + i, "foo", "bar"); + } + + // Create a policy with a repo that doesn't exist + createSnapshotPolicy(policyName, "snap", "*/1 * * * * ?", "bad-repo", indexName); + + assertBusy(() -> { + // Check that the failure is written to the cluster state + Request getReq = new Request("GET", "/_ilm/snapshot/" + policyName); + Response policyMetadata = client().performRequest(getReq); + try (InputStream is = policyMetadata.getEntity().getContent()) { + Map responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); + Map policyMetadataMap = (Map) responseMap.get(policyName); + Long lastFailure = (Long) policyMetadataMap.get("last_failure_date"); + String lastFailureInfo = (String) policyMetadataMap.get("last_failure_info"); + Long modifiedDate = (Long) policyMetadataMap.get("modified_date"); + assertNotNull(lastFailure); + assertNotNull(modifiedDate); + assertThat(lastFailure, greaterThan(modifiedDate)); + + assertNotNull(lastFailureInfo); + assertThat(lastFailureInfo, containsString("[bad-repo] missing")); + } + }); + + Request delReq = new Request("DELETE", "/_ilm/snapshot/" + policyName); + assertOK(client().performRequest(delReq)); + } + + private void createSnapshotPolicy(String policyName, String snapshotNamePattern, String schedule, String repoId, String indexPattern) throws IOException { + Map snapConfig = new HashMap<>(); + snapConfig.put("indices", Collections.singletonList(indexPattern)); + SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy(policyName, snapshotNamePattern, schedule, repoId, snapConfig); + + Request putLifecycle = new Request("PUT", "/_ilm/snapshot/" + policyName); + XContentBuilder lifecycleBuilder = JsonXContent.contentBuilder(); + policy.toXContent(lifecycleBuilder, ToXContent.EMPTY_PARAMS); + putLifecycle.setJsonEntity(Strings.toString(lifecycleBuilder)); + assertOK(client().performRequest(putLifecycle)); + } + + private void inializeRepo(String repoName) throws IOException { + Request request = new Request("PUT", "/_snapshot/" + repoName); + request.setJsonEntity(Strings + .toString(JsonXContent.contentBuilder() + .startObject() + .field("type", "fs") + .startObject("settings") + .field("compress", randomBoolean()) + .field("location", System.getProperty("tests.path.repo")) + .field("max_snapshot_bytes_per_sec", "256b") + .endObject() + .endObject())); + assertOK(client().performRequest(request)); + } + private static void index(RestClient client, String index, String id, Object... fields) throws IOException { XContentBuilder document = jsonBuilder().startObject(); for (int i = 0; i < fields.length; i += 2) { diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java index 78e143a7e60bb..e7a2e899bd44a 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java @@ -8,21 +8,35 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateUpdateTask; +import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.xpack.core.ClientHelper; import org.elasticsearch.xpack.core.scheduler.SchedulerEngine; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecycleMetadata; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicyMetadata; import org.elasticsearch.xpack.indexlifecycle.LifecyclePolicySecurityClient; +import java.io.IOException; +import java.time.Instant; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import java.util.Optional; +import static org.elasticsearch.ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE; + public class SnapshotLifecycleTask implements SchedulerEngine.Listener { private static Logger logger = LogManager.getLogger(SnapshotLifecycleTask.class); @@ -48,16 +62,18 @@ public void triggered(SchedulerEngine.Event event) { clientWithHeaders.admin().cluster().createSnapshot(request, new ActionListener() { @Override public void onResponse(CreateSnapshotResponse createSnapshotResponse) { - // TODO: persist this information in cluster state somewhere logger.info("snapshot response for [{}]: {}", policyMetadata.getPolicy().getId(), Strings.toString(createSnapshotResponse)); + clusterService.submitStateUpdateTask("slm-record-success", + WriteJobStatus.success(policyMetadata.getPolicy().getId(), Instant.now().toEpochMilli())); } @Override public void onFailure(Exception e) { - // TODO: persist the failure information in cluster state somewhere logger.error("failed to issue create snapshot request for snapshot lifecycle policy " + policyMetadata.getPolicy().getId(), e); + clusterService.submitStateUpdateTask("slm-record-failure", + WriteJobStatus.failure(policyMetadata.getPolicy().getId(), Instant.now().toEpochMilli(), e)); } }); return true; @@ -78,4 +94,80 @@ static Optional getSnapPolicyMetadata(final Str .filter(policyMeta -> jobId.equals(SnapshotLifecycleService.getJobId(policyMeta))) .findFirst()); } + + private static class WriteJobStatus extends ClusterStateUpdateTask { + private static final ToXContent.Params STACKTRACE_PARAMS = + new ToXContent.MapParams(Collections.singletonMap(REST_EXCEPTION_SKIP_STACK_TRACE, "false")); + + private final String policyName; + private final long timestamp; + private final Optional exception; + + private WriteJobStatus(String policyName, long timestamp, Optional exception) { + this.policyName = policyName; + this.exception = exception; + this.timestamp = timestamp; + } + + static WriteJobStatus success(String policyId, long timestamp) { + return new WriteJobStatus(policyId, timestamp, Optional.empty()); + } + + static WriteJobStatus failure(String policyId, long timestamp, Exception exception) { + return new WriteJobStatus(policyId, timestamp, Optional.of(exception)); + } + + private String exceptionToString() throws IOException { + if (exception.isPresent()) { + XContentBuilder causeXContentBuilder = JsonXContent.contentBuilder(); + causeXContentBuilder.startObject(); + ElasticsearchException.generateThrowableXContent(causeXContentBuilder, STACKTRACE_PARAMS, exception.get()); + causeXContentBuilder.endObject(); + return BytesReference.bytes(causeXContentBuilder).utf8ToString(); + } + return null; + } + + @Override + public ClusterState execute(ClusterState currentState) throws Exception { + SnapshotLifecycleMetadata snapMeta = currentState.metaData().custom(SnapshotLifecycleMetadata.TYPE); + + assert snapMeta != null : "this should never be called while the snapshot lifecycle cluster metadata is null"; + if (snapMeta == null) { + logger.error("failed to record snapshot [{}] for snapshot policy [{}]: snapshot lifecycle metadata is null", + exception.isPresent() ? "failure" : "success", policyName); + return currentState; + } + + Map snapLifecycles = new HashMap<>(snapMeta.getSnapshotConfigurations()); + SnapshotLifecyclePolicyMetadata policyMetadata = snapLifecycles.get(policyName); + if (policyMetadata == null) { + logger.error("failed to record snapshot [{}] for snapshot policy [{}]: policy not found", + exception.isPresent() ? "failure" : "success", policyName); + return currentState; + } + + SnapshotLifecyclePolicyMetadata.Builder newPolicyMetadata = SnapshotLifecyclePolicyMetadata.builder(policyMetadata); + + if (exception.isPresent()) { + newPolicyMetadata.setLastFailureDate(timestamp); + newPolicyMetadata.setLastFailureInfo(exceptionToString()); + } else { + newPolicyMetadata.setLastSuccessDate(timestamp); + } + + snapLifecycles.put(policyName, newPolicyMetadata.build()); + SnapshotLifecycleMetadata lifecycleMetadata = new SnapshotLifecycleMetadata(snapLifecycles); + MetaData currentMeta = currentState.metaData(); + return ClusterState.builder(currentState) + .metaData(MetaData.builder(currentMeta) + .putCustom(SnapshotLifecycleMetadata.TYPE, lifecycleMetadata)) + .build(); + } + + @Override + public void onFailure(String source, Exception e) { + logger.error("failed to record snapshot policy execution status [{}]: {}", source, e); + } + } } diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportGetSnapshotLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportGetSnapshotLifecycleAction.java index 30cee15128b19..f5ecdfaac34e1 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportGetSnapshotLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportGetSnapshotLifecycleAction.java @@ -76,9 +76,7 @@ protected void masterOperation(final GetSnapshotLifecycleAction.Request request, return ids.contains(meta.getPolicy().getId()); } }) - .map(meta -> - new GetSnapshotLifecycleAction.SnapshotLifecyclePolicyItem(meta.getPolicy(), - meta.getVersion(), meta.getModifiedDate())) + .map(GetSnapshotLifecycleAction.SnapshotLifecyclePolicyItem::new) .collect(Collectors.toList()); listener.onResponse(new GetSnapshotLifecycleAction.Response(lifecycles)); } diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportPutSnapshotLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportPutSnapshotLifecycleAction.java index 01fd69ae9abbd..ec55d616b66f9 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportPutSnapshotLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportPutSnapshotLifecycleAction.java @@ -82,15 +82,22 @@ public ClusterState execute(ClusterState currentState) { String id = request.getLifecycleId(); final SnapshotLifecycleMetadata lifecycleMetadata; if (snapMeta == null) { - SnapshotLifecyclePolicyMetadata meta = new SnapshotLifecyclePolicyMetadata(request.getLifecycle(), filteredHeaders, - 0, Instant.now().toEpochMilli()); + SnapshotLifecyclePolicyMetadata meta = SnapshotLifecyclePolicyMetadata.builder() + .setPolicy(request.getLifecycle()) + .setHeaders(filteredHeaders) + .setModifiedDate(Instant.now().toEpochMilli()) + .build(); lifecycleMetadata = new SnapshotLifecycleMetadata(Collections.singletonMap(id, meta)); logger.info("adding new snapshot lifecycle [{}]", id); } else { Map snapLifecycles = new HashMap<>(snapMeta.getSnapshotConfigurations()); SnapshotLifecyclePolicyMetadata oldLifecycle = snapLifecycles.get(id); - SnapshotLifecyclePolicyMetadata newLifecycle = new SnapshotLifecyclePolicyMetadata(request.getLifecycle(), - filteredHeaders, oldLifecycle == null ? 0L : oldLifecycle.getVersion() + 1, Instant.now().toEpochMilli()); + SnapshotLifecyclePolicyMetadata newLifecycle = SnapshotLifecyclePolicyMetadata.builder(oldLifecycle) + .setPolicy(request.getLifecycle()) + .setHeaders(filteredHeaders) + .setVersion(oldLifecycle == null ? 1L : oldLifecycle.getVersion() + 1) + .setModifiedDate(Instant.now().toEpochMilli()) + .build(); snapLifecycles.put(id, newLifecycle); lifecycleMetadata = new SnapshotLifecycleMetadata(snapLifecycles); if (oldLifecycle == null) { diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleServiceTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleServiceTests.java index 8fc6ecdc29742..730d31fd7cda0 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleServiceTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleServiceTests.java @@ -41,7 +41,12 @@ public void testGetJobId() { String id = randomAlphaOfLengthBetween(1, 10) + (randomBoolean() ? "" : randomLong()); SnapshotLifecyclePolicy policy = createPolicy(id); long version = randomNonNegativeLong(); - SnapshotLifecyclePolicyMetadata meta = new SnapshotLifecyclePolicyMetadata(policy, Collections.emptyMap(), version, 1); + SnapshotLifecyclePolicyMetadata meta = SnapshotLifecyclePolicyMetadata.builder() + .setPolicy(policy) + .setHeaders(Collections.emptyMap()) + .setVersion(version) + .setModifiedDate(1) + .build(); assertThat(SnapshotLifecycleService.getJobId(meta), equalTo(id + "-" + version)); } @@ -63,9 +68,11 @@ public void testPolicyCRUD() throws Exception { ClusterState previousState = createState(snapMeta); Map policies = new HashMap<>(); - SnapshotLifecyclePolicyMetadata policy = - new SnapshotLifecyclePolicyMetadata(createPolicy("foo", "*/1 * * * * ?"), // trigger every second - Collections.emptyMap(), 1, 1); + SnapshotLifecyclePolicyMetadata policy = SnapshotLifecyclePolicyMetadata.builder() + .setPolicy(createPolicy("foo", "*/1 * * * * ?")) + .setHeaders(Collections.emptyMap()) + .setModifiedDate(1) + .build(); policies.put(policy.getPolicy().getId(), policy); snapMeta = new SnapshotLifecycleMetadata(policies); ClusterState state = createState(snapMeta); @@ -89,8 +96,12 @@ public void testPolicyCRUD() throws Exception { clock.freeze(); int currentCount = triggerCount.get(); previousState = state; - SnapshotLifecyclePolicyMetadata newPolicy = - new SnapshotLifecyclePolicyMetadata(createPolicy("foo", "*/1 * * * * ?"), Collections.emptyMap(), 2, 2); + SnapshotLifecyclePolicyMetadata newPolicy = SnapshotLifecyclePolicyMetadata.builder() + .setPolicy(createPolicy("foo", "*/1 * * * * ?")) + .setHeaders(Collections.emptyMap()) + .setVersion(2) + .setModifiedDate(2) + .build(); policies.put(policy.getPolicy().getId(), newPolicy); state = createState(new SnapshotLifecycleMetadata(policies)); event = new ClusterChangedEvent("2", state, previousState); @@ -119,9 +130,12 @@ public void testPolicyCRUD() throws Exception { assertThat(sls.getScheduler().scheduledJobIds(), equalTo(Collections.emptySet())); // When the service is no longer master, all jobs should be automatically cancelled - policy = - new SnapshotLifecyclePolicyMetadata(createPolicy("foo", "*/1 * * * * ?"), // trigger every second - Collections.emptyMap(), 3, 1); + policy = SnapshotLifecyclePolicyMetadata.builder() + .setPolicy(createPolicy("foo", "*/1 * * * * ?")) + .setHeaders(Collections.emptyMap()) + .setVersion(3) + .setModifiedDate(1) + .build(); policies.put(policy.getPolicy().getId(), policy); snapMeta = new SnapshotLifecycleMetadata(policies); previousState = state; @@ -160,9 +174,12 @@ public void testPolicyNamesEndingInNumbers() throws Exception { ClusterState previousState = createState(snapMeta); Map policies = new HashMap<>(); - SnapshotLifecyclePolicyMetadata policy = - new SnapshotLifecyclePolicyMetadata(createPolicy("foo-2", "30 * * * * ?"), - Collections.emptyMap(), 1, 1); + SnapshotLifecyclePolicyMetadata policy = SnapshotLifecyclePolicyMetadata.builder() + .setPolicy(createPolicy("foo-2", "30 * * * * ?")) + .setHeaders(Collections.emptyMap()) + .setVersion(1) + .setModifiedDate(1) + .build(); policies.put(policy.getPolicy().getId(), policy); snapMeta = new SnapshotLifecycleMetadata(policies); ClusterState state = createState(snapMeta); @@ -172,9 +189,12 @@ public void testPolicyNamesEndingInNumbers() throws Exception { assertThat(sls.getScheduler().scheduledJobIds(), equalTo(Collections.singleton("foo-2-1"))); previousState = state; - SnapshotLifecyclePolicyMetadata secondPolicy = - new SnapshotLifecyclePolicyMetadata(createPolicy("foo-1", "45 * * * * ?"), - Collections.emptyMap(), 2, 1); + SnapshotLifecyclePolicyMetadata secondPolicy = SnapshotLifecyclePolicyMetadata.builder() + .setPolicy(createPolicy("foo-1", "45 * * * * ?")) + .setHeaders(Collections.emptyMap()) + .setVersion(2) + .setModifiedDate(1) + .build(); policies.put(secondPolicy.getPolicy().getId(), secondPolicy); snapMeta = new SnapshotLifecycleMetadata(policies); state = createState(snapMeta); diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java index 9676f7eab4f5b..6086ea3a29327 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java @@ -195,6 +195,14 @@ private SnapshotLifecyclePolicyMetadata makePolicyMeta(final String id) { SnapshotLifecyclePolicy policy = SnapshotLifecycleServiceTests.createPolicy(id); Map headers = new HashMap<>(); headers.put("X-Opaque-ID", randomAlphaOfLength(4)); - return new SnapshotLifecyclePolicyMetadata(policy, headers, 1, 1); + return SnapshotLifecyclePolicyMetadata.builder() + .setPolicy(policy) + .setHeaders(headers) + .setVersion(1) + .setModifiedDate(1) + .setLastSuccessDate(null) + .setLastFailureDate(null) + .setLastFailureInfo(null) + .build(); } } From 1895dc17876a00c8e075fcdc5d800eb3f648e928 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Fri, 29 Mar 2019 17:40:39 -0600 Subject: [PATCH 2/5] Review feedback --- .../SnapshotInvocationRecord.java | 107 +++++++++++++++ .../SnapshotLifecyclePolicy.java | 2 +- .../SnapshotLifecyclePolicyMetadata.java | 128 ++++++++---------- .../action/GetSnapshotLifecycleAction.java | 33 ++--- .../SnapshotInvocationRecordTests.java | 61 +++++++++ .../SnapshotLifecyclePolicyMetadataTests.java | 108 +++++++++++++++ .../SnapshotLifecycleIT.java | 22 ++- .../SnapshotLifecycleTask.java | 41 +++--- .../SnapshotLifecycleTaskTests.java | 3 - 9 files changed, 390 insertions(+), 115 deletions(-) create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotInvocationRecord.java create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotInvocationRecordTests.java create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadataTests.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotInvocationRecord.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotInvocationRecord.java new file mode 100644 index 0000000000000..e065a30633f87 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotInvocationRecord.java @@ -0,0 +1,107 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.core.snapshotlifecycle; + +import org.elasticsearch.cluster.AbstractDiffable; +import org.elasticsearch.cluster.Diffable; +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; + +import java.io.IOException; +import java.util.Objects; + +public class SnapshotInvocationRecord extends AbstractDiffable + implements Writeable, ToXContentObject, Diffable { + + static final ParseField SNAPSHOT_NAME = new ParseField("snapshot_name"); + static final ParseField TIMESTAMP = new ParseField("time"); + static final ParseField DETAILS = new ParseField("details"); + + private String snapshotName; + private long timestamp; + private String details; + + public static final ConstructingObjectParser PARSER = + new ConstructingObjectParser<>("snapshot_policy_invocation_record", true, + a -> new SnapshotInvocationRecord((String) a[0], (long) a[1], (String) a[2])); + + static { + PARSER.declareString(ConstructingObjectParser.constructorArg(), SNAPSHOT_NAME); + PARSER.declareLong(ConstructingObjectParser.constructorArg(), TIMESTAMP); + PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), DETAILS); + } + + public static SnapshotInvocationRecord parse(XContentParser parser, String name) { + return PARSER.apply(parser, name); + } + + public SnapshotInvocationRecord(String snapshotName, long timestamp, String details) { + this.snapshotName = Objects.requireNonNull(snapshotName, "snapshot name must be provided"); + this.timestamp = timestamp; + this.details = details; + } + + public SnapshotInvocationRecord(StreamInput in) throws IOException { + this.snapshotName = in.readString(); + this.timestamp = in.readVLong(); + this.details = in.readOptionalString(); + } + + public String getSnapshotName() { + return snapshotName; + } + + public long getTimestamp() { + return timestamp; + } + + public String getDetails() { + return details; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(snapshotName); + out.writeVLong(timestamp); + out.writeOptionalString(details); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + { + builder.field(SNAPSHOT_NAME.getPreferredName(), snapshotName); + builder.timeField(TIMESTAMP.getPreferredName(), "time_string", timestamp); + if (Objects.nonNull(details)) { + builder.field(DETAILS.getPreferredName(), details); + } + } + builder.endObject(); + return builder; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + SnapshotInvocationRecord that = (SnapshotInvocationRecord) o; + return getTimestamp() == that.getTimestamp() && + Objects.equals(getSnapshotName(), that.getSnapshotName()) && + Objects.equals(getDetails(), that.getDetails()); + } + + @Override + public int hashCode() { + return Objects.hash(getSnapshotName(), getTimestamp(), getDetails()); + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java index e1837978541a2..ad52e4256591b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java @@ -73,7 +73,7 @@ public class SnapshotLifecyclePolicy extends AbstractDiffable configuration) { - this.id = id; + this.id = Objects.requireNonNull(id); this.name = name; this.schedule = schedule; this.repository = repository; diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadata.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadata.java index 1fc5366cd235e..5edf574661f60 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadata.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadata.java @@ -10,6 +10,7 @@ import org.elasticsearch.cluster.Diffable; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ConstructingObjectParser; @@ -22,8 +23,10 @@ import java.time.Instant; import java.time.ZoneOffset; import java.time.ZonedDateTime; +import java.util.HashMap; import java.util.Map; import java.util.Objects; +import java.util.Optional; /** * {@code SnapshotLifecyclePolicyMetadata} encapsulates a {@link SnapshotLifecyclePolicy} as well as @@ -38,29 +41,34 @@ public class SnapshotLifecyclePolicyMetadata extends AbstractDiffable headers; private final long version; private final long modifiedDate; @Nullable - private final Long lastSuccessDate; + private final SnapshotInvocationRecord lastSuccess; @Nullable - private final Long lastFailureDate; - @Nullable - private final String lastFailureInfo; + private final SnapshotInvocationRecord lastFailure; @SuppressWarnings("unchecked") public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("snapshot_policy_metadata", a -> { SnapshotLifecyclePolicy policy = (SnapshotLifecyclePolicy) a[0]; - return new Builder().setPolicy(policy).setHeaders((Map) a[1]).setVersion((long) a[2]).setModifiedDate((long) a[3]).setLastSuccessDate((Long) a[4]).setLastFailureDate((Long) a[5]).setLastFailureInfo((String) a[6]).build(); + SnapshotInvocationRecord lastSuccess = (SnapshotInvocationRecord) a[5]; + SnapshotInvocationRecord lastFailure = (SnapshotInvocationRecord) a[6]; + + return builder() + .setPolicy(policy) + .setHeaders((Map) a[1]) + .setVersion((long) a[2]) + .setModifiedDate((long) a[3]) + .setLastSuccess(lastSuccess) + .setLastFailure(lastFailure) + .build(); }); static { @@ -69,11 +77,8 @@ public class SnapshotLifecyclePolicyMetadata extends AbstractDiffable headers, long version, long modifiedDate, - Long lastSuccessDate, Long lastFailureDate, String lastFailureInfo) { - this.policy = Objects.requireNonNull(policy); - this.headers = Objects.requireNonNull(headers); + SnapshotInvocationRecord lastSuccess, SnapshotInvocationRecord lastFailure) { + this.policy = policy; + this.headers = headers; this.version = version; this.modifiedDate = modifiedDate; - this.lastSuccessDate = lastSuccessDate; - this.lastFailureDate = lastFailureDate; - this.lastFailureInfo = lastFailureInfo; + this.lastSuccess = lastSuccess; + this.lastFailure = lastFailure; } @SuppressWarnings("unchecked") @@ -97,9 +101,8 @@ public static SnapshotLifecyclePolicyMetadata parse(XContentParser parser, Strin this.headers = (Map) in.readGenericValue(); this.version = in.readVLong(); this.modifiedDate = in.readVLong(); - this.lastSuccessDate = in.readOptionalLong(); - this.lastFailureDate = in.readOptionalLong(); - this.lastFailureInfo = in.readOptionalString(); + this.lastSuccess = in.readOptionalWriteable(SnapshotInvocationRecord::new); + this.lastFailure = in.readOptionalWriteable(SnapshotInvocationRecord::new); } @Override @@ -108,9 +111,8 @@ public void writeTo(StreamOutput out) throws IOException { out.writeGenericValue(this.headers); out.writeVLong(this.version); out.writeVLong(this.modifiedDate); - out.writeOptionalLong(this.lastSuccessDate); - out.writeOptionalLong(this.lastFailureDate); - out.writeOptionalString(this.lastFailureInfo); + out.writeOptionalWriteable(this.lastSuccess); + out.writeOptionalWriteable(this.lastFailure); } public static Builder builder() { @@ -126,9 +128,8 @@ public static Builder builder(SnapshotLifecyclePolicyMetadata metadata) { .setPolicy(metadata.getPolicy()) .setVersion(metadata.getVersion()) .setModifiedDate(metadata.getModifiedDate()) - .setLastSuccessDate(metadata.getLastSuccessDate()) - .setLastFailureDate(metadata.getLastFailureDate()) - .setLastFailureInfo(metadata.getLastFailureInfo()); + .setLastSuccess(metadata.getLastSuccess()) + .setLastFailure(metadata.getLastFailure()); } public Map getHeaders() { @@ -163,23 +164,12 @@ public String getModifiedDateString() { return dateToDateString(modifiedDate); } - public Long getLastSuccessDate() { - return lastSuccessDate; - } - - public String getLastSuccessDateString() { - return dateToDateString(lastSuccessDate); + public SnapshotInvocationRecord getLastSuccess() { + return lastSuccess; } - public Long getLastFailureDate() { - return lastFailureDate; - } - public String getLastFailureDateString() { - return dateToDateString(lastFailureDate); - } - - public String getLastFailureInfo() { - return lastFailureInfo; + public SnapshotInvocationRecord getLastFailure() { + return lastFailure; } @Override @@ -190,18 +180,19 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field(VERSION.getPreferredName(), version); builder.field(MODIFIED_DATE.getPreferredName(), modifiedDate); builder.field(MODIFIED_DATE_STRING.getPreferredName(), getModifiedDateString()); - builder.field(LAST_SUCCESS_DATE.getPreferredName(), lastSuccessDate); - builder.field(LAST_SUCCESS_DATE_STRING.getPreferredName(), getLastSuccessDateString()); - builder.field(LAST_FAILURE_DATE.getPreferredName(), lastFailureDate); - builder.field(LAST_FAILURE_DATE_STRING.getPreferredName(), getLastFailureDateString()); - builder.field(LAST_FAILURE_INFO.getPreferredName(), lastFailureInfo); + if (Objects.nonNull(lastSuccess)) { + builder.field(LAST_SUCCESS.getPreferredName(), lastSuccess); + } + if (Objects.nonNull(lastFailure)) { + builder.field(LAST_FAILURE.getPreferredName(), lastFailure); + } builder.endObject(); return builder; } @Override public int hashCode() { - return Objects.hash(policy, headers, version, modifiedDate); + return Objects.hash(policy, headers, version, modifiedDate, lastSuccess, lastFailure); } @Override @@ -217,9 +208,8 @@ public boolean equals(Object obj) { Objects.equals(headers, other.headers) && Objects.equals(version, other.version) && Objects.equals(modifiedDate, other.modifiedDate) && - Objects.equals(lastSuccessDate, other.lastSuccessDate) && - Objects.equals(lastFailureDate, other.lastFailureDate) && - Objects.equals(lastFailureInfo, other.lastFailureInfo); + Objects.equals(lastSuccess, other.lastSuccess) && + Objects.equals(lastFailure, other.lastFailure); } @Override @@ -227,20 +217,21 @@ public String toString() { // Note: this is on purpose. While usually we would use Strings.toString(this) to render // this using toXContent, it may contain sensitive information in the headers and thus // should not emit them in case it accidentally gets logged. - return super.toString(); +// return super.toString(); + return Strings.toString(this); //NOCOMMIT } public static class Builder { - private Builder() {} + private Builder() { + } private SnapshotLifecyclePolicy policy; private Map headers; private long version = 1L; private Long modifiedDate; - private Long lastSuccessDate; - private Long lastFailureDate; - private String lastFailureInfo; + private SnapshotInvocationRecord lastSuccessDate; + private SnapshotInvocationRecord lastFailureDate; public Builder setPolicy(SnapshotLifecyclePolicy policy) { this.policy = policy; @@ -262,24 +253,25 @@ public Builder setModifiedDate(long modifiedDate) { return this; } - public Builder setLastSuccessDate(Long lastSuccessDate) { + public Builder setLastSuccess(SnapshotInvocationRecord lastSuccessDate) { this.lastSuccessDate = lastSuccessDate; return this; } - public Builder setLastFailureDate(Long lastFailureDate) { + public Builder setLastFailure(SnapshotInvocationRecord lastFailureDate) { this.lastFailureDate = lastFailureDate; return this; } - public Builder setLastFailureInfo(String lastFailureInfo) { - this.lastFailureInfo = lastFailureInfo; - return this; - } - public SnapshotLifecyclePolicyMetadata build() { - Objects.requireNonNull(modifiedDate, "modifiedDate must be set"); - return new SnapshotLifecyclePolicyMetadata(policy, headers, version, modifiedDate, lastSuccessDate, lastFailureDate, lastFailureInfo); + return new SnapshotLifecyclePolicyMetadata( + Objects.requireNonNull(policy), + Optional.ofNullable(headers).orElse(new HashMap<>()), + version, + Objects.requireNonNull(modifiedDate, "modifiedDate must be set"), + lastSuccessDate, + lastFailureDate); } } + } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/action/GetSnapshotLifecycleAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/action/GetSnapshotLifecycleAction.java index f13ed6936778b..10992a58210cc 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/action/GetSnapshotLifecycleAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/action/GetSnapshotLifecycleAction.java @@ -18,6 +18,7 @@ import org.elasticsearch.common.xcontent.ToXContentFragment; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotInvocationRecord; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicy; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicyMetadata; @@ -158,28 +159,24 @@ public static class SnapshotLifecyclePolicyItem implements ToXContentFragment, W private final long version; private final long modifiedDate; @Nullable - private final Long lastSuccessDate; + private final SnapshotInvocationRecord lastSuccess; @Nullable - private final Long lastFailureDate; - @Nullable - private final String lastFailureInfo; + private final SnapshotInvocationRecord lastFailure; public SnapshotLifecyclePolicyItem(SnapshotLifecyclePolicyMetadata policyMetadata) { this.policy = policyMetadata.getPolicy(); this.version = policyMetadata.getVersion(); this.modifiedDate = policyMetadata.getModifiedDate(); - this.lastSuccessDate = policyMetadata.getLastSuccessDate(); - this.lastFailureDate = policyMetadata.getLastFailureDate(); - this.lastFailureInfo = policyMetadata.getLastFailureInfo(); + this.lastSuccess = policyMetadata.getLastSuccess(); + this.lastFailure = policyMetadata.getLastFailure(); } public SnapshotLifecyclePolicyItem(StreamInput in) throws IOException { this.policy = new SnapshotLifecyclePolicy(in); this.version = in.readVLong(); this.modifiedDate = in.readVLong(); - this.lastSuccessDate = in.readOptionalLong(); - this.lastFailureDate = in.readOptionalLong(); - this.lastFailureInfo = in.readOptionalString(); + this.lastSuccess = in.readOptionalWriteable(SnapshotInvocationRecord::new); + this.lastFailure = in.readOptionalWriteable(SnapshotInvocationRecord::new); } public SnapshotLifecyclePolicy getPolicy() { @@ -199,9 +196,8 @@ public void writeTo(StreamOutput out) throws IOException { policy.writeTo(out); out.writeVLong(version); out.writeVLong(modifiedDate); - out.writeOptionalLong(lastSuccessDate); - out.writeOptionalLong(lastFailureDate); - out.writeOptionalString(lastFailureInfo); + out.writeOptionalWriteable(lastSuccess); + out.writeOptionalWriteable(lastFailure); } @Override @@ -229,14 +225,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field("version", version); builder.field("modified_date", modifiedDate); builder.field("policy", policy); - if (lastSuccessDate != null) { - builder.field("last_success_date", lastSuccessDate); - } - if (lastFailureDate != null) { - builder.field("last_failure_date", lastFailureDate); + if (lastSuccess != null) { + builder.field("last_success", lastSuccess); } - if (lastFailureInfo != null) { - builder.field("last_failure_info", lastFailureInfo); + if (lastFailure != null) { + builder.field("last_failure", lastFailure); } builder.endObject(); return builder; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotInvocationRecordTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotInvocationRecordTests.java new file mode 100644 index 0000000000000..af9511b183e9e --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotInvocationRecordTests.java @@ -0,0 +1,61 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.core.snapshotlifecycle; + +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.test.AbstractSerializingTestCase; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; + +public class SnapshotInvocationRecordTests extends AbstractSerializingTestCase { + + @Override + protected SnapshotInvocationRecord doParseInstance(XContentParser parser) throws IOException { + return SnapshotInvocationRecord.parse(parser, null); + } + + @Override + protected SnapshotInvocationRecord createTestInstance() { + return randomSnapshotInvocationRecord(); + } + + @Override + protected Writeable.Reader instanceReader() { + return SnapshotInvocationRecord::new; + } + + @Override + protected SnapshotInvocationRecord mutateInstance(SnapshotInvocationRecord instance) { + switch (between(0, 2)) { + case 0: + return new SnapshotInvocationRecord( + randomValueOtherThan(instance.getSnapshotName(), () -> randomAlphaOfLengthBetween(2,10)), + instance.getTimestamp(), + instance.getDetails()); + case 1: + return new SnapshotInvocationRecord(instance.getSnapshotName(), + randomValueOtherThan(instance.getTimestamp(), ESTestCase::randomNonNegativeLong), + instance.getDetails()); + case 2: + return new SnapshotInvocationRecord(instance.getSnapshotName(), + instance.getTimestamp(), + randomValueOtherThan(instance.getDetails(), () -> randomAlphaOfLengthBetween(2,10))); + default: + throw new AssertionError("failure, got illegal switch case"); + } + } + + public static SnapshotInvocationRecord randomSnapshotInvocationRecord() { + return new SnapshotInvocationRecord( + randomAlphaOfLengthBetween(5,10), + randomNonNegativeLong(), + randomBoolean() ? null : randomAlphaOfLengthBetween(5, 10)); + } + +} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadataTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadataTests.java new file mode 100644 index 0000000000000..f63cf7f2b241d --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadataTests.java @@ -0,0 +1,108 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.core.snapshotlifecycle; + +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.test.AbstractSerializingTestCase; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +import static org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotInvocationRecordTests.randomSnapshotInvocationRecord; + +public class SnapshotLifecyclePolicyMetadataTests extends AbstractSerializingTestCase { + private String policyId; + + @Override + protected SnapshotLifecyclePolicyMetadata doParseInstance(XContentParser parser) throws IOException { + return SnapshotLifecyclePolicyMetadata.PARSER.apply(parser, policyId); + } + + @Override + protected SnapshotLifecyclePolicyMetadata createTestInstance() { + policyId = randomAlphaOfLength(5); + SnapshotLifecyclePolicyMetadata.Builder builder = SnapshotLifecyclePolicyMetadata.builder() + .setPolicy(createRandomPolicy(policyId)) + .setVersion(randomNonNegativeLong()) + .setModifiedDate(randomNonNegativeLong()); + if (randomBoolean()) { + builder.setHeaders(randomHeaders()); + } + if (randomBoolean()) { + builder.setLastSuccess(randomSnapshotInvocationRecord()); + } + if (randomBoolean()) { + builder.setLastFailure(randomSnapshotInvocationRecord()); + } + return builder.build(); + } + + private Map randomHeaders() { + Map headers = new HashMap<>(); + int headerCount = randomIntBetween(1,10); + for (int i = 0; i < headerCount; i++) { + headers.put(randomAlphaOfLengthBetween(5,10), randomAlphaOfLengthBetween(5,10)); + } + return headers; + } + + @Override + protected Writeable.Reader instanceReader() { + return SnapshotLifecyclePolicyMetadata::new; + } + + @Override + protected SnapshotLifecyclePolicyMetadata mutateInstance(SnapshotLifecyclePolicyMetadata instance) throws IOException { + switch (between(0, 4)) { + case 0: + return SnapshotLifecyclePolicyMetadata.builder(instance) + .setPolicy(randomValueOtherThan(instance.getPolicy(), () -> createRandomPolicy(randomAlphaOfLength(10)))) + .build(); + case 1: + return SnapshotLifecyclePolicyMetadata.builder(instance) + .setVersion(randomValueOtherThan(instance.getVersion(), ESTestCase::randomNonNegativeLong)) + .build(); + case 2: + return SnapshotLifecyclePolicyMetadata.builder(instance) + .setHeaders(randomValueOtherThan(instance.getHeaders(), this::randomHeaders)) + .build(); + case 3: + return SnapshotLifecyclePolicyMetadata.builder(instance) + .setLastSuccess(randomValueOtherThan(instance.getLastSuccess(), + SnapshotInvocationRecordTests::randomSnapshotInvocationRecord)) + .build(); + case 4: + return SnapshotLifecyclePolicyMetadata.builder(instance) + .setLastFailure(randomValueOtherThan(instance.getLastFailure(), + SnapshotInvocationRecordTests::randomSnapshotInvocationRecord)) + .build(); + default: + throw new AssertionError("failure, got illegal switch case"); + } + } + + public static SnapshotLifecyclePolicy createRandomPolicy(String policyId) { + Map config = new HashMap<>(); + for (int i = 0; i < randomIntBetween(2, 5); i++) { + config.put(randomAlphaOfLength(4), randomAlphaOfLength(4)); + } + return new SnapshotLifecyclePolicy(policyId, + randomAlphaOfLength(4), + randomSchedule(), + randomAlphaOfLength(4), + config); + } + + private static String randomSchedule() { + return randomIntBetween(0, 59) + " " + + randomIntBetween(0, 59) + " " + + randomIntBetween(0, 12) + " * * ?"; + } +} diff --git a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java index b202b64ebffbc..771190b73b8dc 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java @@ -72,11 +72,16 @@ public void testFullPolicySnapshot() throws Exception { policyResponseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); } Map policyMetadataMap = (Map) policyResponseMap.get(policyName); - Long lastSuccess = (Long) policyMetadataMap.get("last_success_date"); + Map lastSuccessObject = (Map) policyMetadataMap.get("last_success"); + assertNotNull(lastSuccessObject); + Long lastSuccess = (Long) lastSuccessObject.get("time"); Long modifiedDate = (Long) policyMetadataMap.get("modified_date"); assertNotNull(lastSuccess); assertNotNull(modifiedDate); assertThat(lastSuccess, greaterThan(modifiedDate)); + + String lastSnapshotName = (String) lastSuccessObject.get("snapshot_name"); + assertThat(lastSnapshotName, startsWith("snap-")); }); Request delReq = new Request("DELETE", "/_ilm/snapshot/" + policyName); @@ -89,6 +94,7 @@ public void testFullPolicySnapshot() throws Exception { }); } + @SuppressWarnings("unchecked") public void testPolicyFailure() throws Exception { final String indexName = "test"; final String policyName = "test-policy"; @@ -108,15 +114,22 @@ public void testPolicyFailure() throws Exception { try (InputStream is = policyMetadata.getEntity().getContent()) { Map responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); Map policyMetadataMap = (Map) responseMap.get(policyName); - Long lastFailure = (Long) policyMetadataMap.get("last_failure_date"); - String lastFailureInfo = (String) policyMetadataMap.get("last_failure_info"); + Map lastFailureObject = (Map) policyMetadataMap.get("last_failure"); + assertNotNull(lastFailureObject); + + Long lastFailure = (Long) lastFailureObject.get("time"); Long modifiedDate = (Long) policyMetadataMap.get("modified_date"); assertNotNull(lastFailure); assertNotNull(modifiedDate); assertThat(lastFailure, greaterThan(modifiedDate)); + String lastFailureInfo = (String) lastFailureObject.get("details"); assertNotNull(lastFailureInfo); assertThat(lastFailureInfo, containsString("[bad-repo] missing")); + + String snapshotName = (String) lastFailureObject.get("snapshot_name"); + assertNotNull(snapshotName); + assertThat(snapshotName, startsWith("snap-")); } }); @@ -124,7 +137,8 @@ public void testPolicyFailure() throws Exception { assertOK(client().performRequest(delReq)); } - private void createSnapshotPolicy(String policyName, String snapshotNamePattern, String schedule, String repoId, String indexPattern) throws IOException { + private void createSnapshotPolicy(String policyName, String snapshotNamePattern, String schedule, String repoId, + String indexPattern) throws IOException { Map snapConfig = new HashMap<>(); snapConfig.put("indices", Collections.singletonList(indexPattern)); SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy(policyName, snapshotNamePattern, schedule, repoId, snapConfig); diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java index e7a2e899bd44a..e71cb0d1c7b38 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java @@ -24,6 +24,7 @@ import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.xpack.core.ClientHelper; import org.elasticsearch.xpack.core.scheduler.SchedulerEngine; +import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotInvocationRecord; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecycleMetadata; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicyMetadata; import org.elasticsearch.xpack.indexlifecycle.LifecyclePolicySecurityClient; @@ -64,16 +65,16 @@ public void triggered(SchedulerEngine.Event event) { public void onResponse(CreateSnapshotResponse createSnapshotResponse) { logger.info("snapshot response for [{}]: {}", policyMetadata.getPolicy().getId(), Strings.toString(createSnapshotResponse)); - clusterService.submitStateUpdateTask("slm-record-success", - WriteJobStatus.success(policyMetadata.getPolicy().getId(), Instant.now().toEpochMilli())); + clusterService.submitStateUpdateTask("slm-record-success-" + policyMetadata.getPolicy().getId(), + WriteJobStatus.success(policyMetadata.getPolicy().getId(), request.snapshot(), Instant.now().toEpochMilli())); } @Override public void onFailure(Exception e) { - logger.error("failed to issue create snapshot request for snapshot lifecycle policy " + + logger.error("failed to issue create snapshot request for snapshot lifecycle policy [{}]: {}", policyMetadata.getPolicy().getId(), e); - clusterService.submitStateUpdateTask("slm-record-failure", - WriteJobStatus.failure(policyMetadata.getPolicy().getId(), Instant.now().toEpochMilli(), e)); + clusterService.submitStateUpdateTask("slm-record-failure-" + policyMetadata.getPolicy().getId(), + WriteJobStatus.failure(policyMetadata.getPolicy().getId(), request.snapshot(), Instant.now().toEpochMilli(), e)); } }); return true; @@ -100,30 +101,33 @@ private static class WriteJobStatus extends ClusterStateUpdateTask { new ToXContent.MapParams(Collections.singletonMap(REST_EXCEPTION_SKIP_STACK_TRACE, "false")); private final String policyName; + private final String snapshotName; private final long timestamp; private final Optional exception; - private WriteJobStatus(String policyName, long timestamp, Optional exception) { + private WriteJobStatus(String policyName, String snapshotName, long timestamp, Optional exception) { this.policyName = policyName; + this.snapshotName = snapshotName; this.exception = exception; this.timestamp = timestamp; } - static WriteJobStatus success(String policyId, long timestamp) { - return new WriteJobStatus(policyId, timestamp, Optional.empty()); + static WriteJobStatus success(String policyId, String snapshotName, long timestamp) { + return new WriteJobStatus(policyId, snapshotName, timestamp, Optional.empty()); } - static WriteJobStatus failure(String policyId, long timestamp, Exception exception) { - return new WriteJobStatus(policyId, timestamp, Optional.of(exception)); + static WriteJobStatus failure(String policyId, String snapshotName, long timestamp, Exception exception) { + return new WriteJobStatus(policyId, snapshotName, timestamp, Optional.of(exception)); } private String exceptionToString() throws IOException { if (exception.isPresent()) { - XContentBuilder causeXContentBuilder = JsonXContent.contentBuilder(); - causeXContentBuilder.startObject(); - ElasticsearchException.generateThrowableXContent(causeXContentBuilder, STACKTRACE_PARAMS, exception.get()); - causeXContentBuilder.endObject(); - return BytesReference.bytes(causeXContentBuilder).utf8ToString(); + try (XContentBuilder causeXContentBuilder = JsonXContent.contentBuilder()) { + causeXContentBuilder.startObject(); + ElasticsearchException.generateThrowableXContent(causeXContentBuilder, STACKTRACE_PARAMS, exception.get()); + causeXContentBuilder.endObject(); + return BytesReference.bytes(causeXContentBuilder).utf8ToString(); + } } return null; } @@ -142,7 +146,7 @@ public ClusterState execute(ClusterState currentState) throws Exception { Map snapLifecycles = new HashMap<>(snapMeta.getSnapshotConfigurations()); SnapshotLifecyclePolicyMetadata policyMetadata = snapLifecycles.get(policyName); if (policyMetadata == null) { - logger.error("failed to record snapshot [{}] for snapshot policy [{}]: policy not found", + logger.warn("failed to record snapshot [{}] for snapshot policy [{}]: policy not found", exception.isPresent() ? "failure" : "success", policyName); return currentState; } @@ -150,10 +154,9 @@ public ClusterState execute(ClusterState currentState) throws Exception { SnapshotLifecyclePolicyMetadata.Builder newPolicyMetadata = SnapshotLifecyclePolicyMetadata.builder(policyMetadata); if (exception.isPresent()) { - newPolicyMetadata.setLastFailureDate(timestamp); - newPolicyMetadata.setLastFailureInfo(exceptionToString()); + newPolicyMetadata.setLastFailure(new SnapshotInvocationRecord(snapshotName, timestamp, exceptionToString())); } else { - newPolicyMetadata.setLastSuccessDate(timestamp); + newPolicyMetadata.setLastSuccess(new SnapshotInvocationRecord(snapshotName, timestamp, null)); } snapLifecycles.put(policyName, newPolicyMetadata.build()); diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java index 6086ea3a29327..835de7cf095f0 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTaskTests.java @@ -200,9 +200,6 @@ private SnapshotLifecyclePolicyMetadata makePolicyMeta(final String id) { .setHeaders(headers) .setVersion(1) .setModifiedDate(1) - .setLastSuccessDate(null) - .setLastFailureDate(null) - .setLastFailureInfo(null) .build(); } } From 134bc69dd2f438d83ca016e0e64c2859a807b914 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Mon, 1 Apr 2019 15:49:21 -0600 Subject: [PATCH 3/5] Fix NOCOMMIT --- .../snapshotlifecycle/SnapshotLifecyclePolicyMetadata.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadata.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadata.java index 5edf574661f60..b2b5db865d95d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadata.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicyMetadata.java @@ -10,7 +10,6 @@ import org.elasticsearch.cluster.Diffable; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.ConstructingObjectParser; @@ -217,8 +216,7 @@ public String toString() { // Note: this is on purpose. While usually we would use Strings.toString(this) to render // this using toXContent, it may contain sensitive information in the headers and thus // should not emit them in case it accidentally gets logged. -// return super.toString(); - return Strings.toString(this); //NOCOMMIT + return super.toString(); } public static class Builder { From 42e12b6645084c635bd32017996706268fdb59ea Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Mon, 1 Apr 2019 16:16:28 -0600 Subject: [PATCH 4/5] Review feedback --- .../SnapshotInvocationRecord.java | 4 ++++ .../snapshotlifecycle/SnapshotLifecycleTask.java | 14 +++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotInvocationRecord.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotInvocationRecord.java index e065a30633f87..a39153f991664 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotInvocationRecord.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotInvocationRecord.java @@ -20,6 +20,10 @@ import java.io.IOException; import java.util.Objects; +/** + * Holds information about Snapshots kicked off by Snapshot Lifecycle Management in the cluster state, so that this information can be + * presented to the user. This class is used for both successes and failures as the structure of the data is very similar. + */ public class SnapshotInvocationRecord extends AbstractDiffable implements Writeable, ToXContentObject, Diffable { diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java index e71cb0d1c7b38..73d44a16ddbb9 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleTask.java @@ -96,6 +96,9 @@ static Optional getSnapPolicyMetadata(final Str .findFirst()); } + /** + * A cluster state update task to write the result of a snapshot job to the cluster metadata for the associated policy. + */ private static class WriteJobStatus extends ClusterStateUpdateTask { private static final ToXContent.Params STACKTRACE_PARAMS = new ToXContent.MapParams(Collections.singletonMap(REST_EXCEPTION_SKIP_STACK_TRACE, "false")); @@ -138,16 +141,16 @@ public ClusterState execute(ClusterState currentState) throws Exception { assert snapMeta != null : "this should never be called while the snapshot lifecycle cluster metadata is null"; if (snapMeta == null) { - logger.error("failed to record snapshot [{}] for snapshot policy [{}]: snapshot lifecycle metadata is null", - exception.isPresent() ? "failure" : "success", policyName); + logger.error("failed to record snapshot [{}] for snapshot [{}] in policy [{}]: snapshot lifecycle metadata is null", + exception.isPresent() ? "failure" : "success", snapshotName, policyName); return currentState; } Map snapLifecycles = new HashMap<>(snapMeta.getSnapshotConfigurations()); SnapshotLifecyclePolicyMetadata policyMetadata = snapLifecycles.get(policyName); if (policyMetadata == null) { - logger.warn("failed to record snapshot [{}] for snapshot policy [{}]: policy not found", - exception.isPresent() ? "failure" : "success", policyName); + logger.warn("failed to record snapshot [{}] for snapshot [{}] in policy [{}]: policy not found", + exception.isPresent() ? "failure" : "success", snapshotName, policyName); return currentState; } @@ -170,7 +173,8 @@ public ClusterState execute(ClusterState currentState) throws Exception { @Override public void onFailure(String source, Exception e) { - logger.error("failed to record snapshot policy execution status [{}]: {}", source, e); + logger.error("failed to record snapshot policy execution status for snapshot [{}] in policy [{}], (source: [{}]): {}", + snapshotName, policyName, source, e); } } } From fd95ea54727454aa17f90490a8256622608c1513 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Mon, 1 Apr 2019 17:42:04 -0600 Subject: [PATCH 5/5] Adjust integration test to account for validation --- .../SnapshotLifecycleIT.java | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java index 771190b73b8dc..60cd8944037ac 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java @@ -49,7 +49,7 @@ public void testFullPolicySnapshot() throws Exception { // Create a snapshot repo inializeRepo(repoId); - createSnapshotPolicy(policyName, "snap", "*/1 * * * * ?", repoId, indexName); + createSnapshotPolicy(policyName, "snap", "*/1 * * * * ?", repoId, indexName, true); // Check that the snapshot was actually taken assertBusy(() -> { @@ -96,16 +96,13 @@ public void testFullPolicySnapshot() throws Exception { @SuppressWarnings("unchecked") public void testPolicyFailure() throws Exception { - final String indexName = "test"; final String policyName = "test-policy"; - int docCount = randomIntBetween(10, 50); - List indexReqs = new ArrayList<>(); - for (int i = 0; i < docCount; i++) { - index(client(), indexName, "" + i, "foo", "bar"); - } + final String repoName = "test-repo"; + final String indexPattern = "index-doesnt-exist"; + inializeRepo(repoName); - // Create a policy with a repo that doesn't exist - createSnapshotPolicy(policyName, "snap", "*/1 * * * * ?", "bad-repo", indexName); + // Create a policy with ignore_unvailable: false and an index that doesn't exist + createSnapshotPolicy(policyName, "snap", "*/1 * * * * ?", repoName, indexPattern, false); assertBusy(() -> { // Check that the failure is written to the cluster state @@ -125,7 +122,7 @@ public void testPolicyFailure() throws Exception { String lastFailureInfo = (String) lastFailureObject.get("details"); assertNotNull(lastFailureInfo); - assertThat(lastFailureInfo, containsString("[bad-repo] missing")); + assertThat(lastFailureInfo, containsString("no such index [index-doesnt-exist]")); String snapshotName = (String) lastFailureObject.get("snapshot_name"); assertNotNull(snapshotName); @@ -138,9 +135,10 @@ public void testPolicyFailure() throws Exception { } private void createSnapshotPolicy(String policyName, String snapshotNamePattern, String schedule, String repoId, - String indexPattern) throws IOException { + String indexPattern, boolean ignoreUnavailable) throws IOException { Map snapConfig = new HashMap<>(); snapConfig.put("indices", Collections.singletonList(indexPattern)); + snapConfig.put("ignore_unavailable", ignoreUnavailable); SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy(policyName, snapshotNamePattern, schedule, repoId, snapConfig); Request putLifecycle = new Request("PUT", "/_ilm/snapshot/" + policyName);