[Rollup] Remove `computed` field from rollup docs
The computed field contained a list of all aggs that were computed for this particular rollup doc. It was used to help filter to the correct rollup job/set of jobs. But this functionality was never perfect, and has been obsoleted by validating the rollup caps while searching. So we can remove the computed field and save a bunch of space (since they were quite bulky) Original commit: elastic/x-pack-elasticsearch@455644488f
This commit is contained in:
Zachary Tong
parent
b4bf9ed87e
commit
f682ecc576
|
|
@ -85,8 +85,6 @@ public class Rollup extends Plugin implements ActionPlugin, PersistentTaskPlugin
|
|||
public static final String TASK_THREAD_POOL_NAME = RollupField.NAME + "_indexing";
|
||||
public static final String SCHEDULE_THREAD_POOL_NAME = RollupField.NAME + "_scheduler";
|
||||
|
||||
public static final String COMPUTED_ROLLUPS_FIELD = "computed";
|
||||
|
||||
public static final String MAPPING_METADATA_PLACEHOLDER = "\"ROLLUP_METADATA_PLACEHOLDER\":\"ROLLUP_METADATA_PLACEHOLDER\"";
|
||||
public static final String ROLLUP_TEMPLATE_VERSION_FIELD = "rollup-version";
|
||||
public static final String ROLLUP_TEMPLATE_VERSION_PATTERN =
|
||||
|
|
|
|||
|
|
@ -36,10 +36,6 @@ import java.util.function.Supplier;
|
|||
* Documents are expected to follow a convention that looks like:
|
||||
* <pre>{@code
|
||||
* {
|
||||
* "_rollup.computed": [
|
||||
* "ts.date_histogram", foo.max", "title.terms"
|
||||
* ]
|
||||
* },
|
||||
* "_rollup.version": 1,
|
||||
* "ts.date_histogram.timestamp": 1494796800000,
|
||||
* "ts.date_histogram._count": 1000000000,
|
||||
|
|
@ -90,9 +86,7 @@ public class RollupRequestTranslator {
|
|||
* "bool" : {
|
||||
* "must" : [
|
||||
* { "term" : { "_rollup.version" : 1 } },
|
||||
* { "term": { "_rollup.computed" : "ts.date_histogram" } },
|
||||
* { "term": { "ts.date_histogram.interval" : "1d" } }
|
||||
* { "term": { "_rollup.computed" : "foo.max" } }
|
||||
* ]
|
||||
* }
|
||||
* },
|
||||
|
|
@ -204,14 +198,6 @@ public class RollupRequestTranslator {
|
|||
* <li>
|
||||
* <ul>
|
||||
* <li>Query type: TermQuery</li>
|
||||
* <li>Field: `_rollup.computed`</li>
|
||||
* <li>Value: `{timestamp field}.date_histogram`</li>
|
||||
* </ul>
|
||||
* </li>
|
||||
* <li>Add a filter condition:</li>
|
||||
* <li>
|
||||
* <ul>
|
||||
* <li>Query type: TermQuery</li>
|
||||
* <li>Field: `{timestamp_field}.date_histogram.interval`</li>
|
||||
* <li>Value: `{source interval}`</li>
|
||||
* </ul>
|
||||
|
|
@ -331,14 +317,6 @@ public class RollupRequestTranslator {
|
|||
* <li>Field: `{field name}.terms._count`</li>
|
||||
* </ul>
|
||||
* </li>
|
||||
* <li>Add a filter condition:</li>
|
||||
* <li>
|
||||
* <ul>
|
||||
* <li>Query type: TermQuery</li>
|
||||
* <li>Field: `_rollup.computed`</li>
|
||||
* <li>Value: `{timestamp field}.terms`</li>
|
||||
* </ul>
|
||||
* </li>
|
||||
* </ul>
|
||||
*
|
||||
*/
|
||||
|
|
@ -391,11 +369,6 @@ public class RollupRequestTranslator {
|
|||
|
||||
T rolled = factory.get();
|
||||
|
||||
// add the filter condition for this agg, e.g.
|
||||
// "term": { "_rollup.computed": "foo.histogram" }
|
||||
filterConditions.add(new TermQueryBuilder(RollupField.formatMetaField(Rollup.COMPUTED_ROLLUPS_FIELD),
|
||||
RollupField.formatComputed(source.field(), source.getType())));
|
||||
|
||||
// Translate all subaggs and add to the newly translated agg
|
||||
// NOTE: using for loop instead of stream because compiler explodes with a bug :/
|
||||
for (AggregationBuilder subAgg : source.getSubAggregations()) {
|
||||
|
|
@ -462,14 +435,6 @@ public class RollupRequestTranslator {
|
|||
* <li>Agg type: same as source agg</li>
|
||||
* <li>Named: same as the source agg</li>
|
||||
* <li>Field: `{agg_type}.{field_name}.value`</li>
|
||||
* <li>Add a filter condition:</li>
|
||||
* <li>
|
||||
* <ul>
|
||||
* <li>Query type: TermQuery</li>
|
||||
* <li>Field: `_rollup.computed`</li>
|
||||
* <li>Value: `{field_name}.{agg_type}`</li>
|
||||
* </ul>
|
||||
* </li>
|
||||
* </ul>
|
||||
*
|
||||
* IF the agg is an AvgAgg, the following additional conventions are added:
|
||||
|
|
|
|||
|
|
@ -65,17 +65,6 @@ class IndexerUtils {
|
|||
docId.update(vs, 0, vs.length);
|
||||
processMetrics(metrics, doc);
|
||||
|
||||
Set<String> computed = new HashSet<>(keys.size() + metrics.size());
|
||||
|
||||
// Add just the original key/agg names, to facilitate filtering later
|
||||
computed.addAll(keys.entrySet().stream().map(Map.Entry::getKey).collect(Collectors.toList()));
|
||||
computed.addAll(metrics.stream()
|
||||
.map(m -> m.getName()
|
||||
.replace("." + RollupField.COUNT_FIELD, "")
|
||||
.replace("." + RollupField.VALUE, ""))
|
||||
.collect(Collectors.toList()));
|
||||
|
||||
doc.put(RollupField.ROLLUP_META + "." + Rollup.COMPUTED_ROLLUPS_FIELD, computed);
|
||||
doc.put(RollupField.ROLLUP_META + "." + RollupField.VERSION_FIELD, Rollup.ROLLUP_VERSION);
|
||||
doc.put(RollupField.ROLLUP_META + "." + RollupField.ID.getPreferredName(), jobId);
|
||||
|
||||
|
|
|
|||
|
|
@ -99,16 +99,13 @@ public class RollupRequestTranslationTests extends ESTestCase {
|
|||
assertThat(((SumAggregationBuilder)subAggs.get("test_histo._count")).field(),
|
||||
equalTo("foo.date_histogram._count"));
|
||||
|
||||
assertThat(filterConditions.size(), equalTo(2));
|
||||
assertThat(filterConditions.size(), equalTo(1));
|
||||
for (QueryBuilder q : filterConditions) {
|
||||
if (q instanceof TermQueryBuilder) {
|
||||
switch (((TermQueryBuilder) q).fieldName()) {
|
||||
case "foo.date_histogram.time_zone":
|
||||
assertThat(((TermQueryBuilder) q).value(), equalTo("UTC"));
|
||||
break;
|
||||
case "_rollup.computed":
|
||||
assertThat(((TermQueryBuilder) q).value(), equalTo("foo.date_histogram"));
|
||||
break;
|
||||
default:
|
||||
fail("Unexpected Term Query in filter conditions: [" + ((TermQueryBuilder) q).fieldName() + "]");
|
||||
break;
|
||||
|
|
@ -203,14 +200,12 @@ public class RollupRequestTranslationTests extends ESTestCase {
|
|||
assertThat(((SumAggregationBuilder)subAggs.get("test_histo._count")).field(),
|
||||
equalTo("foo.date_histogram._count"));
|
||||
|
||||
assertThat(filterConditions.size(), equalTo(2));
|
||||
assertThat(filterConditions.size(), equalTo(1));
|
||||
|
||||
for (QueryBuilder q : filterConditions) {
|
||||
if (q instanceof TermQueryBuilder) {
|
||||
if (((TermQueryBuilder) q).fieldName().equals("foo.date_histogram.time_zone")) {
|
||||
assertThat(((TermQueryBuilder) q).value(), equalTo("UTC"));
|
||||
} else if (((TermQueryBuilder) q).fieldName().equals("_rollup.computed")) {
|
||||
assertThat(((TermQueryBuilder) q).value(), equalTo("foo.date_histogram"));
|
||||
} else {
|
||||
fail("Unexpected Term Query in filter conditions: [" + ((TermQueryBuilder) q).fieldName() + "]");
|
||||
}
|
||||
|
|
@ -255,15 +250,13 @@ public class RollupRequestTranslationTests extends ESTestCase {
|
|||
assertThat(((SumAggregationBuilder)subAggs.get("test_histo._count")).field(),
|
||||
equalTo("foo.date_histogram._count"));
|
||||
|
||||
assertThat(filterConditions.size(), equalTo(2));
|
||||
assertThat(filterConditions.size(), equalTo(1));
|
||||
|
||||
for (QueryBuilder q : filterConditions) {
|
||||
if (q instanceof TermQueryBuilder) {
|
||||
if (((TermQueryBuilder) q).fieldName().equals("foo.date_histogram.time_zone")) {
|
||||
assertThat(((TermQueryBuilder) q).value(), equalTo("UTC"));
|
||||
} else if (((TermQueryBuilder) q).fieldName().equals("_rollup.computed")) {
|
||||
assertThat(((TermQueryBuilder) q).value(), equalTo("foo.date_histogram"));
|
||||
} else {
|
||||
} else {
|
||||
fail("Unexpected Term Query in filter conditions: [" + ((TermQueryBuilder) q).fieldName() + "]");
|
||||
}
|
||||
} else {
|
||||
|
|
@ -326,11 +319,7 @@ public class RollupRequestTranslationTests extends ESTestCase {
|
|||
assertThat(((SumAggregationBuilder)subAggs.get("test_string_terms._count")).field(),
|
||||
equalTo("foo.terms._count"));
|
||||
|
||||
assertThat(filterConditions.size(), equalTo(1));
|
||||
assertThat(filterConditions.get(0), Matchers.instanceOf(TermQueryBuilder.class));
|
||||
TermQueryBuilder computedFilter = (TermQueryBuilder)filterConditions.get(0);
|
||||
assertThat(computedFilter.fieldName(), equalTo("_rollup.computed"));
|
||||
assertThat(computedFilter.value(), equalTo("foo.terms"));
|
||||
assertThat(filterConditions.size(), equalTo(0));
|
||||
}
|
||||
|
||||
public void testBasicHisto() {
|
||||
|
|
@ -369,13 +358,10 @@ public class RollupRequestTranslationTests extends ESTestCase {
|
|||
assertThat(((SumAggregationBuilder)subAggs.get("test_histo._count")).field(),
|
||||
equalTo("foo.histogram._count"));
|
||||
|
||||
assertThat(filterConditions.size(), equalTo(1));
|
||||
assertThat(filterConditions.size(), equalTo(0));
|
||||
for (QueryBuilder q : filterConditions) {
|
||||
if (q instanceof TermQueryBuilder) {
|
||||
switch (((TermQueryBuilder) q).fieldName()) {
|
||||
case "_rollup.computed":
|
||||
assertThat(((TermQueryBuilder) q).value(), equalTo("foo.histogram"));
|
||||
break;
|
||||
default:
|
||||
fail("Unexpected Term Query in filter conditions: [" + ((TermQueryBuilder) q).fieldName() + "]");
|
||||
break;
|
||||
|
|
|
|||
|
|
@ -188,14 +188,6 @@ public class IndexerUtilsTests extends AggregatorTestCase {
|
|||
Map<String, Object> map = doc.sourceAsMap();
|
||||
assertNotNull( map.get(valueField + "." + MaxAggregationBuilder.NAME + "." + RollupField.VALUE));
|
||||
assertThat(map.get("the_histo." + DateHistogramAggregationBuilder.NAME + "." + RollupField.COUNT_FIELD), equalTo(1));
|
||||
Object computed = map.get(RollupField.ROLLUP_META + "." + Rollup.COMPUTED_ROLLUPS_FIELD);
|
||||
|
||||
assertTrue(computed instanceof List);
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
List<String> computedList = (ArrayList<String>)computed;
|
||||
assertTrue(computedList.contains("the_histo.date_histogram"));
|
||||
assertTrue(computedList.contains("the_avg.max"));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -252,14 +244,6 @@ public class IndexerUtilsTests extends AggregatorTestCase {
|
|||
Map<String, Object> map = doc.sourceAsMap();
|
||||
assertNotNull( map.get(valueField + "." + MaxAggregationBuilder.NAME + "." + RollupField.VALUE));
|
||||
assertThat(map.get("the_terms." + TermsAggregationBuilder.NAME + "." + RollupField.COUNT_FIELD), equalTo(1));
|
||||
Object computed = map.get(RollupField.ROLLUP_META + "." + Rollup.COMPUTED_ROLLUPS_FIELD);
|
||||
|
||||
assertTrue(computed instanceof List);
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
List<String> computedList = (ArrayList<String>)computed;
|
||||
assertTrue(computedList.contains("the_terms.terms"));
|
||||
assertTrue(computedList.contains("the_avg.max"));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -327,14 +311,6 @@ public class IndexerUtilsTests extends AggregatorTestCase {
|
|||
assertNull(map.get("another_field." + AvgAggregationBuilder.NAME + "." + RollupField.VALUE));
|
||||
assertNotNull(map.get("another_field." + SumAggregationBuilder.NAME + "." + RollupField.VALUE));
|
||||
assertThat(map.get("the_histo." + DateHistogramAggregationBuilder.NAME + "." + RollupField.COUNT_FIELD), equalTo(1));
|
||||
Object computed = map.get(RollupField.ROLLUP_META + "." + Rollup.COMPUTED_ROLLUPS_FIELD);
|
||||
|
||||
assertTrue(computed instanceof List);
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
List<String> computedList = (ArrayList<String>)computed;
|
||||
assertTrue(computedList.contains("the_histo.date_histogram"));
|
||||
assertTrue(computedList.contains("another_field.sum"));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -113,7 +113,6 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
|
|||
asMap(
|
||||
"_rollup.version", 1,
|
||||
"the_histo.date_histogram.timestamp", 3,
|
||||
"_rollup.computed", Collections.singletonList("the_histo.date_histogram"),
|
||||
"the_histo.date_histogram.interval", "1ms",
|
||||
"the_histo.date_histogram._count", 2,
|
||||
"the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(),
|
||||
|
|
@ -127,7 +126,6 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
|
|||
asMap(
|
||||
"_rollup.version", 1,
|
||||
"the_histo.date_histogram.timestamp", 7,
|
||||
"_rollup.computed", Collections.singletonList("the_histo.date_histogram"),
|
||||
"the_histo.date_histogram.interval", "1ms",
|
||||
"the_histo.date_histogram._count", 1,
|
||||
"the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(),
|
||||
|
|
@ -176,8 +174,6 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
|
|||
asMap(
|
||||
"_rollup.version", 1,
|
||||
"the_histo.date_histogram.timestamp", asLong("2015-03-31T03:00:00"),
|
||||
"_rollup.computed",
|
||||
Arrays.asList("counter.max", "counter.avg", "counter.min", "counter.sum", "the_histo.date_histogram"),
|
||||
"the_histo.date_histogram.interval", "1h",
|
||||
"the_histo.date_histogram._count", 3,
|
||||
"counter.avg._count", 3.0,
|
||||
|
|
@ -196,8 +192,6 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
|
|||
asMap(
|
||||
"_rollup.version", 1,
|
||||
"the_histo.date_histogram.timestamp", asLong("2015-03-31T04:00:00"),
|
||||
"_rollup.computed",
|
||||
Arrays.asList("counter.max", "counter.avg", "counter.min", "counter.sum", "the_histo.date_histogram"),
|
||||
"the_histo.date_histogram.interval", "1h",
|
||||
"the_histo.date_histogram._count", 3,
|
||||
"counter.avg._count", 3.0,
|
||||
|
|
@ -216,8 +210,6 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
|
|||
asMap(
|
||||
"_rollup.version", 1,
|
||||
"the_histo.date_histogram.timestamp", asLong("2015-03-31T05:00:00"),
|
||||
"_rollup.computed",
|
||||
Arrays.asList("counter.max", "counter.avg", "counter.min", "counter.sum", "the_histo.date_histogram"),
|
||||
"the_histo.date_histogram.interval", "1h",
|
||||
"the_histo.date_histogram._count", 4,
|
||||
"counter.avg._count", 4.0,
|
||||
|
|
@ -236,8 +228,6 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
|
|||
asMap(
|
||||
"_rollup.version", 1,
|
||||
"the_histo.date_histogram.timestamp", asLong("2015-03-31T06:00:00"),
|
||||
"_rollup.computed",
|
||||
Arrays.asList("counter.max", "counter.avg", "counter.min", "counter.sum", "the_histo.date_histogram"),
|
||||
"the_histo.date_histogram.interval", "1h",
|
||||
"the_histo.date_histogram._count", 3,
|
||||
"counter.avg._count", 3.0,
|
||||
|
|
@ -256,8 +246,6 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
|
|||
asMap(
|
||||
"_rollup.version", 1,
|
||||
"the_histo.date_histogram.timestamp", asLong("2015-03-31T07:00:00"),
|
||||
"_rollup.computed",
|
||||
Arrays.asList("counter.max", "counter.avg", "counter.min", "counter.sum", "the_histo.date_histogram"),
|
||||
"the_histo.date_histogram.interval", "1h",
|
||||
"the_histo.date_histogram._count", 3,
|
||||
"counter.avg._count", 3.0,
|
||||
|
|
@ -307,7 +295,6 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
|
|||
asMap(
|
||||
"_rollup.version", 1,
|
||||
"the_histo.date_histogram.timestamp", rounding.round(now - TimeValue.timeValueHours(5).getMillis()),
|
||||
"_rollup.computed", Collections.singletonList("the_histo.date_histogram"),
|
||||
"the_histo.date_histogram.interval", "1m",
|
||||
"the_histo.date_histogram._count", 2,
|
||||
"the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(),
|
||||
|
|
@ -321,7 +308,6 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
|
|||
asMap(
|
||||
"_rollup.version", 1,
|
||||
"the_histo.date_histogram.timestamp", rounding.round(now - TimeValue.timeValueMinutes(75).getMillis()),
|
||||
"_rollup.computed", Collections.singletonList("the_histo.date_histogram"),
|
||||
"the_histo.date_histogram.interval", "1m",
|
||||
"the_histo.date_histogram._count", 2,
|
||||
"the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(),
|
||||
|
|
@ -335,7 +321,6 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
|
|||
asMap(
|
||||
"_rollup.version", 1,
|
||||
"the_histo.date_histogram.timestamp", rounding.round(now - TimeValue.timeValueMinutes(61).getMillis()),
|
||||
"_rollup.computed", Collections.singletonList("the_histo.date_histogram"),
|
||||
"the_histo.date_histogram.interval", "1m",
|
||||
"the_histo.date_histogram._count", 1,
|
||||
"the_histo.date_histogram.time_zone", DateTimeZone.UTC.toString(),
|
||||
|
|
@ -379,7 +364,6 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
|
|||
asMap(
|
||||
"_rollup.version", 1,
|
||||
"the_histo.date_histogram.timestamp", asLong("2015-03-31T03:00:00"),
|
||||
"_rollup.computed", Collections.singletonList("the_histo.date_histogram"),
|
||||
"the_histo.date_histogram.interval", "1d",
|
||||
"the_histo.date_histogram._count", 2,
|
||||
"the_histo.date_histogram.time_zone", timeZone.toString(),
|
||||
|
|
@ -399,7 +383,6 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
|
|||
asMap(
|
||||
"_rollup.version", 1,
|
||||
"the_histo.date_histogram.timestamp", asLong("2015-03-31T03:00:00"),
|
||||
"_rollup.computed", Collections.singletonList("the_histo.date_histogram"),
|
||||
"the_histo.date_histogram.interval", "1d",
|
||||
"the_histo.date_histogram._count", 2,
|
||||
"the_histo.date_histogram.time_zone", timeZone.toString(),
|
||||
|
|
@ -413,7 +396,6 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
|
|||
asMap(
|
||||
"_rollup.version", 1,
|
||||
"the_histo.date_histogram.timestamp", asLong("2015-04-01T03:00:00"),
|
||||
"_rollup.computed", Collections.singletonList("the_histo.date_histogram"),
|
||||
"the_histo.date_histogram.interval", "1d",
|
||||
"the_histo.date_histogram._count", 5,
|
||||
"the_histo.date_histogram.time_zone", timeZone.toString(),
|
||||
|
|
@ -452,7 +434,6 @@ public class RollupIndexerIndexingTests extends AggregatorTestCase {
|
|||
Map<String, Object> source = ((IndexRequest) request).sourceAsMap();
|
||||
|
||||
assertThat(source.get("_rollup.version"), equalTo(1));
|
||||
assertTrue(((List<?>) source.get("_rollup.computed")).containsAll(Arrays.asList("ts.date_histogram", "the_avg.avg")));
|
||||
assertThat(source.get("ts.date_histogram.interval"), equalTo(timeInterval.toString()));
|
||||
assertNotNull(source.get("the_avg.avg._count"));
|
||||
assertNotNull(source.get("the_avg.avg.value"));
|
||||
|
|
|
|||
Loading…
Reference in New Issue