From a954b150059d8c4090c5ef5064a60d81246ed71c Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Wed, 29 Jan 2020 10:51:58 -0500 Subject: [PATCH 1/5] Wire Percentiles aggregator into new VS framework This required a bit of a refactor to percentiles itself. Before, the Builder would switch on the chosen algo to generate an algo-specific factory. This doesn't work (or at least, would be difficult) in the new VS framework. This refactor consolidates both factories together and introduces a PercentilesConfig object to act as a standardized way to pass algo-specific parameters through the factory. This object is then used when deciding which kind of aggregator to create Note: CoreValuesSourceType.HISTOGRAM still lives in core, and will be moved in a subsequent PR. --- .../elasticsearch/search/SearchModule.java | 3 +- .../HDRPercentilesAggregatorFactory.java | 77 ----------- .../PercentilesAggregationBuilder.java | 70 ++++++++-- .../metrics/PercentilesAggregatorFactory.java | 126 ++++++++++++++++++ .../PercentilesAggregatorSupplier.java | 44 ++++++ .../TDigestPercentilesAggregatorFactory.java | 72 ---------- 6 files changed, 234 insertions(+), 158 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesAggregatorFactory.java create mode 100644 server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregatorFactory.java create mode 100644 server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregatorSupplier.java delete mode 100644 server/src/main/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesAggregatorFactory.java diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index e170b0b8ad3d1..a916dbb44c5d6 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -349,7 +349,8 @@ private void registerAggregations(List plugins) { registerAggregation(new AggregationSpec(PercentilesAggregationBuilder.NAME, PercentilesAggregationBuilder::new, PercentilesAggregationBuilder::parse) .addResultReader(InternalTDigestPercentiles.NAME, InternalTDigestPercentiles::new) - .addResultReader(InternalHDRPercentiles.NAME, InternalHDRPercentiles::new)); + .addResultReader(InternalHDRPercentiles.NAME, InternalHDRPercentiles::new) + .setAggregatorRegistrar(PercentilesAggregationBuilder::registerAggregators)); registerAggregation(new AggregationSpec(PercentileRanksAggregationBuilder.NAME, PercentileRanksAggregationBuilder::new, PercentileRanksAggregationBuilder::parse) .addResultReader(InternalTDigestPercentileRanks.NAME, InternalTDigestPercentileRanks::new) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesAggregatorFactory.java deleted file mode 100644 index 3bc9a8c243ff5..0000000000000 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesAggregatorFactory.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.search.aggregations.metrics; - -import org.elasticsearch.index.query.QueryShardContext; -import org.elasticsearch.search.aggregations.Aggregator; -import org.elasticsearch.search.aggregations.AggregatorFactories; -import org.elasticsearch.search.aggregations.AggregatorFactory; -import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; -import org.elasticsearch.search.aggregations.support.ValuesSource; -import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory; -import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; -import org.elasticsearch.search.internal.SearchContext; - -import java.io.IOException; -import java.util.List; -import java.util.Map; - -class HDRPercentilesAggregatorFactory extends ValuesSourceAggregatorFactory { - private final double[] percents; - private final int numberOfSignificantValueDigits; - private final boolean keyed; - - HDRPercentilesAggregatorFactory(String name, - ValuesSourceConfig config, - double[] percents, - int numberOfSignificantValueDigits, - boolean keyed, - QueryShardContext queryShardContext, - AggregatorFactory parent, - AggregatorFactories.Builder subFactoriesBuilder, - Map metaData) throws IOException { - super(name, config, queryShardContext, parent, subFactoriesBuilder, metaData); - this.percents = percents; - this.numberOfSignificantValueDigits = numberOfSignificantValueDigits; - this.keyed = keyed; - } - - @Override - protected Aggregator createUnmapped(SearchContext searchContext, - Aggregator parent, - List pipelineAggregators, - Map metaData) - throws IOException { - return new HDRPercentilesAggregator(name, null, searchContext, parent, percents, numberOfSignificantValueDigits, keyed, - config.format(), pipelineAggregators, metaData); - } - - @Override - protected Aggregator doCreateInternal(ValuesSource valuesSource, - SearchContext searchContext, - Aggregator parent, - boolean collectsFromSingleBucket, - List pipelineAggregators, - Map metaData) throws IOException { - return new HDRPercentilesAggregator(name, valuesSource, searchContext, parent, percents, numberOfSignificantValueDigits, - keyed, config.format(), pipelineAggregators, metaData); - } - -} diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregationBuilder.java index cbad8c8625f55..b905fe70a211b 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregationBuilder.java @@ -35,12 +35,14 @@ import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; import org.elasticsearch.search.aggregations.support.ValuesSourceParserHelper; +import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; import org.elasticsearch.search.aggregations.support.ValuesSourceType; import java.io.IOException; import java.util.Arrays; import java.util.Map; import java.util.Objects; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; public class PercentilesAggregationBuilder extends LeafOnly { @@ -118,6 +120,13 @@ public static AggregationBuilder parse(String aggregationName, XContentParser pa return returnedAgg; } + private static AtomicBoolean wasRegistered = new AtomicBoolean(false); + public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) { + if (wasRegistered.compareAndSet(false, true) == true) { + PercentilesAggregatorFactory.registerAggregators(valuesSourceRegistry); + } + } + private static void setIfNotNull(Consumer consumer, T value) { if (value != null) { consumer.accept(value); @@ -271,16 +280,18 @@ protected ValuesSourceAggregatorFactory innerBuild(QueryShardContext queryShardC ValuesSourceConfig config, AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException { - switch (method) { - case TDIGEST: - return new TDigestPercentilesAggregatorFactory(name, config, percents, compression, keyed, queryShardContext, parent, - subFactoriesBuilder, metaData); - case HDR: - return new HDRPercentilesAggregatorFactory(name, config, percents, - numberOfSignificantValueDigits, keyed, queryShardContext, parent, subFactoriesBuilder, metaData); - default: + PercentilesConfig percentilesConfig; + if (method.equals(PercentilesMethod.TDIGEST)) { + percentilesConfig = new PercentilesConfig.TDigestConfig(compression); + } else if (method.equals(PercentilesMethod.HDR)) { + percentilesConfig = new PercentilesConfig.HdrHistoConfig(numberOfSignificantValueDigits); + } else { throw new IllegalStateException("Illegal method [" + method + "]"); } + + return new PercentilesAggregatorFactory(name, config, percents, percentilesConfig, keyed, queryShardContext, parent, + subFactoriesBuilder, metaData); + } @Override @@ -365,4 +376,47 @@ public InternalBuilder method(PercentilesMethod method) { } } } + + /** + * A small config object that carries algo-specific settings. This allows the factory to have + * a single unified constructor for both algos, but internally switch execution + * depending on which algo is selected + */ + abstract static class PercentilesConfig { + private final PercentilesMethod method; + + PercentilesConfig(PercentilesMethod method) { + this.method = method; + } + + public PercentilesMethod getMethod() { + return method; + } + + static class TDigestConfig extends PercentilesConfig { + private final double compression; + + TDigestConfig(double compression) { + super(PercentilesMethod.TDIGEST); + this.compression = compression; + } + + public double getCompression() { + return compression; + } + } + + static class HdrHistoConfig extends PercentilesConfig { + private final int numberOfSignificantValueDigits; + + HdrHistoConfig(int numberOfSignificantValueDigits) { + super(PercentilesMethod.HDR); + this.numberOfSignificantValueDigits = numberOfSignificantValueDigits; + } + + public int getNumberOfSignificantValueDigits() { + return numberOfSignificantValueDigits; + } + } + } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregatorFactory.java new file mode 100644 index 0000000000000..fd84360256d63 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregatorFactory.java @@ -0,0 +1,126 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.search.aggregations.metrics; + +import org.elasticsearch.index.query.QueryShardContext; +import org.elasticsearch.search.DocValueFormat; +import org.elasticsearch.search.aggregations.AggregationExecutionException; +import org.elasticsearch.search.aggregations.Aggregator; +import org.elasticsearch.search.aggregations.AggregatorFactories; +import org.elasticsearch.search.aggregations.AggregatorFactory; +import org.elasticsearch.search.aggregations.metrics.PercentilesAggregationBuilder.PercentilesConfig; +import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; +import org.elasticsearch.search.aggregations.support.AggregatorSupplier; +import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; +import org.elasticsearch.search.aggregations.support.ValuesSource; +import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory; +import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; +import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; +import org.elasticsearch.search.internal.SearchContext; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +/** + * This factory is used to generate both TDigest and HDRHisto aggregators, depending + * on the selected method + */ +class PercentilesAggregatorFactory extends ValuesSourceAggregatorFactory { + + private final double[] percents; + private final PercentilesConfig percentilesConfig; + private final boolean keyed; + + static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) { + valuesSourceRegistry.register(PercentilesAggregationBuilder.NAME, + List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.HISTOGRAM), + new PercentilesAggregatorSupplier() { + @Override + public Aggregator build(String name, ValuesSource valuesSource, SearchContext context, Aggregator parent, + double[] percents, PercentilesConfig percentilesConfig, boolean keyed, DocValueFormat formatter, + List pipelineAggregators, Map metaData) throws IOException { + + if (percentilesConfig.getMethod().equals(PercentilesMethod.TDIGEST)) { + double compression = ((PercentilesConfig.TDigestConfig)percentilesConfig).getCompression(); + return new TDigestPercentilesAggregator(name, valuesSource, context, parent, percents, compression, keyed, + formatter, pipelineAggregators, metaData); + } else if (percentilesConfig.getMethod().equals(PercentilesMethod.HDR)) { + int numSigFig = ((PercentilesConfig.HdrHistoConfig)percentilesConfig).getNumberOfSignificantValueDigits(); + return new HDRPercentilesAggregator(name, valuesSource, context, parent, percents, numSigFig, keyed, + formatter, pipelineAggregators, metaData); + } + + // This should already have thrown but just in case + throw new IllegalArgumentException("Unknown percentiles method: [" + percentilesConfig.getMethod().toString() + "]"); + } + } + ); + } + + PercentilesAggregatorFactory(String name, ValuesSourceConfig config, double[] percents, + PercentilesConfig percentilesConfig, boolean keyed, QueryShardContext queryShardContext, + AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, + Map metaData) throws IOException { + super(name, config, queryShardContext, parent, subFactoriesBuilder, metaData); + this.percents = percents; + this.percentilesConfig = percentilesConfig; + this.keyed = keyed; + } + + @Override + protected Aggregator createUnmapped(SearchContext searchContext, + Aggregator parent, + List pipelineAggregators, + Map metaData) throws IOException { + if (percentilesConfig.getMethod().equals(PercentilesMethod.TDIGEST)) { + double compression = ((PercentilesConfig.TDigestConfig)percentilesConfig).getCompression(); + return new TDigestPercentilesAggregator(name, null, searchContext, parent, percents, compression, keyed, config.format(), + pipelineAggregators, metaData); + } else if (percentilesConfig.getMethod().equals(PercentilesMethod.HDR)) { + int numSigFig = ((PercentilesConfig.HdrHistoConfig)percentilesConfig).getNumberOfSignificantValueDigits(); + return new HDRPercentilesAggregator(name, null, searchContext, parent, percents, numSigFig, keyed, + config.format(), pipelineAggregators, metaData); + } + + // This should already have thrown but just in case + throw new IllegalArgumentException("Unknown percentiles method: [" + percentilesConfig.getMethod().toString() + "]"); + } + + @Override + protected Aggregator doCreateInternal(ValuesSource valuesSource, + SearchContext searchContext, + Aggregator parent, + boolean collectsFromSingleBucket, + List pipelineAggregators, + Map metaData) throws IOException { + + AggregatorSupplier aggregatorSupplier = ValuesSourceRegistry.getInstance().getAggregator(config.valueSourceType(), + PercentilesAggregationBuilder.NAME); + + if (aggregatorSupplier instanceof PercentilesAggregatorSupplier == false) { + throw new AggregationExecutionException("Registry miss-match - expected PercentilesAggregatorSupplier, found [" + + aggregatorSupplier.getClass().toString() + "]"); + } + PercentilesAggregatorSupplier percentilesAggregatorSupplier = (PercentilesAggregatorSupplier) aggregatorSupplier; + return percentilesAggregatorSupplier.build(name, valuesSource, searchContext, parent, percents, percentilesConfig, keyed, + config.format(), pipelineAggregators, metaData); + } +} diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregatorSupplier.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregatorSupplier.java new file mode 100644 index 0000000000000..1fcc96e58ea1a --- /dev/null +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregatorSupplier.java @@ -0,0 +1,44 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.search.aggregations.metrics; + +import org.elasticsearch.search.DocValueFormat; +import org.elasticsearch.search.aggregations.Aggregator; +import org.elasticsearch.search.aggregations.metrics.PercentilesAggregationBuilder.PercentilesConfig; +import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; +import org.elasticsearch.search.aggregations.support.AggregatorSupplier; +import org.elasticsearch.search.aggregations.support.ValuesSource; +import org.elasticsearch.search.internal.SearchContext; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +public interface PercentilesAggregatorSupplier extends AggregatorSupplier { + Aggregator build(String name, + ValuesSource valuesSource, + SearchContext context, + Aggregator parent, + double[] percents, + PercentilesConfig percentilesConfig, + boolean keyed, + DocValueFormat formatter, + List pipelineAggregators, + Map metaData) throws IOException; +} diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesAggregatorFactory.java deleted file mode 100644 index 1d9ed57f7109f..0000000000000 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesAggregatorFactory.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.search.aggregations.metrics; - -import org.elasticsearch.index.query.QueryShardContext; -import org.elasticsearch.search.aggregations.Aggregator; -import org.elasticsearch.search.aggregations.AggregatorFactories; -import org.elasticsearch.search.aggregations.AggregatorFactory; -import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; -import org.elasticsearch.search.aggregations.support.ValuesSource; -import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory; -import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; -import org.elasticsearch.search.internal.SearchContext; - -import java.io.IOException; -import java.util.List; -import java.util.Map; - -class TDigestPercentilesAggregatorFactory - extends ValuesSourceAggregatorFactory { - - private final double[] percents; - private final double compression; - private final boolean keyed; - - TDigestPercentilesAggregatorFactory(String name, ValuesSourceConfig config, double[] percents, - double compression, boolean keyed, QueryShardContext queryShardContext, AggregatorFactory parent, - AggregatorFactories.Builder subFactoriesBuilder, Map metaData) throws IOException { - super(name, config, queryShardContext, parent, subFactoriesBuilder, metaData); - this.percents = percents; - this.compression = compression; - this.keyed = keyed; - } - - @Override - protected Aggregator createUnmapped(SearchContext searchContext, - Aggregator parent, - List pipelineAggregators, - Map metaData) throws IOException { - return new TDigestPercentilesAggregator(name, null, searchContext, parent, percents, compression, keyed, config.format(), - pipelineAggregators, metaData); - } - - @Override - protected Aggregator doCreateInternal(ValuesSource valuesSource, - SearchContext searchContext, - Aggregator parent, - boolean collectsFromSingleBucket, - List pipelineAggregators, - Map metaData) throws IOException { - return new TDigestPercentilesAggregator(name, valuesSource, searchContext, parent, percents, compression, keyed, - config.format(), pipelineAggregators, metaData); - } - -} From ad19003291f41b9c9218058bddda9d6e2f1983ac Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Thu, 30 Jan 2020 12:52:23 -0500 Subject: [PATCH 2/5] Wire PercentileRanks aggregator into new VS framework Like Percentiles, this refactors consolidates both factories together, using PercentilesConfig object to act as a standardized way to pass algo-specific parameters through the factory. --- .../elasticsearch/search/SearchModule.java | 3 +- .../HDRPercentileRanksAggregatorFactory.java | 72 ---------- .../PercentileRanksAggregationBuilder.java | 27 ++-- .../PercentileRanksAggregatorFactory.java | 123 ++++++++++++++++++ 4 files changed, 144 insertions(+), 81 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksAggregatorFactory.java create mode 100644 server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index a916dbb44c5d6..cc6888e9f9f1b 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -354,7 +354,8 @@ private void registerAggregations(List plugins) { registerAggregation(new AggregationSpec(PercentileRanksAggregationBuilder.NAME, PercentileRanksAggregationBuilder::new, PercentileRanksAggregationBuilder::parse) .addResultReader(InternalTDigestPercentileRanks.NAME, InternalTDigestPercentileRanks::new) - .addResultReader(InternalHDRPercentileRanks.NAME, InternalHDRPercentileRanks::new)); + .addResultReader(InternalHDRPercentileRanks.NAME, InternalHDRPercentileRanks::new) + .setAggregatorRegistrar(PercentileRanksAggregationBuilder::registerAggregators)); registerAggregation(new AggregationSpec(MedianAbsoluteDeviationAggregationBuilder.NAME, MedianAbsoluteDeviationAggregationBuilder::new, MedianAbsoluteDeviationAggregationBuilder::parse) .addResultReader(InternalMedianAbsoluteDeviation::new)); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksAggregatorFactory.java deleted file mode 100644 index 7c42708865529..0000000000000 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksAggregatorFactory.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.search.aggregations.metrics; - -import org.elasticsearch.index.query.QueryShardContext; -import org.elasticsearch.search.aggregations.Aggregator; -import org.elasticsearch.search.aggregations.AggregatorFactories; -import org.elasticsearch.search.aggregations.AggregatorFactory; -import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; -import org.elasticsearch.search.aggregations.support.ValuesSource; -import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory; -import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; -import org.elasticsearch.search.internal.SearchContext; - -import java.io.IOException; -import java.util.List; -import java.util.Map; - -class HDRPercentileRanksAggregatorFactory extends ValuesSourceAggregatorFactory { - private final double[] values; - private final int numberOfSignificantValueDigits; - private final boolean keyed; - - HDRPercentileRanksAggregatorFactory(String name, ValuesSourceConfig config, double[] values, - int numberOfSignificantValueDigits, boolean keyed, QueryShardContext queryShardContext, - AggregatorFactory parent, AggregatorFactories.Builder subFactoriesBuilder, - Map metaData) throws IOException { - super(name, config, queryShardContext, parent, subFactoriesBuilder, metaData); - this.values = values; - this.numberOfSignificantValueDigits = numberOfSignificantValueDigits; - this.keyed = keyed; - } - - @Override - protected Aggregator createUnmapped(SearchContext searchContext, - Aggregator parent, - List pipelineAggregators, - Map metaData) throws IOException { - return new HDRPercentileRanksAggregator(name, null, searchContext, parent, values, numberOfSignificantValueDigits, keyed, - config.format(), pipelineAggregators, metaData); - } - - @Override - protected Aggregator doCreateInternal(ValuesSource valuesSource, - SearchContext searchContext, - Aggregator parent, - boolean collectsFromSingleBucket, - List pipelineAggregators, - Map metaData) throws IOException { - - return new HDRPercentileRanksAggregator(name, valuesSource, searchContext, parent, values, - numberOfSignificantValueDigits, keyed, config.format(), pipelineAggregators, metaData); - } - -} diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregationBuilder.java index d1f927ecfe501..418aecf651b87 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregationBuilder.java @@ -30,12 +30,14 @@ import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; import org.elasticsearch.search.aggregations.AggregatorFactory; +import org.elasticsearch.search.aggregations.metrics.PercentilesAggregationBuilder.PercentilesConfig; import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; import org.elasticsearch.search.aggregations.support.ValuesSource; import org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder.LeafOnly; import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; import org.elasticsearch.search.aggregations.support.ValuesSourceParserHelper; +import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; import org.elasticsearch.search.aggregations.support.ValuesSourceType; import java.io.IOException; @@ -43,6 +45,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.atomic.AtomicBoolean; import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; @@ -98,6 +101,13 @@ private static class HDROptions { }, HDR_OPTIONS_PARSER::parse, PercentilesMethod.HDR.getParseField(), ObjectParser.ValueType.OBJECT); } + private static AtomicBoolean wasRegistered = new AtomicBoolean(false); + public static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) { + if (wasRegistered.compareAndSet(false, true) == true) { + PercentileRanksAggregatorFactory.registerAggregators(valuesSourceRegistry); + } + } + public static AggregationBuilder parse(String aggregationName, XContentParser parser) throws IOException { // the aggregation name is supplied to the parser as a Context. See note at top of Parser for more details return PARSER.parse(parser, aggregationName); @@ -246,16 +256,17 @@ public PercentilesMethod method() { @Override protected ValuesSourceAggregatorFactory innerBuild(QueryShardContext queryShardContext, ValuesSourceConfig config, AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException { - switch (method) { - case TDIGEST: - return new TDigestPercentileRanksAggregatorFactory(name, config, values, compression, keyed, queryShardContext, parent, - subFactoriesBuilder, metaData); - case HDR: - return new HDRPercentileRanksAggregatorFactory(name, config, values, numberOfSignificantValueDigits, keyed, queryShardContext, - parent, subFactoriesBuilder, metaData); - default: + PercentilesConfig percentilesConfig; + if (method.equals(PercentilesMethod.TDIGEST)) { + percentilesConfig = new PercentilesConfig.TDigestConfig(compression); + } else if (method.equals(PercentilesMethod.HDR)) { + percentilesConfig = new PercentilesConfig.HdrHistoConfig(numberOfSignificantValueDigits); + } else { throw new IllegalStateException("Illegal method [" + method + "]"); } + + return new PercentileRanksAggregatorFactory(name, config, values, percentilesConfig, keyed, queryShardContext, parent, + subFactoriesBuilder, metaData); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java new file mode 100644 index 0000000000000..6a20f51e5895d --- /dev/null +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java @@ -0,0 +1,123 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.search.aggregations.metrics; + +import org.elasticsearch.index.query.QueryShardContext; +import org.elasticsearch.search.DocValueFormat; +import org.elasticsearch.search.aggregations.AggregationExecutionException; +import org.elasticsearch.search.aggregations.Aggregator; +import org.elasticsearch.search.aggregations.AggregatorFactories; +import org.elasticsearch.search.aggregations.AggregatorFactory; +import org.elasticsearch.search.aggregations.metrics.PercentilesAggregationBuilder.PercentilesConfig; +import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; +import org.elasticsearch.search.aggregations.support.AggregatorSupplier; +import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; +import org.elasticsearch.search.aggregations.support.ValuesSource; +import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFactory; +import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; +import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; +import org.elasticsearch.search.internal.SearchContext; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +class PercentileRanksAggregatorFactory extends ValuesSourceAggregatorFactory { + private final double[] values; + private final PercentilesConfig percentilesConfig; + private final boolean keyed; + + static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) { + valuesSourceRegistry.register(PercentileRanksAggregationBuilder.NAME, + List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.HISTOGRAM), + new PercentilesAggregatorSupplier() { + @Override + public Aggregator build(String name, ValuesSource valuesSource, SearchContext context, Aggregator parent, + double[] percents, PercentilesConfig percentilesConfig, boolean keyed, DocValueFormat formatter, + List pipelineAggregators, Map metaData) throws IOException { + + if (percentilesConfig.getMethod().equals(PercentilesMethod.TDIGEST)) { + double compression = ((PercentilesConfig.TDigestConfig)percentilesConfig).getCompression(); + return new TDigestPercentileRanksAggregator(name, valuesSource, context, parent, percents, compression, keyed, + formatter, pipelineAggregators, metaData); + } else if (percentilesConfig.getMethod().equals(PercentilesMethod.HDR)) { + int numSigFig = ((PercentilesConfig.HdrHistoConfig)percentilesConfig).getNumberOfSignificantValueDigits(); + return new HDRPercentileRanksAggregator(name, valuesSource, context, parent, percents, numSigFig, keyed, + formatter, pipelineAggregators, metaData); + } + + // This should already have thrown but just in case + throw new IllegalArgumentException("Unknown percentiles method: [" + percentilesConfig.getMethod().toString() + "]"); + } + } + ); + } + + PercentileRanksAggregatorFactory(String name, ValuesSourceConfig config, double[] values, + PercentilesConfig percentilesConfig, boolean keyed, + QueryShardContext queryShardContext, AggregatorFactory parent, + AggregatorFactories.Builder subFactoriesBuilder, + Map metaData) throws IOException { + super(name, config, queryShardContext, parent, subFactoriesBuilder, metaData); + this.values = values; + this.percentilesConfig = percentilesConfig; + this.keyed = keyed; + } + + @Override + protected Aggregator createUnmapped(SearchContext searchContext, + Aggregator parent, + List pipelineAggregators, + Map metaData) throws IOException { + if (percentilesConfig.getMethod().equals(PercentilesMethod.TDIGEST)) { + double compression = ((PercentilesConfig.TDigestConfig)percentilesConfig).getCompression(); + return new TDigestPercentileRanksAggregator(name, null, searchContext, parent, values, compression, keyed, config.format(), + pipelineAggregators, metaData); + } else if (percentilesConfig.getMethod().equals(PercentilesMethod.HDR)) { + int numSigFig = ((PercentilesConfig.HdrHistoConfig)percentilesConfig).getNumberOfSignificantValueDigits(); + return new HDRPercentileRanksAggregator(name, null, searchContext, parent, values, numSigFig, keyed, + config.format(), pipelineAggregators, metaData); + } + + // This should already have thrown but just in case + throw new IllegalArgumentException("Unknown percentiles method: [" + percentilesConfig.getMethod().toString() + "]"); + } + + @Override + protected Aggregator doCreateInternal(ValuesSource valuesSource, + SearchContext searchContext, + Aggregator parent, + boolean collectsFromSingleBucket, + List pipelineAggregators, + Map metaData) throws IOException { + + AggregatorSupplier aggregatorSupplier = ValuesSourceRegistry.getInstance().getAggregator(config.valueSourceType(), + PercentileRanksAggregationBuilder.NAME); + + if (aggregatorSupplier instanceof PercentilesAggregatorSupplier == false) { + throw new AggregationExecutionException("Registry miss-match - expected PercentilesAggregatorSupplier, found [" + + aggregatorSupplier.getClass().toString() + "]"); + } + PercentilesAggregatorSupplier percentilesAggregatorSupplier = (PercentilesAggregatorSupplier) aggregatorSupplier; + return percentilesAggregatorSupplier.build(name, valuesSource, searchContext, parent, values, percentilesConfig, keyed, + config.format(), pipelineAggregators, metaData); + } + +} From d70449879ac2b5beb66e855562ef312760fe374f Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Fri, 31 Jan 2020 13:17:40 -0500 Subject: [PATCH 3/5] Throw IllegalStateException for bad methods --- .../metrics/PercentileRanksAggregatorFactory.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java index 6a20f51e5895d..23930e570f164 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java @@ -64,7 +64,7 @@ public Aggregator build(String name, ValuesSource valuesSource, SearchContext co } // This should already have thrown but just in case - throw new IllegalArgumentException("Unknown percentiles method: [" + percentilesConfig.getMethod().toString() + "]"); + throw new IllegalStateException("Unknown percentiles method: [" + percentilesConfig.getMethod().toString() + "]"); } } ); @@ -97,7 +97,7 @@ protected Aggregator createUnmapped(SearchContext searchContext, } // This should already have thrown but just in case - throw new IllegalArgumentException("Unknown percentiles method: [" + percentilesConfig.getMethod().toString() + "]"); + throw new IllegalStateException("Unknown percentiles method: [" + percentilesConfig.getMethod().toString() + "]"); } @Override From cc6693e2e0d96d54e8554146a4f1c8c655a5d902 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Mon, 2 Mar 2020 16:27:51 -0500 Subject: [PATCH 4/5] Adjust supported types --- .../aggregations/metrics/PercentileRanksAggregatorFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java index 05e4e30cacad1..a2c827bff373f 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java @@ -46,7 +46,7 @@ class PercentileRanksAggregatorFactory extends ValuesSourceAggregatorFactory { static void registerAggregators(ValuesSourceRegistry valuesSourceRegistry) { valuesSourceRegistry.register(PercentileRanksAggregationBuilder.NAME, - List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.HISTOGRAM), + List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.HISTOGRAM, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), new PercentilesAggregatorSupplier() { @Override public Aggregator build(String name, ValuesSource valuesSource, SearchContext context, Aggregator parent, From 1f607c3a4e2c992e1c97c12249fd91ac8e530ed7 Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Mon, 2 Mar 2020 16:37:31 -0500 Subject: [PATCH 5/5] Fix formatting --- server/src/main/java/org/elasticsearch/search/SearchModule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index 8d775509d0b7a..6116261e301cb 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -369,7 +369,7 @@ private void registerAggregations(List plugins) { PercentileRanksAggregationBuilder::parse) .addResultReader(InternalTDigestPercentileRanks.NAME, InternalTDigestPercentileRanks::new) .addResultReader(InternalHDRPercentileRanks.NAME, InternalHDRPercentileRanks::new) - .setAggregatorRegistrar(PercentileRanksAggregationBuilder::registerAggregators)); + .setAggregatorRegistrar(PercentileRanksAggregationBuilder::registerAggregators)); registerAggregation(new AggregationSpec(MedianAbsoluteDeviationAggregationBuilder.NAME, MedianAbsoluteDeviationAggregationBuilder::new, MedianAbsoluteDeviationAggregationBuilder::parse) .addResultReader(InternalMedianAbsoluteDeviation::new));