honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-02-17 10:25:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

[Rollup] Fix Caps Comparator to handle calendar/fixed time (#33336)

Browse Source 
The comparator used TimeValue parsing, which meant it couldn't handle
calendar time.  This fixes the comparator to handle either (and potentially
mixed).  The mixing shouldn't be an issue since the validation code
upstream will prevent it, but was simplest to allow the comparator
to handle both.
This commit is contained in:
Zachary Tong 2018-09-03 04:49:19 -04:00 committed by Jim Ferenczi
parent 9310d2eaf3
commit 90ce3a6224
3 changed files with 160 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfigSerializingTests.java
View File

@ -125,6 +125,20 @@ public class DateHistogramGroupConfigSerializingTests extends AbstractSerializin
assertThat(e.validationErrors().size(), equalTo(0));
}
public void testValidateWeek() {
ActionRequestValidationException e = new ActionRequestValidationException();
Map<String, Map<String, FieldCapabilities>> responseMap = new HashMap<>();
// Have to mock fieldcaps because the ctor's aren't public...
FieldCapabilities fieldCaps = mock(FieldCapabilities.class);
when(fieldCaps.isAggregatable()).thenReturn(true);
responseMap.put("my_field", Collections.singletonMap("date", fieldCaps));
DateHistogramGroupConfig config = new DateHistogramGroupConfig("my_field", new DateHistogramInterval("1w"), null, null);
config.validateMappings(responseMap, e);
assertThat(e.validationErrors().size(), equalTo(0));
}
/**
* Tests that a DateHistogramGroupConfig can be serialized/deserialized correctly after
* the timezone was changed from DateTimeZone to String.

60
x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtils.java
View File

@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.rollup;
import org.elasticsearch.common.rounding.DateTimeUnit;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.search.aggregations.AggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder;
@ -18,9 +19,7 @@ import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig;
import org.joda.time.DateTimeZone;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@ -33,30 +32,7 @@ import java.util.Set;
*/
public class RollupJobIdentifierUtils {
private static final Comparator<RollupJobCaps> COMPARATOR = RollupJobIdentifierUtils.getComparator();
public static final Map<String, Integer> CALENDAR_ORDERING;
static {
Map<String, Integer> dateFieldUnits = new HashMap<>(16);
dateFieldUnits.put("year", 8);
dateFieldUnits.put("1y", 8);
dateFieldUnits.put("quarter", 7);
dateFieldUnits.put("1q", 7);
dateFieldUnits.put("month", 6);
dateFieldUnits.put("1M", 6);
dateFieldUnits.put("week", 5);
dateFieldUnits.put("1w", 5);
dateFieldUnits.put("day", 4);
dateFieldUnits.put("1d", 4);
dateFieldUnits.put("hour", 3);
dateFieldUnits.put("1h", 3);
dateFieldUnits.put("minute", 2);
dateFieldUnits.put("1m", 2);
dateFieldUnits.put("second", 1);
dateFieldUnits.put("1s", 1);
CALENDAR_ORDERING = Collections.unmodifiableMap(dateFieldUnits);
}
static final Comparator<RollupJobCaps> COMPARATOR = RollupJobIdentifierUtils.getComparator();
/**
* Given the aggregation tree and a list of available job capabilities, this method will return a set
@ -176,8 +152,10 @@ public class RollupJobIdentifierUtils {
// The request must be gte the config. The CALENDAR_ORDERING map values are integers representing
// relative orders between the calendar units
int requestOrder = CALENDAR_ORDERING.getOrDefault(requestInterval.toString(), Integer.MAX_VALUE);
int configOrder = CALENDAR_ORDERING.getOrDefault(configInterval.toString(), Integer.MAX_VALUE);
DateTimeUnit requestUnit = DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(requestInterval.toString());
long requestOrder = requestUnit.field(DateTimeZone.UTC).getDurationField().getUnitMillis();
DateTimeUnit configUnit = DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(configInterval.toString());
long configOrder = configUnit.field(DateTimeZone.UTC).getDurationField().getUnitMillis();
// All calendar units are multiples naturally, so we just care about gte
return requestOrder >= configOrder;
@ -190,7 +168,7 @@ public class RollupJobIdentifierUtils {
return false;
}
// Both are fixed, good to conver to millis now
// Both are fixed, good to convert to millis now
long configIntervalMillis = TimeValue.parseTimeValue(configInterval.toString(),
"date_histo.config.interval").getMillis();
long requestIntervalMillis = TimeValue.parseTimeValue(requestInterval.toString(),
@ -326,8 +304,8 @@ public class RollupJobIdentifierUtils {
return 0;
}
TimeValue thisTime = null;
TimeValue thatTime = null;
long thisTime = Long.MAX_VALUE;
long thatTime = Long.MAX_VALUE;
// histogram intervals are averaged and compared, with the idea that
// a larger average == better, because it will generate fewer documents
@ -344,7 +322,7 @@ public class RollupJobIdentifierUtils {
for (RollupJobCaps.RollupFieldCaps fieldCaps : o1.getFieldCaps().values()) {
for (Map<String, Object> agg : fieldCaps.getAggs()) {
if (agg.get(RollupField.AGG).equals(DateHistogramAggregationBuilder.NAME)) {
thisTime = TimeValue.parseTimeValue((String) agg.get(RollupField.INTERVAL), RollupField.INTERVAL);
thisTime = getMillisFixedOrCalendar((String) agg.get(RollupField.INTERVAL));
} else if (agg.get(RollupField.AGG).equals(HistogramAggregationBuilder.NAME)) {
thisHistoWeights += (long) agg.get(RollupField.INTERVAL);
counter += 1;
@ -360,7 +338,7 @@ public class RollupJobIdentifierUtils {
for (RollupJobCaps.RollupFieldCaps fieldCaps : o2.getFieldCaps().values()) {
for (Map<String, Object> agg : fieldCaps.getAggs()) {
if (agg.get(RollupField.AGG).equals(DateHistogramAggregationBuilder.NAME)) {
thatTime = TimeValue.parseTimeValue((String) agg.get(RollupField.INTERVAL), RollupField.INTERVAL);
thatTime = getMillisFixedOrCalendar((String) agg.get(RollupField.INTERVAL));
} else if (agg.get(RollupField.AGG).equals(HistogramAggregationBuilder.NAME)) {
thatHistoWeights += (long) agg.get(RollupField.INTERVAL);
counter += 1;
@ -371,13 +349,9 @@ public class RollupJobIdentifierUtils {
}
thatHistoWeights = counter == 0 ? 0 : thatHistoWeights / counter;
// DateHistos are mandatory so these should always be present no matter what
assert thisTime != null;
assert thatTime != null;
// Compare on date interval first
// The "smaller" job is the one with the larger interval
int timeCompare = thisTime.compareTo(thatTime);
int timeCompare = Long.compare(thisTime, thatTime);
if (timeCompare != 0) {
return -timeCompare;
}
@ -409,4 +383,14 @@ public class RollupJobIdentifierUtils {
// coverage
};
}
static long getMillisFixedOrCalendar(String value) {
DateHistogramInterval interval = new DateHistogramInterval(value);
if (isCalendarInterval(interval)) {
DateTimeUnit intervalUnit = DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(interval.toString());
return intervalUnit.field(DateTimeZone.UTC).getDurationField().getUnitMillis();
} else {
return TimeValue.parseTimeValue(value, "date_histo.comparator.interval").getMillis();
}
}
}

124
x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/RollupJobIdentifierUtilTests.java
View File

@ -15,6 +15,7 @@ import org.elasticsearch.search.aggregations.metrics.min.MinAggregationBuilder;
import org.elasticsearch.search.aggregations.metrics.sum.SumAggregationBuilder;
import org.elasticsearch.search.aggregations.support.ValueType;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.core.rollup.RollupField;
import org.elasticsearch.xpack.core.rollup.action.RollupJobCaps;
import org.elasticsearch.xpack.core.rollup.job.DateHistogramGroupConfig;
import org.elasticsearch.xpack.core.rollup.job.GroupConfig;
@ -24,17 +25,22 @@ import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig;
import org.elasticsearch.xpack.core.rollup.job.TermsGroupConfig;
import org.joda.time.DateTimeZone;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
public class RollupJobIdentifierUtilTests extends ESTestCase {
private static final List<String> UNITS = new ArrayList<>(DateHistogramAggregationBuilder.DATE_FIELD_UNITS.keySet());
public void testOneMatch() {
final GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", new DateHistogramInterval("1h")));
final RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10, group, emptyList(), null);
@ -577,6 +583,124 @@ public class RollupJobIdentifierUtilTests extends ESTestCase {
assertFalse(valid);
}
public void testComparatorMixed() {
int numCaps = randomIntBetween(1, 10);
List<RollupJobCaps> caps = new ArrayList<>(numCaps);
for (int i = 0; i < numCaps; i++) {
DateHistogramInterval interval = getRandomInterval();
GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", interval));
RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10, group, emptyList(), null);
RollupJobCaps cap = new RollupJobCaps(job);
caps.add(cap);
}
caps.sort(RollupJobIdentifierUtils.COMPARATOR);
// This only tests for calendar/fixed ordering, ignoring the other criteria
for (int i = 1; i < numCaps; i++) {
RollupJobCaps a = caps.get(i - 1);
RollupJobCaps b = caps.get(i);
long aMillis = getMillis(a);
long bMillis = getMillis(b);
assertThat(aMillis, greaterThanOrEqualTo(bMillis));
}
}
public void testComparatorFixed() {
int numCaps = randomIntBetween(1, 10);
List<RollupJobCaps> caps = new ArrayList<>(numCaps);
for (int i = 0; i < numCaps; i++) {
DateHistogramInterval interval = getRandomFixedInterval();
GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", interval));
RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10, group, emptyList(), null);
RollupJobCaps cap = new RollupJobCaps(job);
caps.add(cap);
}
caps.sort(RollupJobIdentifierUtils.COMPARATOR);
// This only tests for fixed ordering, ignoring the other criteria
for (int i = 1; i < numCaps; i++) {
RollupJobCaps a = caps.get(i - 1);
RollupJobCaps b = caps.get(i);
long aMillis = getMillis(a);
long bMillis = getMillis(b);
assertThat(aMillis, greaterThanOrEqualTo(bMillis));
}
}
public void testComparatorCalendar() {
int numCaps = randomIntBetween(1, 10);
List<RollupJobCaps> caps = new ArrayList<>(numCaps);
for (int i = 0; i < numCaps; i++) {
DateHistogramInterval interval = getRandomCalendarInterval();
GroupConfig group = new GroupConfig(new DateHistogramGroupConfig("foo", interval));
RollupJobConfig job = new RollupJobConfig("foo", "index", "rollup", "*/5 * * * * ?", 10, group, emptyList(), null);
RollupJobCaps cap = new RollupJobCaps(job);
caps.add(cap);
}
caps.sort(RollupJobIdentifierUtils.COMPARATOR);
// This only tests for calendar ordering, ignoring the other criteria
for (int i = 1; i < numCaps; i++) {
RollupJobCaps a = caps.get(i - 1);
RollupJobCaps b = caps.get(i);
long aMillis = getMillis(a);
long bMillis = getMillis(b);
assertThat(aMillis, greaterThanOrEqualTo(bMillis));
}
}
private static long getMillis(RollupJobCaps cap) {
for (RollupJobCaps.RollupFieldCaps fieldCaps : cap.getFieldCaps().values()) {
for (Map<String, Object> agg : fieldCaps.getAggs()) {
if (agg.get(RollupField.AGG).equals(DateHistogramAggregationBuilder.NAME)) {
return RollupJobIdentifierUtils.getMillisFixedOrCalendar((String) agg.get(RollupField.INTERVAL));
}
}
}
return Long.MAX_VALUE;
}
private static DateHistogramInterval getRandomInterval() {
if (randomBoolean()) {
return getRandomFixedInterval();
}
return getRandomCalendarInterval();
}
private static DateHistogramInterval getRandomFixedInterval() {
int value = randomIntBetween(1, 1000);
String unit;
int randomValue = randomInt(4);
if (randomValue == 0) {
unit = "ms";
} else if (randomValue == 1) {
unit = "s";
} else if (randomValue == 2) {
unit = "m";
} else if (randomValue == 3) {
unit = "h";
} else {
unit = "d";
}
return new DateHistogramInterval(Integer.toString(value) + unit);
}
private static DateHistogramInterval getRandomCalendarInterval() {
return new DateHistogramInterval(UNITS.get(randomIntBetween(0, UNITS.size()-1)));
}
private Set<RollupJobCaps> singletonSet(RollupJobCaps cap) {
Set<RollupJobCaps> caps = new HashSet<>();
caps.add(cap);