Skip to content
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

[Data stream lifecycle] Conditionally display effective retention #107964

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -16,6 +16,7 @@
import org.elasticsearch.action.support.master.MasterNodeReadRequest;
import org.elasticsearch.cluster.metadata.ComponentTemplate;
import org.elasticsearch.cluster.metadata.DataStreamGlobalRetention;
import org.elasticsearch.cluster.metadata.DataStreamLifecycle;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.core.Nullable;
Expand Down Expand Up @@ -196,7 +197,13 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.startObject();
builder.field(NAME.getPreferredName(), componentTemplate.getKey());
builder.field(COMPONENT_TEMPLATE.getPreferredName());
componentTemplate.getValue().toXContent(builder, params, rolloverConfiguration, globalRetention);
componentTemplate.getValue()
.toXContent(
builder,
DataStreamLifecycle.maybeAddEffectiveRetentionParams(params),
rolloverConfiguration,
globalRetention
);
builder.endObject();
}
builder.endArray();
Expand Down
Expand Up @@ -16,6 +16,7 @@
import org.elasticsearch.action.support.master.MasterNodeReadRequest;
import org.elasticsearch.cluster.metadata.ComposableIndexTemplate;
import org.elasticsearch.cluster.metadata.DataStreamGlobalRetention;
import org.elasticsearch.cluster.metadata.DataStreamLifecycle;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.core.Nullable;
Expand Down Expand Up @@ -157,10 +158,6 @@ public Map<String, ComposableIndexTemplate> indexTemplates() {
return indexTemplates;
}

public RolloverConfiguration getRolloverConfiguration() {
return rolloverConfiguration;
}

public DataStreamGlobalRetention getGlobalRetention() {
return globalRetention;
}
Expand Down Expand Up @@ -199,7 +196,13 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.startObject();
builder.field(NAME.getPreferredName(), indexTemplate.getKey());
builder.field(INDEX_TEMPLATE.getPreferredName());
indexTemplate.getValue().toXContent(builder, params, rolloverConfiguration, globalRetention);
indexTemplate.getValue()
.toXContent(
builder,
DataStreamLifecycle.maybeAddEffectiveRetentionParams(params),
rolloverConfiguration,
globalRetention
);
builder.endObject();
}
builder.endArray();
Expand Down
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.action.admin.indices.rollover.RolloverConfiguration;
import org.elasticsearch.cluster.metadata.DataStreamGlobalRetention;
import org.elasticsearch.cluster.metadata.DataStreamLifecycle;
import org.elasticsearch.cluster.metadata.Template;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
Expand Down Expand Up @@ -69,22 +70,10 @@ public SimulateIndexTemplateResponse(
this.globalRetention = globalRetention;
}

public Template getResolvedTemplate() {
return resolvedTemplate;
}

public Map<String, List<String>> getOverlappingTemplates() {
return overlappingTemplates;
}

public RolloverConfiguration getRolloverConfiguration() {
return rolloverConfiguration;
}

public DataStreamGlobalRetention getGlobalRetention() {
return globalRetention;
}

public SimulateIndexTemplateResponse(StreamInput in) throws IOException {
super(in);
resolvedTemplate = in.readOptionalWriteable(Template::new);
Expand Down Expand Up @@ -132,7 +121,12 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.startObject();
if (this.resolvedTemplate != null) {
builder.field(TEMPLATE.getPreferredName());
this.resolvedTemplate.toXContent(builder, params, rolloverConfiguration, globalRetention);
this.resolvedTemplate.toXContent(
builder,
DataStreamLifecycle.maybeAddEffectiveRetentionParams(params),
rolloverConfiguration,
globalRetention
);
}
if (this.overlappingTemplates != null) {
builder.startArray(OVERLAPPING.getPreferredName());
Expand Down
Expand Up @@ -20,6 +20,7 @@
import org.elasticsearch.cluster.metadata.DataStream;
import org.elasticsearch.cluster.metadata.DataStreamAutoShardingEvent;
import org.elasticsearch.cluster.metadata.DataStreamGlobalRetention;
import org.elasticsearch.cluster.metadata.DataStreamLifecycle;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
Expand Down Expand Up @@ -546,7 +547,12 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.startObject();
builder.startArray(DATA_STREAMS_FIELD.getPreferredName());
for (DataStreamInfo dataStream : dataStreams) {
dataStream.toXContent(builder, params, rolloverConfiguration, globalRetention);
dataStream.toXContent(
builder,
DataStreamLifecycle.maybeAddEffectiveRetentionParams(params),
rolloverConfiguration,
globalRetention
);
}
builder.endArray();
builder.endObject();
Expand Down
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.master.MasterNodeReadRequest;
import org.elasticsearch.cluster.metadata.DataStreamGlobalRetention;
import org.elasticsearch.cluster.metadata.DataStreamLifecycle;
import org.elasticsearch.common.collect.Iterators;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
Expand Down Expand Up @@ -212,7 +213,12 @@ public Iterator<? extends ToXContent> toXContentChunked(ToXContent.Params outerP
return builder;
}), Iterators.map(indices.iterator(), explainIndexDataLifecycle -> (builder, params) -> {
builder.field(explainIndexDataLifecycle.getIndex());
explainIndexDataLifecycle.toXContent(builder, outerParams, rolloverConfiguration, globalRetention);
explainIndexDataLifecycle.toXContent(
builder,
DataStreamLifecycle.maybeAddEffectiveRetentionParams(outerParams),
rolloverConfiguration,
globalRetention
);
return builder;
}), Iterators.single((builder, params) -> {
builder.endObject();
Expand Down
Expand Up @@ -174,7 +174,12 @@ public XContentBuilder toXContent(
builder.field(NAME_FIELD.getPreferredName(), dataStreamName);
if (lifecycle != null) {
builder.field(LIFECYCLE_FIELD.getPreferredName());
lifecycle.toXContent(builder, params, rolloverConfiguration, globalRetention);
lifecycle.toXContent(
builder,
org.elasticsearch.cluster.metadata.DataStreamLifecycle.maybeAddEffectiveRetentionParams(params),
rolloverConfiguration,
globalRetention
);
}
builder.endObject();
return builder;
Expand Down
Expand Up @@ -24,11 +24,13 @@
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval;
import org.elasticsearch.xcontent.AbstractObjectParser;
import org.elasticsearch.xcontent.ConstructingObjectParser;
import org.elasticsearch.xcontent.ObjectParser;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.ToXContent;
import org.elasticsearch.xcontent.ToXContentFragment;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;
Expand Down Expand Up @@ -355,6 +357,17 @@ public static DataStreamLifecycle fromXContent(XContentParser parser) throws IOE
return PARSER.parse(parser, null);
}

/**
* Adds a retention param to signal that this serialisation should include the effective retention metadata
*/
public static ToXContent.Params maybeAddEffectiveRetentionParams(ToXContent.Params params) {
boolean shouldAddEffectiveRetention = Objects.equals(params.param(RestRequest.PATH_RESTRICTED), "serverless");
return new DelegatingMapParams(
Copy link
Member

Choose a reason for hiding this comment

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

You told me this when we spoke offline and it made sense, but now I can't remember -- why can't we just check PATH_RESTRICTED in toXContent(), and not worry about remembering to call maybeAddEffectiveRetentionParams?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem it's a lot to keep in mind. We want only the specific APIs to add the effective retention to their response, at the DataStreamLifecycle level we do not know who called us and probably multiple places set the PATH_RESTRICTED. So, we are trying to ensure we do not expose this more than we should by explicitly choosing when to set the maybeAddEffectiveRetentionParams.

Is this clear now?

Map.of(INCLUDE_EFFECTIVE_RETENTION_PARAM_NAME, Boolean.toString(shouldAddEffectiveRetention)),
params
);
}

public static Builder newBuilder(DataStreamLifecycle lifecycle) {
return new Builder().dataRetention(lifecycle.getDataRetention())
.downsampling(lifecycle.getDownsampling())
Expand Down
Expand Up @@ -31,13 +31,15 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;

import static org.elasticsearch.cluster.metadata.DataStreamLifecycle.RetentionSource.DATA_STREAM_CONFIGURATION;
import static org.elasticsearch.cluster.metadata.DataStreamLifecycle.RetentionSource.DEFAULT_GLOBAL_RETENTION;
import static org.elasticsearch.cluster.metadata.DataStreamLifecycle.RetentionSource.MAX_GLOBAL_RETENTION;
import static org.elasticsearch.rest.RestRequest.PATH_RESTRICTED;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
Expand Down Expand Up @@ -343,6 +345,25 @@ public void testEffectiveRetention() {
}
}

public void testEffectiveRetentionParams() {
{
ToXContent.Params params = DataStreamLifecycle.maybeAddEffectiveRetentionParams(new ToXContent.MapParams(Map.of()));
assertThat(params.paramAsBoolean(DataStreamLifecycle.INCLUDE_EFFECTIVE_RETENTION_PARAM_NAME, false), equalTo(false));
}
{
ToXContent.Params params = DataStreamLifecycle.maybeAddEffectiveRetentionParams(
new ToXContent.MapParams(Map.of(PATH_RESTRICTED, "not-serverless"))
);
assertThat(params.paramAsBoolean(DataStreamLifecycle.INCLUDE_EFFECTIVE_RETENTION_PARAM_NAME, false), equalTo(false));
}
{
ToXContent.Params params = DataStreamLifecycle.maybeAddEffectiveRetentionParams(
new ToXContent.MapParams(Map.of(PATH_RESTRICTED, "serverless"))
);
assertThat(params.paramAsBoolean(DataStreamLifecycle.INCLUDE_EFFECTIVE_RETENTION_PARAM_NAME, false), equalTo(true));
}
}

@Nullable
public static DataStreamLifecycle randomLifecycle() {
return DataStreamLifecycle.newBuilder()
Expand Down