Skip to content

add notion of version and creation_date to LifecyclePolicyMetadata #33450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 6, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,33 @@
import org.elasticsearch.common.xcontent.XContentParser;

import java.io.IOException;
import java.time.Instant;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.Map;
import java.util.Objects;

public class LifecyclePolicyMetadata extends AbstractDiffable<LifecyclePolicyMetadata>
implements ToXContentObject, Diffable<LifecyclePolicyMetadata> {

public static final ParseField POLICY = new ParseField("policy");
public static final ParseField HEADERS = new ParseField("headers");
static final ParseField POLICY = new ParseField("policy");
static final ParseField HEADERS = new ParseField("headers");
static final ParseField VERSION = new ParseField("version");
static final ParseField CREATION_DATE = new ParseField("creation_date");
static final ParseField CREATION_DATE_STRING = new ParseField("creation_date_string");

@SuppressWarnings("unchecked")
public static final ConstructingObjectParser<LifecyclePolicyMetadata, String> PARSER = new ConstructingObjectParser<>("policy_metadata",
a -> {
LifecyclePolicy policy = (LifecyclePolicy) a[0];
return new LifecyclePolicyMetadata(policy, (Map<String, String>) a[1]);
return new LifecyclePolicyMetadata(policy, (Map<String, String>) a[1], (long) a[2], (long) a[3]);
});
static {
PARSER.declareObject(ConstructingObjectParser.constructorArg(), LifecyclePolicy::parse, POLICY);
PARSER.declareField(ConstructingObjectParser.constructorArg(), XContentParser::mapStrings, HEADERS, ValueType.OBJECT);
PARSER.declareLong(ConstructingObjectParser.constructorArg(), VERSION);
PARSER.declareLong(ConstructingObjectParser.constructorArg(), CREATION_DATE);
PARSER.declareString(ConstructingObjectParser.constructorArg(), CREATION_DATE_STRING);
}

public static LifecyclePolicyMetadata parse(XContentParser parser, String name) {
Expand All @@ -43,16 +53,22 @@ public static LifecyclePolicyMetadata parse(XContentParser parser, String name)

private final LifecyclePolicy policy;
private final Map<String, String> headers;
private final long version;
private final long creationDate;

public LifecyclePolicyMetadata(LifecyclePolicy policy, Map<String, String> headers) {
public LifecyclePolicyMetadata(LifecyclePolicy policy, Map<String, String> headers, long version, long creationDate) {
this.policy = policy;
this.headers = headers;
this.version = version;
this.creationDate = creationDate;
}

@SuppressWarnings("unchecked")
public LifecyclePolicyMetadata(StreamInput in) throws IOException {
this.policy = new LifecyclePolicy(in);
this.headers = (Map<String, String>) in.readGenericValue();
this.version = in.readVLong();
this.creationDate = in.readVLong();
}

public Map<String, String> getHeaders() {
Expand All @@ -67,11 +83,27 @@ public String getName() {
return policy.getName();
}

public long getVersion() {
return version;
}

public long getCreationDate() {
return creationDate;
}

public String getCreationDateString() {
ZonedDateTime creationDateTime = ZonedDateTime.ofInstant(Instant.ofEpochMilli(creationDate), ZoneOffset.UTC);
return creationDateTime.toString();
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field(POLICY.getPreferredName(), policy);
builder.field(HEADERS.getPreferredName(), headers);
builder.field(VERSION.getPreferredName(), version);
builder.field(CREATION_DATE.getPreferredName(), creationDate);
builder.field(CREATION_DATE_STRING.getPreferredName(), getCreationDateString());
builder.endObject();
return builder;
}
Expand All @@ -80,11 +112,13 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
public void writeTo(StreamOutput out) throws IOException {
policy.writeTo(out);
out.writeGenericValue(headers);
out.writeVLong(version);
out.writeVLong(creationDate);
}

@Override
public int hashCode() {
return Objects.hash(policy, headers);
return Objects.hash(policy, headers, version, creationDate);
}

@Override
Expand All @@ -97,7 +131,9 @@ public boolean equals(Object obj) {
}
LifecyclePolicyMetadata other = (LifecyclePolicyMetadata) obj;
return Objects.equals(policy, other.policy) &&
Objects.equals(headers, other.headers);
Objects.equals(headers, other.headers) &&
Objects.equals(version, other.version) &&
Objects.equals(creationDate, other.creationDate);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.action.support.master.AcknowledgedRequest;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ToXContentObject;
Expand All @@ -19,7 +20,7 @@

import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Objects;

public class GetLifecycleAction extends Action<GetLifecycleAction.Response> {
Expand All @@ -37,42 +38,49 @@ public Response newResponse() {

public static class Response extends ActionResponse implements ToXContentObject {

private List<LifecyclePolicy> policies;
private Map<LifecyclePolicy, Tuple<Long, String>> policiesToMetadata;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should make this nicer here. Having a map is not nice because the key is not something you are realistically going to to use to lookup things and Tuples are generally not particularly nice to consume. Can we create a wrapper class around the policy, date and version so we have an object that is easier to consume both for us internally and for users of the transport API (given this class will be exposed to the Transport API)?


public Response() {
}

public Response(List<LifecyclePolicy> policies) {
this.policies = policies;
public Response(Map<LifecyclePolicy, Tuple<Long, String>> policiesToMetadata) {
this.policiesToMetadata = policiesToMetadata;
}

public List<LifecyclePolicy> getPolicies() {
return policies;
public Map<LifecyclePolicy, Tuple<Long, String>> getPoliciesToMetadata() {
return policiesToMetadata;
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
for (LifecyclePolicy policy : policies) {
builder.field(policy.getName(), policy);
for (Map.Entry<LifecyclePolicy, Tuple<Long, String>> entry : policiesToMetadata.entrySet()) {
builder.startObject(entry.getKey().getName());
builder.field("version", entry.getValue().v1());
builder.field("creation_date", entry.getValue().v2());
builder.field("policy", entry.getKey());
builder.endObject();
}
builder.endObject();
return builder;
}

@Override
public void readFrom(StreamInput in) throws IOException {
policies = in.readList(LifecyclePolicy::new);
policiesToMetadata = in.readMap(LifecyclePolicy::new, (inn) -> new Tuple<>(inn.readVLong(), inn.readString()));
}

@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeList(policies);
out.writeMap(policiesToMetadata, (o, policy) -> policy.writeTo(o), (o, tuple) -> {
o.writeVLong(tuple.v1());
o.writeString(tuple.v2());
});
}

@Override
public int hashCode() {
return Objects.hash(policies);
return Objects.hash(policiesToMetadata);
}

@Override
Expand All @@ -84,7 +92,7 @@ public boolean equals(Object obj) {
return false;
}
Response other = (Response) obj;
return Objects.equals(policies, other.policies);
return Objects.equals(policiesToMetadata, other.policiesToMetadata);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ protected LifecyclePolicyMetadata createTestInstance() {
for (int i = 0; i < numberHeaders; i++) {
headers.put(randomAlphaOfLength(10), randomAlphaOfLength(10));
}
return new LifecyclePolicyMetadata(LifecyclePolicyTests.randomTimeseriesLifecyclePolicy(lifecycleName), headers);
return new LifecyclePolicyMetadata(LifecyclePolicyTests.randomTimeseriesLifecyclePolicy(lifecycleName), headers,
randomNonNegativeLong(), randomNonNegativeLong());
}

@Override
Expand All @@ -87,7 +88,9 @@ protected Reader<LifecyclePolicyMetadata> instanceReader() {
protected LifecyclePolicyMetadata mutateInstance(LifecyclePolicyMetadata instance) throws IOException {
LifecyclePolicy policy = instance.getPolicy();
Map<String, String> headers = instance.getHeaders();
switch (between(0, 1)) {
long version = instance.getVersion();
long creationDate = instance.getCreationDate();
switch (between(0, 3)) {
case 0:
policy = new LifecyclePolicy(TimeseriesLifecycleType.INSTANCE, policy.getName() + randomAlphaOfLengthBetween(1, 5),
policy.getPhases());
Expand All @@ -96,10 +99,16 @@ protected LifecyclePolicyMetadata mutateInstance(LifecyclePolicyMetadata instanc
headers = new HashMap<>(headers);
headers.put(randomAlphaOfLength(11), randomAlphaOfLength(11));
break;
case 2:
version++;
break;
case 3:
creationDate++;
break;
default:
throw new AssertionError("Illegal randomisation branch");
}
return new LifecyclePolicyMetadata(policy, headers);
return new LifecyclePolicyMetadata(policy, headers, version, creationDate);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.core.indexlifecycle.action;

import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.test.AbstractStreamableTestCase;
import org.elasticsearch.xpack.core.indexlifecycle.LifecycleAction;
Expand All @@ -14,9 +15,9 @@
import org.elasticsearch.xpack.core.indexlifecycle.TestLifecycleType;
import org.elasticsearch.xpack.core.indexlifecycle.action.GetLifecycleAction.Response;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.HashMap;
import java.util.Map;

import static org.elasticsearch.xpack.core.indexlifecycle.LifecyclePolicyTests.randomTestLifecyclePolicy;

Expand All @@ -25,11 +26,12 @@ public class GetLifecycleResponseTests extends AbstractStreamableTestCase<GetLif
@Override
protected Response createTestInstance() {
String randomPrefix = randomAlphaOfLength(5);
List<LifecyclePolicy> policies = new ArrayList<>();
Map<LifecyclePolicy, Tuple<Long, String>> policyMap = new HashMap<>();
for (int i = 0; i < randomIntBetween(0, 2); i++) {
policies.add(randomTestLifecyclePolicy(randomPrefix + i));
policyMap.put(randomTestLifecyclePolicy(randomPrefix + i),
new Tuple<>(randomNonNegativeLong(), randomAlphaOfLength(8)));
}
return new Response(policies);
return new Response(policyMap);
}

@Override
Expand All @@ -45,16 +47,17 @@ protected NamedWriteableRegistry getNamedWriteableRegistry() {

@Override
protected Response mutateInstance(Response response) {
List<LifecyclePolicy> policies = new ArrayList<>(response.getPolicies());
if (policies.size() > 0) {
Map<LifecyclePolicy, Tuple<Long, String>> policyMap = new HashMap<>(response.getPoliciesToMetadata());
if (policyMap.size() > 0) {
if (randomBoolean()) {
policies.add(randomTestLifecyclePolicy(randomAlphaOfLength(5)));
policyMap.put(randomTestLifecyclePolicy(randomAlphaOfLength(5)),
new Tuple<>(randomNonNegativeLong(), randomAlphaOfLength(4)));
} else {
policies.remove(policies.size() - 1);
policyMap.remove(randomFrom(response.getPoliciesToMetadata().keySet()));
}
} else {
policies.add(randomTestLifecyclePolicy(randomAlphaOfLength(2)));
policyMap.put(randomTestLifecyclePolicy(randomAlphaOfLength(2)), new Tuple<>(randomNonNegativeLong(), randomAlphaOfLength(4)));
}
return new Response(policies);
return new Response(policyMap);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,23 @@
import org.elasticsearch.cluster.block.ClusterBlockLevel;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.core.indexlifecycle.IndexLifecycleMetadata;
import org.elasticsearch.xpack.core.indexlifecycle.LifecyclePolicy;
import org.elasticsearch.xpack.core.indexlifecycle.LifecyclePolicyMetadata;
import org.elasticsearch.xpack.core.indexlifecycle.action.GetLifecycleAction;
import org.elasticsearch.xpack.core.indexlifecycle.action.GetLifecycleAction.Request;
import org.elasticsearch.xpack.core.indexlifecycle.action.GetLifecycleAction.Response;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class TransportGetLifecycleAction extends TransportMasterNodeAction<Request, Response> {

Expand All @@ -49,32 +53,37 @@ protected Response newResponse() {
}

@Override
protected void masterOperation(Request request, ClusterState state, ActionListener<Response> listener) throws Exception {
protected void masterOperation(Request request, ClusterState state, ActionListener<Response> listener) {
IndexLifecycleMetadata metadata = clusterService.state().metaData().custom(IndexLifecycleMetadata.TYPE);
if (metadata == null) {
listener.onFailure(new ResourceNotFoundException("Lifecycle policy not found: {}", Arrays.toString(request.getPolicyNames())));
} else {
List<LifecyclePolicy> requestedPolicies;
List<LifecyclePolicyMetadata> requestedPolicies;
// if no policies explicitly provided, behave as if `*` was specified
if (request.getPolicyNames().length == 0) {
requestedPolicies = new ArrayList<>(metadata.getPolicies().values());
requestedPolicies = new ArrayList<>(metadata.getPolicyMetadatas().values());
} else {
requestedPolicies = new ArrayList<>(request.getPolicyNames().length);
for (String name : request.getPolicyNames()) {
LifecyclePolicy policy = metadata.getPolicies().get(name);
if (policy == null) {
LifecyclePolicyMetadata policyMetadata = metadata.getPolicyMetadatas().get(name);
if (policyMetadata == null) {
listener.onFailure(new ResourceNotFoundException("Lifecycle policy not found: {}", name));
return;
}
requestedPolicies.add(policy);
requestedPolicies.add(policyMetadata);
}
}
listener.onResponse(new Response(requestedPolicies));
Map<LifecyclePolicy, Tuple<Long, String>> policyMap = new HashMap<>(requestedPolicies.size());
for (LifecyclePolicyMetadata policyMetadata : requestedPolicies) {
policyMap.put(policyMetadata.getPolicy(),
new Tuple<>(policyMetadata.getVersion(), policyMetadata.getCreationDateString()));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not create the list of LifecyclePolicyResponseItem directly rather than first creating the list of LifecyclePolicyMetadata?

listener.onResponse(new Response(policyMap));
}
}

@Override
protected ClusterBlockException checkBlock(Request request, ClusterState state) {
return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_WRITE);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.xpack.core.indexlifecycle.action.PutLifecycleAction.Request;
import org.elasticsearch.xpack.core.indexlifecycle.action.PutLifecycleAction.Response;

import java.time.Instant;
import java.util.Map;
import java.util.SortedMap;
import java.util.TreeMap;
Expand Down Expand Up @@ -79,9 +80,12 @@ public ClusterState execute(ClusterState currentState) throws Exception {
if (currentMetadata == null) { // first time using index-lifecycle feature, bootstrap metadata
currentMetadata = IndexLifecycleMetadata.EMPTY;
}
// NORELEASE Check if current step exists in new policy and if not move to next available step
LifecyclePolicyMetadata existingPolicyMetadata = currentMetadata.getPolicyMetadatas()
.get(request.getPolicy().getName());
long nextVersion = (existingPolicyMetadata == null) ? 1L : existingPolicyMetadata.getVersion() + 1L;
SortedMap<String, LifecyclePolicyMetadata> newPolicies = new TreeMap<>(currentMetadata.getPolicyMetadatas());
LifecyclePolicyMetadata lifecyclePolicyMetadata = new LifecyclePolicyMetadata(request.getPolicy(), filteredHeaders);
LifecyclePolicyMetadata lifecyclePolicyMetadata = new LifecyclePolicyMetadata(request.getPolicy(), filteredHeaders,
nextVersion, Instant.now().toEpochMilli());
newPolicies.put(lifecyclePolicyMetadata.getName(), lifecyclePolicyMetadata);
IndexLifecycleMetadata newMetadata = new IndexLifecycleMetadata(newPolicies, OperationMode.RUNNING);
newState.metaData(MetaData.builder(currentState.getMetaData())
Expand Down
Loading