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.

Sign up for GitHub

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

Jump to bottom

[7.x Backport] Force selection of calendar or fixed intervals #41906

Merged
merged 9 commits into from
May 20, 2019
Merged

[7.x Backport] Force selection of calendar or fixed intervals #41906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3d86288
Force selection of calendar or fixed intervals in date histo agg (#33…
polyfractal May 6, 2019
fe5a9fb
Update version skips
polyfractal May 6, 2019
2f1dc24
Update movavg doc tests
polyfractal May 7, 2019
0787658
Tweak rolling restart test for 6x, 7x
polyfractal May 8, 2019
fa7e1b6
Merge remote-tracking branch 'origin/7.x' into date_histo_explicit_in…
polyfractal May 8, 2019
b8e622d
Throw exception if legacy interval cannot be parsed
polyfractal May 8, 2019
7049882
Fix max_bucket test by disallowing partial results (#41959)
polyfractal May 9, 2019
9e56889
Add documentation for calendar/fixed intervals (#41919)
polyfractal May 10, 2019
b60737a
Merge remote-tracking branch 'origin/7.x' into date_histo_explicit_in…
polyfractal May 20, 2019
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
137 changes: 125 additions & 12 deletions ...el/src/main/java/org/elasticsearch/client/rollup/job/config/DateHistogramGroupConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.elasticsearch.client.ValidationException;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
Expand All @@ -30,8 +31,11 @@
import org.joda.time.DateTimeZone;

import java.io.IOException;
import java.util.Collections;
import java.util.HashSet;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;

import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg;
Expand Down Expand Up @@ -59,14 +63,63 @@ public class DateHistogramGroupConfig implements Validatable, ToXContentObject {
private static final String TIME_ZONE = "time_zone";
private static final String DELAY = "delay";
private static final String DEFAULT_TIMEZONE = "UTC";
private static final String CALENDAR_INTERVAL = "calendar_interval";
private static final String FIXED_INTERVAL = "fixed_interval";

// From DateHistogramAggregationBuilder in core, transplanted and modified to a set
// so we don't need to import a dependency on the class
private static final Set<String> DATE_FIELD_UNITS;
static {
Set<String> dateFieldUnits = new HashSet<>();
dateFieldUnits.add("year");
dateFieldUnits.add("1y");
dateFieldUnits.add("quarter");
dateFieldUnits.add("1q");
dateFieldUnits.add("month");
dateFieldUnits.add("1M");
dateFieldUnits.add("week");
dateFieldUnits.add("1w");
dateFieldUnits.add("day");
dateFieldUnits.add("1d");
dateFieldUnits.add("hour");
dateFieldUnits.add("1h");
dateFieldUnits.add("minute");
dateFieldUnits.add("1m");
dateFieldUnits.add("second");
dateFieldUnits.add("1s");
DATE_FIELD_UNITS = Collections.unmodifiableSet(dateFieldUnits);
}

private static final ConstructingObjectParser<DateHistogramGroupConfig, Void> PARSER;
static {
PARSER = new ConstructingObjectParser<>(NAME, true, a ->
new DateHistogramGroupConfig((String) a[0], (DateHistogramInterval) a[1], (DateHistogramInterval) a[2], (String) a[3]));
PARSER = new ConstructingObjectParser<>(NAME, true, a -> {
DateHistogramInterval oldInterval = (DateHistogramInterval) a[1];
DateHistogramInterval calendarInterval = (DateHistogramInterval) a[2];
DateHistogramInterval fixedInterval = (DateHistogramInterval) a[3];

if (oldInterval != null) {
if (calendarInterval != null || fixedInterval != null) {
throw new IllegalArgumentException("Cannot use [interval] with [fixed_interval] or [calendar_interval] " +
"configuration options.");
}
return new DateHistogramGroupConfig((String) a[0], oldInterval, (DateHistogramInterval) a[4], (String) a[5]);
} else if (calendarInterval != null && fixedInterval == null) {
return new CalendarInterval((String) a[0], calendarInterval, (DateHistogramInterval) a[4], (String) a[5]);
} else if (calendarInterval == null && fixedInterval != null) {
return new FixedInterval((String) a[0], fixedInterval, (DateHistogramInterval) a[4], (String) a[5]);
} else if (calendarInterval != null && fixedInterval != null) {
throw new IllegalArgumentException("Cannot set both [fixed_interval] and [calendar_interval] at the same time");
} else {
throw new IllegalArgumentException("An interval is required. Use [fixed_interval] or [calendar_interval].");
}
});
PARSER.declareString(constructorArg(), new ParseField(FIELD));
PARSER.declareField(constructorArg(), p -> new DateHistogramInterval(p.text()), new ParseField(INTERVAL), ValueType.STRING);
PARSER.declareField(optionalConstructorArg(), p -> new DateHistogramInterval(p.text()), new ParseField(DELAY), ValueType.STRING);
PARSER.declareField(optionalConstructorArg(), p -> new DateHistogramInterval(p.text()), new ParseField(INTERVAL), ValueType.STRING);
PARSER.declareField(optionalConstructorArg(), p -> new DateHistogramInterval(p.text()),
new ParseField(CALENDAR_INTERVAL), ValueType.STRING);
PARSER.declareField(optionalConstructorArg(), p -> new DateHistogramInterval(p.text()),
new ParseField(FIXED_INTERVAL), ValueType.STRING);
PARSER.declareField(optionalConstructorArg(), p -> new DateHistogramInterval(p.text()), new ParseField(DELAY), ValueType.STRING);
PARSER.declareString(optionalConstructorArg(), new ParseField(TIME_ZONE));
}

Expand All @@ -75,27 +128,81 @@ public class DateHistogramGroupConfig implements Validatable, ToXContentObject {
private final DateHistogramInterval delay;
private final String timeZone;

/**
* FixedInterval is a {@link DateHistogramGroupConfig} that uses a fixed time interval for rolling up data.
* The fixed time interval is one or multiples of SI units and has no calendar-awareness (e.g. doesn't account
* for leap corrections, does not have variable length months, etc).
*
* For calendar-aware rollups, use {@link CalendarInterval}
*/
public static class FixedInterval extends DateHistogramGroupConfig {
public FixedInterval(String field, DateHistogramInterval interval) {
this(field, interval, null, null);
}

public FixedInterval(String field, DateHistogramInterval interval, DateHistogramInterval delay, String timeZone) {
super(field, interval, delay, timeZone);
// validate fixed time
TimeValue.parseTimeValue(interval.toString(), NAME + ".FixedInterval");
}
}

/**
* CalendarInterval is a {@link DateHistogramGroupConfig} that uses calendar-aware intervals for rolling up data.
* Calendar time intervals understand leap corrections and contextual differences in certain calendar units (e.g.
* months are variable length depending on the month). Calendar units are only available in singular quantities:
* 1s, 1m, 1h, 1d, 1w, 1q, 1M, 1y
*
* For fixed time rollups, use {@link FixedInterval}
*/
public static class CalendarInterval extends DateHistogramGroupConfig {
public CalendarInterval(String field, DateHistogramInterval interval) {
this(field, interval, null, null);

}

public CalendarInterval(String field, DateHistogramInterval interval, DateHistogramInterval delay, String timeZone) {
super(field, interval, delay, timeZone);
if (DATE_FIELD_UNITS.contains(interval.toString()) == false) {
throw new IllegalArgumentException("The supplied interval [" + interval +"] could not be parsed " +
"as a calendar interval.");
}
}

}

/**
* Create a new {@link DateHistogramGroupConfig} using the given field and interval parameters.
*
* @deprecated Build a DateHistoConfig using {@link DateHistogramGroupConfig.CalendarInterval}
* or {@link DateHistogramGroupConfig.FixedInterval} instead
*
* @since 7.2.0
*/
@Deprecated
public DateHistogramGroupConfig(final String field, final DateHistogramInterval interval) {
this(field, interval, null, null);
}

/**
* Create a new {@link DateHistogramGroupConfig} using the given configuration parameters.
* <p>
* The {@code field} and {@code interval} are required to compute the date histogram for the rolled up documents.
* The {@code delay} is optional and can be set to {@code null}. It defines how long to wait before rolling up new documents.
* The {@code timeZone} is optional and can be set to {@code null}. When configured, the time zone value is resolved using
* ({@link DateTimeZone#forID(String)} and must match a time zone identifier provided by the Joda Time library.
* The {@code field} and {@code interval} are required to compute the date histogram for the rolled up documents.
* The {@code delay} is optional and can be set to {@code null}. It defines how long to wait before rolling up new documents.
* The {@code timeZone} is optional and can be set to {@code null}. When configured, the time zone value is resolved using
* ({@link DateTimeZone#forID(String)} and must match a time zone identifier provided by the Joda Time library.
* </p>
*
* @param field the name of the date field to use for the date histogram (required)
* @param field the name of the date field to use for the date histogram (required)
* @param interval the interval to use for the date histogram (required)
* @param delay the time delay (optional)
* @param delay the time delay (optional)
* @param timeZone the id of time zone to use to calculate the date histogram (optional). When {@code null}, the UTC timezone is used.
*
* @deprecated Build a DateHistoConfig using {@link DateHistogramGroupConfig.CalendarInterval}
* or {@link DateHistogramGroupConfig.FixedInterval} instead
*
* @since 7.2.0
*/
@Deprecated
public DateHistogramGroupConfig(final String field,
final DateHistogramInterval interval,
final @Nullable DateHistogramInterval delay,
Expand Down Expand Up @@ -153,7 +260,13 @@ public String getTimeZone() {
public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException {
builder.startObject();
{
builder.field(INTERVAL, interval.toString());
if (this.getClass().equals(CalendarInterval.class)) {
builder.field(CALENDAR_INTERVAL, interval.toString());
} else if (this.getClass().equals(FixedInterval.class)) {
builder.field(FIXED_INTERVAL, interval.toString());
} else {
builder.field(INTERVAL, interval.toString());
}
builder.field(FIELD, field);
if (delay != null) {
builder.field(DELAY, delay.toString());
Expand Down
12 changes: 6 additions & 6 deletions client/rest-high-level/src/test/java/org/elasticsearch/client/RollupIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public int indexDocs() throws Exception {


public void testDeleteRollupJob() throws Exception {
final GroupConfig groups = new GroupConfig(new DateHistogramGroupConfig("date", DateHistogramInterval.DAY));
final GroupConfig groups = new GroupConfig(new DateHistogramGroupConfig.CalendarInterval("date", DateHistogramInterval.DAY));
final List<MetricConfig> metrics = Collections.singletonList(new MetricConfig("value", SUPPORTED_METRICS));
final TimeValue timeout = TimeValue.timeValueSeconds(randomIntBetween(30, 600));
PutRollupJobRequest putRollupJobRequest =
Expand All @@ -174,7 +174,7 @@ public void testDeleteMissingRollupJob() {

public void testPutStartAndGetRollupJob() throws Exception {
// TODO expand this to also test with histogram and terms?
final GroupConfig groups = new GroupConfig(new DateHistogramGroupConfig("date", DateHistogramInterval.DAY));
final GroupConfig groups = new GroupConfig(new DateHistogramGroupConfig.CalendarInterval("date", DateHistogramInterval.DAY));
final List<MetricConfig> metrics = Collections.singletonList(new MetricConfig("value", SUPPORTED_METRICS));
final TimeValue timeout = TimeValue.timeValueSeconds(randomIntBetween(30, 600));

Expand Down Expand Up @@ -334,7 +334,7 @@ public void testGetRollupCaps() throws Exception {
final String cron = "*/1 * * * * ?";
final int pageSize = randomIntBetween(numDocs, numDocs * 10);
// TODO expand this to also test with histogram and terms?
final GroupConfig groups = new GroupConfig(new DateHistogramGroupConfig("date", DateHistogramInterval.DAY));
final GroupConfig groups = new GroupConfig(new DateHistogramGroupConfig.CalendarInterval("date", DateHistogramInterval.DAY));
final List<MetricConfig> metrics = Collections.singletonList(new MetricConfig("value", SUPPORTED_METRICS));
final TimeValue timeout = TimeValue.timeValueSeconds(randomIntBetween(30, 600));

Expand Down Expand Up @@ -378,7 +378,7 @@ public void testGetRollupCaps() throws Exception {
case "delay":
assertThat(entry.getValue(), equalTo("foo"));
break;
case "interval":
case "calendar_interval":
assertThat(entry.getValue(), equalTo("1d"));
break;
case "time_zone":
Expand Down Expand Up @@ -446,7 +446,7 @@ public void testGetRollupIndexCaps() throws Exception {
final String cron = "*/1 * * * * ?";
final int pageSize = randomIntBetween(numDocs, numDocs * 10);
// TODO expand this to also test with histogram and terms?
final GroupConfig groups = new GroupConfig(new DateHistogramGroupConfig("date", DateHistogramInterval.DAY));
final GroupConfig groups = new GroupConfig(new DateHistogramGroupConfig.CalendarInterval("date", DateHistogramInterval.DAY));
final List<MetricConfig> metrics = Collections.singletonList(new MetricConfig("value", SUPPORTED_METRICS));
final TimeValue timeout = TimeValue.timeValueSeconds(randomIntBetween(30, 600));

Expand Down Expand Up @@ -490,7 +490,7 @@ public void testGetRollupIndexCaps() throws Exception {
case "delay":
assertThat(entry.getValue(), equalTo("foo"));
break;
case "interval":
case "calendar_interval":
assertThat(entry.getValue(), equalTo("1d"));
break;
case "time_zone":
Expand Down
14 changes: 8 additions & 6 deletions ...igh-level/src/test/java/org/elasticsearch/client/documentation/RollupDocumentationIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,8 +399,8 @@ public void onFailure(Exception e) {
public void testGetRollupCaps() throws Exception {
RestHighLevelClient client = highLevelClient();

DateHistogramGroupConfig dateHistogram =
new DateHistogramGroupConfig("timestamp", DateHistogramInterval.HOUR, new DateHistogramInterval("7d"), "UTC"); // <1>
DateHistogramGroupConfig dateHistogram = new DateHistogramGroupConfig.FixedInterval(
"timestamp", DateHistogramInterval.HOUR, new DateHistogramInterval("7d"), "UTC"); // <1>
TermsGroupConfig terms = new TermsGroupConfig("hostname", "datacenter");
HistogramGroupConfig histogram = new HistogramGroupConfig(5L, "load", "net_in", "net_out");
GroupConfig groups = new GroupConfig(dateHistogram, histogram, terms);
Expand Down Expand Up @@ -473,7 +473,8 @@ public void testGetRollupCaps() throws Exception {
// item represents a different aggregation that can be run against the "timestamp"
// field, and any additional details specific to that agg (interval, etc)
List<Map<String, Object>> timestampCaps = fieldCaps.get("timestamp").getAggs();
assert timestampCaps.get(0).toString().equals("{agg=date_histogram, delay=7d, interval=1h, time_zone=UTC}");
logger.error(timestampCaps.get(0).toString());
assert timestampCaps.get(0).toString().equals("{agg=date_histogram, fixed_interval=1h, delay=7d, time_zone=UTC}");

// In contrast to the timestamp field, the temperature field has multiple aggs configured
List<Map<String, Object>> temperatureCaps = fieldCaps.get("temperature").getAggs();
Expand Down Expand Up @@ -515,8 +516,8 @@ public void onFailure(Exception e) {
public void testGetRollupIndexCaps() throws Exception {
RestHighLevelClient client = highLevelClient();

DateHistogramGroupConfig dateHistogram =
new DateHistogramGroupConfig("timestamp", DateHistogramInterval.HOUR, new DateHistogramInterval("7d"), "UTC"); // <1>
DateHistogramGroupConfig dateHistogram = new DateHistogramGroupConfig.FixedInterval(
"timestamp", DateHistogramInterval.HOUR, new DateHistogramInterval("7d"), "UTC"); // <1>
TermsGroupConfig terms = new TermsGroupConfig("hostname", "datacenter");
HistogramGroupConfig histogram = new HistogramGroupConfig(5L, "load", "net_in", "net_out");
GroupConfig groups = new GroupConfig(dateHistogram, histogram, terms);
Expand Down Expand Up @@ -587,7 +588,8 @@ public void testGetRollupIndexCaps() throws Exception {
// item represents a different aggregation that can be run against the "timestamp"
// field, and any additional details specific to that agg (interval, etc)
List<Map<String, Object>> timestampCaps = fieldCaps.get("timestamp").getAggs();
assert timestampCaps.get(0).toString().equals("{agg=date_histogram, delay=7d, interval=1h, time_zone=UTC}");
logger.error(timestampCaps.get(0).toString());
assert timestampCaps.get(0).toString().equals("{agg=date_histogram, fixed_interval=1h, delay=7d, time_zone=UTC}");

// In contrast to the timestamp field, the temperature field has multiple aggs configured
List<Map<String, Object>> temperatureCaps = fieldCaps.get("temperature").getAggs();
Expand Down
3 changes: 2 additions & 1 deletion ...st-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedConfigTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.search.aggregations.AggregationBuilders;
import org.elasticsearch.search.aggregations.AggregatorFactories;
import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval;
import org.elasticsearch.search.aggregations.metrics.MaxAggregationBuilder;
import org.elasticsearch.search.builder.SearchSourceBuilder.ScriptField;
import org.elasticsearch.test.AbstractXContentTestCase;
Expand Down Expand Up @@ -79,7 +80,7 @@ public static DatafeedConfig.Builder createRandomBuilder() {
aggHistogramInterval = aggHistogramInterval <= 0 ? 1 : aggHistogramInterval;
MaxAggregationBuilder maxTime = AggregationBuilders.max("time").field("time");
aggs.addAggregator(AggregationBuilders.dateHistogram("buckets")
.interval(aggHistogramInterval).subAggregation(maxTime).field("time"));
.fixedInterval(new DateHistogramInterval(aggHistogramInterval + "ms")).subAggregation(maxTime).field("time"));
try {
builder.setAggregations(aggs);
} catch (IOException e) {
Expand Down
2 changes: 1 addition & 1 deletion ...t-high-level/src/test/java/org/elasticsearch/client/rollup/GetRollupJobResponseTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void testFromXContent() throws IOException {
this::createTestInstance,
this::toXContent,
GetRollupJobResponse::fromXContent)
.supportsUnknownFields(true)
.supportsUnknownFields(false)
.randomFieldsExcludeFilter(field ->
field.endsWith("status.current_position"))
.test();
Expand Down
2 changes: 1 addition & 1 deletion ...st-high-level/src/test/java/org/elasticsearch/client/rollup/PutRollupJobRequestTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ protected PutRollupJobRequest doParseInstance(final XContentParser parser) throw

@Override
protected boolean supportsUnknownFields() {
return true;
return false;
}

public void testRequireConfiguration() {
Expand Down
Loading