From 4d7f4665c2f66b620255c48a8b4892e714c9c018 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Thu, 2 Feb 2017 09:22:41 +0900 Subject: [PATCH 01/10] Add PostAggregators to generator cache keys for top-n queries --- .../theta/SketchEstimatePostAggregator.java | 10 + .../theta/SketchSetPostAggregator.java | 16 +- .../ApproximateHistogramPostAggregator.java | 4 +- .../histogram/BucketsPostAggregator.java | 18 +- .../CustomBucketsPostAggregator.java | 13 +- .../histogram/EqualBucketsPostAggregator.java | 22 ++- .../histogram/MaxPostAggregator.java | 15 +- .../histogram/MinPostAggregator.java | 15 +- .../histogram/QuantilePostAggregator.java | 18 +- .../histogram/QuantilesPostAggregator.java | 22 ++- .../StandardDeviationPostAggregator.java | 11 ++ .../io/druid/granularity/AllGranularity.java | 2 +- .../granularity/BaseQueryGranularity.java | 2 - .../granularity/DurationGranularity.java | 2 +- .../io/druid/granularity/NoneGranularity.java | 2 +- .../druid/granularity/PeriodGranularity.java | 2 +- .../druid/granularity/QueryGranularity.java | 5 +- .../query/aggregation/AggregatorFactory.java | 5 +- .../query/aggregation/PostAggregator.java | 12 +- .../HyperUniqueFinalizingPostAggregator.java | 10 + .../post/ArithmeticPostAggregator.java | 11 ++ .../post/ConstantPostAggregator.java | 16 +- .../post/DoubleGreatestPostAggregator.java | 9 + .../post/DoubleLeastPostAggregator.java | 9 + .../post/ExpressionPostAggregator.java | 10 + .../post/FieldAccessPostAggregator.java | 15 +- .../post/JavaScriptPostAggregator.java | 39 ++-- .../post/LongGreatestPostAggregator.java | 9 + .../post/LongLeastPostAggregator.java | 9 + .../aggregation/post/PostAggregatorIds.java | 44 +++++ .../io/druid/query/cache/CacheKeyBuilder.java | 175 ++++++++++++++++++ .../Cacheable.java} | 36 +--- .../druid/query/dimension/DimensionSpec.java | 5 +- .../query/dimension/LookupDimensionSpec.java | 2 +- .../extraction/TimeFormatExtractionFn.java | 2 +- .../java/io/druid/query/filter/DimFilter.java | 11 +- .../groupby/GroupByQueryQueryToolChest.java | 49 +---- .../search/SearchQueryQueryToolChest.java | 2 +- .../select/SelectQueryQueryToolChest.java | 2 +- .../TimeseriesQueryQueryToolChest.java | 27 +-- .../io/druid/query/topn/TopNMetricSpec.java | 19 +- .../query/topn/TopNQueryQueryToolChest.java | 41 ++-- .../post/JavaScriptPostAggregatorTest.java | 11 +- .../query/cache/CacheKeyBuilderTest.java | 121 ++++++++++++ .../topn/TopNQueryQueryToolChestTest.java | 75 +++++++- .../client/CachingClusteredClientTest.java | 144 +++++++------- 46 files changed, 798 insertions(+), 301 deletions(-) create mode 100644 processing/src/main/java/io/druid/query/aggregation/post/PostAggregatorIds.java create mode 100644 processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java rename processing/src/main/java/io/druid/query/{QueryCacheHelper.java => cache/Cacheable.java} (52%) create mode 100644 processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java diff --git a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchEstimatePostAggregator.java b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchEstimatePostAggregator.java index 373f7a30cd4c..215acc4f8db6 100644 --- a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchEstimatePostAggregator.java +++ b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchEstimatePostAggregator.java @@ -26,6 +26,8 @@ import com.google.common.collect.Sets; import com.google.common.primitives.Doubles; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.aggregation.post.PostAggregatorIds; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.Map; @@ -147,4 +149,12 @@ public int hashCode() result = 31 * result + (errorBoundsStdDev != null ? errorBoundsStdDev.hashCode() : 0); return result; } + + @Override + public byte[] getCacheKey() + { + final CacheKeyBuilder builder = new CacheKeyBuilder(PostAggregatorIds.SKETCH_ESTIMATE) + .appendCacheable(field); + return errorBoundsStdDev == null ? builder.build() : builder.appendInt(errorBoundsStdDev).build(); + } } diff --git a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java index 40849ce37a40..5120ed91dd8c 100644 --- a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java +++ b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java @@ -24,8 +24,9 @@ import com.google.common.collect.Sets; import com.yahoo.sketches.Util; import io.druid.java.util.common.IAE; -import io.druid.java.util.common.logger.Logger; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.aggregation.post.PostAggregatorIds; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.List; @@ -34,9 +35,6 @@ public class SketchSetPostAggregator implements PostAggregator { - - private static final Logger LOG = new Logger(SketchSetPostAggregator.class); - private final String name; private final List fields; private final SketchHolder.Func func; @@ -163,4 +161,14 @@ public int hashCode() result = 31 * result + maxSketchSize; return result; } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.SKETCH_SET) + .appendString(getFunc()) + .appendCacheableList(fields) + .appendInt(maxSketchSize) + .build(); + } } diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/ApproximateHistogramPostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/ApproximateHistogramPostAggregator.java index 58a864bc4e12..f8136a6bc376 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/ApproximateHistogramPostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/ApproximateHistogramPostAggregator.java @@ -29,8 +29,8 @@ public abstract class ApproximateHistogramPostAggregator implements PostAggregat { private static final Comparator COMPARATOR = ApproximateHistogramAggregator.COMPARATOR; - private final String name; - private final String fieldName; + protected final String name; + protected final String fieldName; public ApproximateHistogramPostAggregator( String name, diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/BucketsPostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/BucketsPostAggregator.java index f117f507f145..86f54eb78afe 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/BucketsPostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/BucketsPostAggregator.java @@ -23,8 +23,9 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeName; import com.google.common.collect.Sets; - import io.druid.java.util.common.IAE; +import io.druid.query.aggregation.post.PostAggregatorIds; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Map; import java.util.Set; @@ -35,8 +36,6 @@ public class BucketsPostAggregator extends ApproximateHistogramPostAggregator private final float bucketSize; private final float offset; - private String fieldName; - @JsonCreator public BucketsPostAggregator( @JsonProperty("name") String name, @@ -51,7 +50,6 @@ public BucketsPostAggregator( throw new IAE("Illegal bucketSize [%s], must be > 0", this.bucketSize); } this.offset = offset; - this.fieldName = fieldName; } @Override @@ -63,7 +61,7 @@ public Set getDependentFields() @Override public Object compute(Map values) { - ApproximateHistogram ah = (ApproximateHistogram) values.get(this.getFieldName()); + ApproximateHistogram ah = (ApproximateHistogram) values.get(fieldName); return ah.toHistogram(bucketSize, offset); } @@ -89,4 +87,14 @@ public String toString() ", offset=" + this.getOffset() + '}'; } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.BUCKETS) + .appendString(fieldName) + .appendFloat(bucketSize) + .appendFloat(offset) + .build(); + } } diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/CustomBucketsPostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/CustomBucketsPostAggregator.java index aa958601a2ad..ee47f22d9138 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/CustomBucketsPostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/CustomBucketsPostAggregator.java @@ -23,6 +23,8 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeName; import com.google.common.collect.Sets; +import io.druid.query.aggregation.post.PostAggregatorIds; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Arrays; import java.util.Map; @@ -55,7 +57,7 @@ public Set getDependentFields() @Override public Object compute(Map values) { - ApproximateHistogram ah = (ApproximateHistogram) values.get(this.getFieldName()); + ApproximateHistogram ah = (ApproximateHistogram) values.get(fieldName); return ah.toHistogram(breaks); } @@ -74,4 +76,13 @@ public String toString() ", breaks=" + Arrays.toString(this.getBreaks()) + '}'; } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.CUSTOM_BUCKETS) + .appendString(fieldName) + .appendFloatArray(breaks) + .build(); + } } diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/EqualBucketsPostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/EqualBucketsPostAggregator.java index 60e9de60e81a..5156aa97bc0e 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/EqualBucketsPostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/EqualBucketsPostAggregator.java @@ -23,8 +23,9 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeName; import com.google.common.collect.Sets; - import io.druid.java.util.common.IAE; +import io.druid.query.aggregation.post.PostAggregatorIds; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Map; import java.util.Set; @@ -33,7 +34,6 @@ public class EqualBucketsPostAggregator extends ApproximateHistogramPostAggregator { private final int numBuckets; - private String fieldName; @JsonCreator public EqualBucketsPostAggregator( @@ -47,7 +47,6 @@ public EqualBucketsPostAggregator( if (this.numBuckets <= 1) { throw new IAE("Illegal number of buckets[%s], must be > 1", this.numBuckets); } - this.fieldName = fieldName; } @Override @@ -59,7 +58,7 @@ public Set getDependentFields() @Override public Object compute(Map values) { - ApproximateHistogram ah = (ApproximateHistogram) values.get(this.getFieldName()); + ApproximateHistogram ah = (ApproximateHistogram) values.get(fieldName); return ah.toHistogram(numBuckets); } @@ -73,9 +72,18 @@ public int getNumBuckets() public String toString() { return "EqualBucketsPostAggregator{" + - "name='" + this.getName() + '\'' + - ", fieldName='" + this.getFieldName() + '\'' + - ", numBuckets=" + this.getNumBuckets() + + "name='" + name + '\'' + + ", fieldName='" + fieldName + '\'' + + ", numBuckets=" + numBuckets + '}'; } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.EQUAL_BUCKETS) + .appendString(fieldName) + .appendInt(numBuckets) + .build(); + } } diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MaxPostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MaxPostAggregator.java index 120f4bb9f088..86aa0f6843c2 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MaxPostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MaxPostAggregator.java @@ -23,6 +23,8 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeName; import com.google.common.collect.Sets; +import io.druid.query.aggregation.post.PostAggregatorIds; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.Map; @@ -40,8 +42,6 @@ public int compare(Object o, Object o1) } }; - private String fieldName; - @JsonCreator public MaxPostAggregator( @JsonProperty("name") String name, @@ -49,7 +49,6 @@ public MaxPostAggregator( ) { super(name, fieldName); - this.fieldName = fieldName; } @Override @@ -67,7 +66,7 @@ public Set getDependentFields() @Override public Object compute(Map values) { - final ApproximateHistogram ah = (ApproximateHistogram) values.get(this.getFieldName()); + final ApproximateHistogram ah = (ApproximateHistogram) values.get(fieldName); return ah.getMax(); } @@ -78,4 +77,12 @@ public String toString() "fieldName='" + fieldName + '\'' + '}'; } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.MAX) + .appendString(fieldName) + .build(); + } } diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MinPostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MinPostAggregator.java index fbf030c6a65a..3badc55b5792 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MinPostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MinPostAggregator.java @@ -23,6 +23,8 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeName; import com.google.common.collect.Sets; +import io.druid.query.aggregation.post.PostAggregatorIds; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.Map; @@ -40,8 +42,6 @@ public int compare(Object o, Object o1) } }; - private String fieldName; - @JsonCreator public MinPostAggregator( @JsonProperty("name") String name, @@ -49,7 +49,6 @@ public MinPostAggregator( ) { super(name, fieldName); - this.fieldName = fieldName; } @Override @@ -67,7 +66,7 @@ public Set getDependentFields() @Override public Object compute(Map values) { - final ApproximateHistogram ah = (ApproximateHistogram) values.get(this.getFieldName()); + final ApproximateHistogram ah = (ApproximateHistogram) values.get(fieldName); return ah.getMin(); } @@ -78,4 +77,12 @@ public String toString() "fieldName='" + fieldName + '\'' + '}'; } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.MIN) + .appendString(fieldName) + .build(); + } } diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilePostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilePostAggregator.java index 7eefe04ff59f..09ef7abbc2df 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilePostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilePostAggregator.java @@ -23,8 +23,9 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeName; import com.google.common.collect.Sets; - import io.druid.java.util.common.IAE; +import io.druid.query.aggregation.post.PostAggregatorIds; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.Map; @@ -43,7 +44,7 @@ public int compare(Object o, Object o1) }; private final float probability; - private String fieldName; + private final String fieldName; @JsonCreator public QuantilePostAggregator( @@ -76,8 +77,8 @@ public Set getDependentFields() @Override public Object compute(Map values) { - final ApproximateHistogram ah = (ApproximateHistogram) values.get(this.getFieldName()); - return ah.getQuantiles(new float[]{this.getProbability()})[0]; + final ApproximateHistogram ah = (ApproximateHistogram) values.get(fieldName); + return ah.getQuantiles(new float[]{probability})[0]; } @JsonProperty @@ -120,4 +121,13 @@ public String toString() ", fieldName='" + fieldName + '\'' + '}'; } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.QUANTILE) + .appendString(fieldName) + .appendFloat(probability) + .build(); + } } diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilesPostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilesPostAggregator.java index 7999a45f341a..b7ad2e84b836 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilesPostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilesPostAggregator.java @@ -23,8 +23,9 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeName; import com.google.common.collect.Sets; - import io.druid.java.util.common.IAE; +import io.druid.query.aggregation.post.PostAggregatorIds; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Arrays; import java.util.Comparator; @@ -35,7 +36,6 @@ public class QuantilesPostAggregator extends ApproximateHistogramPostAggregator { private final float[] probabilities; - private String fieldName; @JsonCreator public QuantilesPostAggregator( @@ -46,7 +46,6 @@ public QuantilesPostAggregator( { super(name, fieldName); this.probabilities = probabilities; - this.fieldName = fieldName; for (float p : probabilities) { if (p < 0 | p > 1) { @@ -70,9 +69,9 @@ public Set getDependentFields() @Override public Object compute(Map values) { - final ApproximateHistogram ah = (ApproximateHistogram) values.get(this.getFieldName()); + final ApproximateHistogram ah = (ApproximateHistogram) values.get(fieldName); - return new Quantiles(this.getProbabilities(), ah.getQuantiles(this.getProbabilities()), ah.getMin(), ah.getMax()); + return new Quantiles(probabilities, ah.getQuantiles(probabilities), ah.getMin(), ah.getMax()); } @JsonProperty @@ -85,9 +84,18 @@ public float[] getProbabilities() public String toString() { return "EqualBucketsPostAggregator{" + - "name='" + this.getName() + '\'' + - ", fieldName='" + this.getFieldName() + '\'' + + "name='" + name + '\'' + + ", fieldName='" + fieldName + '\'' + ", probabilities=" + Arrays.toString(this.getProbabilities()) + '}'; } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.QUANTILES) + .appendString(fieldName) + .appendFloatArray(probabilities) + .build(); + } } diff --git a/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/StandardDeviationPostAggregator.java b/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/StandardDeviationPostAggregator.java index 2bdcb0da9c91..400fef370388 100644 --- a/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/StandardDeviationPostAggregator.java +++ b/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/StandardDeviationPostAggregator.java @@ -26,6 +26,8 @@ import com.google.common.collect.Sets; import io.druid.query.aggregation.PostAggregator; import io.druid.query.aggregation.post.ArithmeticPostAggregator; +import io.druid.query.aggregation.post.PostAggregatorIds; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.Map; @@ -101,4 +103,13 @@ public String toString() ", isVariancePop='" + isVariancePop + '\'' + '}'; } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.STANDARD_DEVIATION) + .appendString(fieldName) + .appendBoolean(isVariancePop) + .build(); + } } diff --git a/processing/src/main/java/io/druid/granularity/AllGranularity.java b/processing/src/main/java/io/druid/granularity/AllGranularity.java index bae98e42b4f0..bcd0b87b8405 100644 --- a/processing/src/main/java/io/druid/granularity/AllGranularity.java +++ b/processing/src/main/java/io/druid/granularity/AllGranularity.java @@ -37,7 +37,7 @@ public long truncate(long offset) } @Override - public byte[] cacheKey() + public byte[] getCacheKey() { return new byte[]{0x7f}; } diff --git a/processing/src/main/java/io/druid/granularity/BaseQueryGranularity.java b/processing/src/main/java/io/druid/granularity/BaseQueryGranularity.java index f6580287c2a6..4330d61d73fe 100644 --- a/processing/src/main/java/io/druid/granularity/BaseQueryGranularity.java +++ b/processing/src/main/java/io/druid/granularity/BaseQueryGranularity.java @@ -31,8 +31,6 @@ public abstract class BaseQueryGranularity extends QueryGranularity public abstract long truncate(long offset); - public abstract byte[] cacheKey(); - public DateTime toDateTime(long offset) { return new DateTime(offset, DateTimeZone.UTC); diff --git a/processing/src/main/java/io/druid/granularity/DurationGranularity.java b/processing/src/main/java/io/druid/granularity/DurationGranularity.java index 54066999369b..14d3ce3c9fec 100644 --- a/processing/src/main/java/io/druid/granularity/DurationGranularity.java +++ b/processing/src/main/java/io/druid/granularity/DurationGranularity.java @@ -83,7 +83,7 @@ public long truncate(final long t) } @Override - public byte[] cacheKey() + public byte[] getCacheKey() { return ByteBuffer.allocate(2 * Longs.BYTES).putLong(length).putLong(origin).array(); } diff --git a/processing/src/main/java/io/druid/granularity/NoneGranularity.java b/processing/src/main/java/io/druid/granularity/NoneGranularity.java index c47130d794a6..0e41bfb0ca57 100644 --- a/processing/src/main/java/io/druid/granularity/NoneGranularity.java +++ b/processing/src/main/java/io/druid/granularity/NoneGranularity.java @@ -34,7 +34,7 @@ public long truncate(long offset) } @Override - public byte[] cacheKey() + public byte[] getCacheKey() { return new byte[]{0x0}; } diff --git a/processing/src/main/java/io/druid/granularity/PeriodGranularity.java b/processing/src/main/java/io/druid/granularity/PeriodGranularity.java index beabc1648106..3b1ed2ae1662 100644 --- a/processing/src/main/java/io/druid/granularity/PeriodGranularity.java +++ b/processing/src/main/java/io/druid/granularity/PeriodGranularity.java @@ -352,7 +352,7 @@ private long truncateMillisPeriod(final long t) } @Override - public byte[] cacheKey() + public byte[] getCacheKey() { return StringUtils.toUtf8(period.toString() + ":" + chronology.getZone().toString() + ":" + origin); } diff --git a/processing/src/main/java/io/druid/granularity/QueryGranularity.java b/processing/src/main/java/io/druid/granularity/QueryGranularity.java index 6d906e6b01f0..30aaca1f1df9 100644 --- a/processing/src/main/java/io/druid/granularity/QueryGranularity.java +++ b/processing/src/main/java/io/druid/granularity/QueryGranularity.java @@ -21,20 +21,19 @@ import com.fasterxml.jackson.annotation.JsonCreator; import io.druid.java.util.common.IAE; +import io.druid.query.cache.Cacheable; import org.joda.time.DateTime; import org.joda.time.ReadableDuration; import java.util.List; import java.util.Objects; -public abstract class QueryGranularity +public abstract class QueryGranularity implements Cacheable { public abstract long next(long offset); public abstract long truncate(long offset); - public abstract byte[] cacheKey(); - public abstract DateTime toDateTime(long offset); public abstract Iterable iterable(final long start, final long end); diff --git a/processing/src/main/java/io/druid/query/aggregation/AggregatorFactory.java b/processing/src/main/java/io/druid/query/aggregation/AggregatorFactory.java index 2bdc2b2298e7..3a2e99355489 100644 --- a/processing/src/main/java/io/druid/query/aggregation/AggregatorFactory.java +++ b/processing/src/main/java/io/druid/query/aggregation/AggregatorFactory.java @@ -20,6 +20,7 @@ package io.druid.query.aggregation; import io.druid.java.util.common.logger.Logger; +import io.druid.query.cache.Cacheable; import io.druid.segment.ColumnSelectorFactory; import java.util.Comparator; @@ -37,7 +38,7 @@ * provided to the Aggregator through the MetricSelector object, so whatever creates that object gets to choose how * the data is actually stored and accessed. */ -public abstract class AggregatorFactory +public abstract class AggregatorFactory implements Cacheable { private static final Logger log = new Logger(AggregatorFactory.class); @@ -115,8 +116,6 @@ public AggregatorFactory getMergingFactory(AggregatorFactory other) throws Aggre public abstract List requiredFields(); - public abstract byte[] getCacheKey(); - public abstract String getTypeName(); /** diff --git a/processing/src/main/java/io/druid/query/aggregation/PostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/PostAggregator.java index d3037e9ec2e9..cdcb41686041 100644 --- a/processing/src/main/java/io/druid/query/aggregation/PostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/PostAggregator.java @@ -19,6 +19,8 @@ package io.druid.query.aggregation; +import io.druid.query.cache.Cacheable; + import java.util.Comparator; import java.util.Map; import java.util.Set; @@ -26,13 +28,13 @@ /** * Functionally similar to an Aggregator. See the Aggregator interface for more comments. */ -public interface PostAggregator +public interface PostAggregator extends Cacheable { - public Set getDependentFields(); + Set getDependentFields(); - public Comparator getComparator(); + Comparator getComparator(); - public Object compute(Map combinedAggregators); + Object compute(Map combinedAggregators); - public String getName(); + String getName(); } diff --git a/processing/src/main/java/io/druid/query/aggregation/hyperloglog/HyperUniqueFinalizingPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/hyperloglog/HyperUniqueFinalizingPostAggregator.java index e84bb5ab5fcf..c197cfb925f3 100644 --- a/processing/src/main/java/io/druid/query/aggregation/hyperloglog/HyperUniqueFinalizingPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/hyperloglog/HyperUniqueFinalizingPostAggregator.java @@ -25,6 +25,8 @@ import com.google.common.collect.Ordering; import com.google.common.collect.Sets; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.aggregation.post.PostAggregatorIds; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.Map; @@ -124,4 +126,12 @@ public String toString() ", fieldName='" + fieldName + '\'' + '}'; } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.HYPER_UNIQUE_FINALIZING) + .appendString(fieldName) + .build(); + } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java index 989118441cb5..bc800c0eedae 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java @@ -26,6 +26,7 @@ import com.google.common.collect.Sets; import io.druid.java.util.common.IAE; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.Iterator; @@ -123,6 +124,16 @@ public String getName() return name; } + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.ARITHMETIC) + .appendString(fnName) + .appendCacheableList(fields) + .appendString(ordering) + .build(); + } + @JsonProperty("fn") public String getFnName() { diff --git a/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java index d0a482c9bce7..343793438c3c 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/ConstantPostAggregator.java @@ -24,6 +24,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Sets; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.Map; @@ -106,11 +107,7 @@ public boolean equals(Object o) ConstantPostAggregator that = (ConstantPostAggregator) o; - if (constantValue != null && that.constantValue != null) { - if (constantValue.doubleValue() != that.constantValue.doubleValue()) { - return false; - } - } else if (constantValue != that.constantValue) { + if (constantValue.doubleValue() != that.constantValue.doubleValue()) { return false; } @@ -125,8 +122,15 @@ public boolean equals(Object o) public int hashCode() { int result = name != null ? name.hashCode() : 0; - result = 31 * result + (constantValue != null ? constantValue.hashCode() : 0); + result = 31 * result + constantValue.hashCode(); return result; } + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.CONSTANT) + .appendDouble(constantValue.doubleValue()) + .build(); + } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/DoubleGreatestPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/DoubleGreatestPostAggregator.java index 8a238847d163..8342dfa2cec6 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/DoubleGreatestPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/DoubleGreatestPostAggregator.java @@ -24,6 +24,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Sets; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.Iterator; @@ -142,4 +143,12 @@ public int hashCode() result = 31 * result + fields.hashCode(); return result; } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.DOUBLE_GREATEST) + .appendCacheableList(fields) + .build(); + } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/DoubleLeastPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/DoubleLeastPostAggregator.java index 88e4f765fed0..a1ac6688242f 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/DoubleLeastPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/DoubleLeastPostAggregator.java @@ -24,6 +24,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Sets; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.Iterator; @@ -142,4 +143,12 @@ public int hashCode() result = 31 * result + fields.hashCode(); return result; } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.DOUBLE_LEAST) + .appendCacheableList(fields) + .build(); + } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/ExpressionPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/ExpressionPostAggregator.java index b917869cd525..85dea1a06b44 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/ExpressionPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/ExpressionPostAggregator.java @@ -26,6 +26,7 @@ import io.druid.math.expr.Expr; import io.druid.math.expr.Parser; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.List; @@ -127,6 +128,15 @@ public String toString() '}'; } + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.EXPRESSION) + .appendString(expression) + .appendString(ordering) + .build(); + } + public static enum Ordering implements Comparator { // ensures the following order: numeric > NaN > Infinite diff --git a/processing/src/main/java/io/druid/query/aggregation/post/FieldAccessPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/FieldAccessPostAggregator.java index 64b369366498..d0867e08111b 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/FieldAccessPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/FieldAccessPostAggregator.java @@ -21,8 +21,10 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Preconditions; import com.google.common.collect.Sets; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.Map; @@ -41,6 +43,7 @@ public FieldAccessPostAggregator( @JsonProperty("fieldName") String fieldName ) { + Preconditions.checkNotNull(fieldName); this.name = name; this.fieldName = fieldName; } @@ -70,6 +73,14 @@ public String getName() return name; } + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.FIELD_ACCESS) + .appendString(fieldName) + .build(); + } + @JsonProperty public String getFieldName() { @@ -97,7 +108,7 @@ public boolean equals(Object o) FieldAccessPostAggregator that = (FieldAccessPostAggregator) o; - if (fieldName != null ? !fieldName.equals(that.fieldName) : that.fieldName != null) { + if (!fieldName.equals(that.fieldName)) { return false; } if (name != null ? !name.equals(that.name) : that.name != null) { @@ -111,7 +122,7 @@ public boolean equals(Object o) public int hashCode() { int result = name != null ? name.hashCode() : 0; - result = 31 * result + (fieldName != null ? fieldName.hashCode() : 0); + result = 31 * result + fieldName.hashCode(); return result; } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/JavaScriptPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/JavaScriptPostAggregator.java index 866bb835b653..83040ef16878 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/JavaScriptPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/JavaScriptPostAggregator.java @@ -24,9 +24,9 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; import com.google.common.collect.Sets; -import io.druid.java.util.common.ISE; import io.druid.js.JavaScriptConfig; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.cache.CacheKeyBuilder; import org.mozilla.javascript.Context; import org.mozilla.javascript.ContextFactory; import org.mozilla.javascript.ScriptableObject; @@ -97,16 +97,12 @@ public JavaScriptPostAggregator( Preconditions.checkNotNull(name, "Must have a valid, non-null post-aggregator name"); Preconditions.checkNotNull(fieldNames, "Must have a valid, non-null fieldNames"); Preconditions.checkNotNull(function, "Must have a valid, non-null function"); + Preconditions.checkState(!config.isDisabled(), "JavaScript is disabled"); this.name = name; this.fieldNames = fieldNames; this.function = function; - - if (config.isDisabled()) { - this.fn = null; - } else { - this.fn = compile(function); - } + this.fn = compile(function); } @Override @@ -124,10 +120,6 @@ public Comparator getComparator() @Override public Object compute(Map combinedAggregators) { - if (fn == null) { - throw new ISE("JavaScript is disabled"); - } - final Object[] args = new Object[fieldNames.size()]; int i = 0; for (String field : fieldNames) { @@ -136,6 +128,15 @@ public Object compute(Map combinedAggregators) return fn.apply(args); } + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.JAVA_SCRIPT) + .appendStringList(fieldNames) + .appendString(function) + .build(); + } + @JsonProperty @Override public String getName() @@ -167,16 +168,16 @@ public boolean equals(Object o) JavaScriptPostAggregator that = (JavaScriptPostAggregator) o; - if (fieldNames != null ? !fieldNames.equals(that.fieldNames) : that.fieldNames != null) { + if (!fieldNames.equals(that.fieldNames)) { return false; } - if (fn != null ? !fn.equals(that.fn) : that.fn != null) { + if (!fn.equals(that.fn)) { return false; } - if (function != null ? !function.equals(that.function) : that.function != null) { + if (!function.equals(that.function)) { return false; } - if (name != null ? !name.equals(that.name) : that.name != null) { + if (!name.equals(that.name)) { return false; } @@ -186,10 +187,10 @@ public boolean equals(Object o) @Override public int hashCode() { - int result = name != null ? name.hashCode() : 0; - result = 31 * result + (fieldNames != null ? fieldNames.hashCode() : 0); - result = 31 * result + (function != null ? function.hashCode() : 0); - result = 31 * result + (fn != null ? fn.hashCode() : 0); + int result = name.hashCode(); + result = 31 * result + fieldNames.hashCode(); + result = 31 * result + function.hashCode(); + result = 31 * result + fn.hashCode(); return result; } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/LongGreatestPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/LongGreatestPostAggregator.java index 8f9e5cdf31be..0b00e69fb1a7 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/LongGreatestPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/LongGreatestPostAggregator.java @@ -25,6 +25,7 @@ import com.google.common.collect.Sets; import com.google.common.primitives.Longs; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.Iterator; @@ -143,4 +144,12 @@ public int hashCode() result = 31 * result + fields.hashCode(); return result; } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.LONG_GREATEST) + .appendCacheableList(fields) + .build(); + } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/LongLeastPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/LongLeastPostAggregator.java index 80f4325857cd..64621bf6a52b 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/LongLeastPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/LongLeastPostAggregator.java @@ -25,6 +25,7 @@ import com.google.common.collect.Sets; import com.google.common.primitives.Longs; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.cache.CacheKeyBuilder; import java.util.Comparator; import java.util.Iterator; @@ -143,4 +144,12 @@ public int hashCode() result = 31 * result + fields.hashCode(); return result; } + + @Override + public byte[] getCacheKey() + { + return new CacheKeyBuilder(PostAggregatorIds.LONG_LEAST) + .appendCacheableList(fields) + .build(); + } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/PostAggregatorIds.java b/processing/src/main/java/io/druid/query/aggregation/post/PostAggregatorIds.java new file mode 100644 index 000000000000..4da712891489 --- /dev/null +++ b/processing/src/main/java/io/druid/query/aggregation/post/PostAggregatorIds.java @@ -0,0 +1,44 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets 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 io.druid.query.aggregation.post; + +public class PostAggregatorIds +{ + public static final byte ARITHMETIC = 0; + public static final byte BUCKETS = 1; + public static final byte CONSTANT = 2; + public static final byte CUSTOM_BUCKETS = 3; + public static final byte DOUBLE_GREATEST = 4; + public static final byte DOUBLE_LEAST = 5; + public static final byte EQUAL_BUCKETS = 6; + public static final byte EXPRESSION = 7; + public static final byte FIELD_ACCESS = 8; + public static final byte HYPER_UNIQUE_FINALIZING = 9; + public static final byte JAVA_SCRIPT = 10; + public static final byte LONG_GREATEST = 11; + public static final byte LONG_LEAST = 12; + public static final byte MAX = 13; + public static final byte MIN = 14; + public static final byte QUANTILE = 15; + public static final byte QUANTILES = 16; + public static final byte SKETCH_ESTIMATE = 17; + public static final byte SKETCH_SET = 18; + public static final byte STANDARD_DEVIATION = 19; +} diff --git a/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java b/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java new file mode 100644 index 000000000000..48b253f9b8d7 --- /dev/null +++ b/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java @@ -0,0 +1,175 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets 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 io.druid.query.cache; + +import com.google.common.collect.Lists; +import com.google.common.primitives.Doubles; +import com.google.common.primitives.Floats; +import com.google.common.primitives.Ints; +import io.druid.common.utils.StringUtils; + +import javax.annotation.Nullable; +import java.nio.ByteBuffer; +import java.util.List; + +public class CacheKeyBuilder +{ + static final byte[] DEFAULT_SEPARATOR = new byte[]{(byte) 0xFF}; + public static final byte[] EMPTY_BYTES = new byte[0]; + + private static byte[] byteToByteArray(byte input) + { + return new byte[]{input}; + } + + private static byte[] stringToByteArray(String input) + { + return StringUtils.toUtf8WithNullToEmpty(input); + } + + private static byte[] booleanToByteArray(boolean input) + { + return new byte[]{(byte) (input ? 1 : 0)}; + } + + private static byte[] intToByteArray(int input) + { + return Ints.toByteArray(input); + } + + private static byte[] floatToByteArray(float input) + { + return ByteBuffer.allocate(Floats.BYTES).putFloat(input).array(); + } + + private static byte[] floatArrayToByteArray(float[] input) + { + final ByteBuffer buffer = ByteBuffer.allocate(Floats.BYTES * input.length); + buffer.asFloatBuffer().put(input); + return buffer.array(); + } + + private static byte[] doubleToByteArray(double input) + { + return ByteBuffer.allocate(Doubles.BYTES).putDouble(input).array(); + } + + private final List items = Lists.newArrayList(); + private final byte id; + private final byte[] separator; + private int size; + + public CacheKeyBuilder(byte id) + { + this(id, DEFAULT_SEPARATOR); + } + + public CacheKeyBuilder(byte id, byte[] separator) + { + this.id = id; + this.size = 1; + this.separator = separator; + } + + public CacheKeyBuilder appendByte(byte input) + { + appendItem(byteToByteArray(input)); + return this; + } + + public CacheKeyBuilder appendString(@Nullable String input) + { + appendItem(stringToByteArray(input)); + return this; + } + + public CacheKeyBuilder appendStringList(List input) + { + for (String eachStr : input) { + appendItem(stringToByteArray(eachStr)); + } + return this; + } + + public CacheKeyBuilder appendBoolean(boolean input) + { + appendItem(booleanToByteArray(input)); + return this; + } + + public CacheKeyBuilder appendInt(int input) + { + appendItem(intToByteArray(input)); + return this; + } + + public CacheKeyBuilder appendFloat(float input) + { + appendItem(floatToByteArray(input)); + return this; + } + + public CacheKeyBuilder appendDouble(double input) + { + appendItem(doubleToByteArray(input)); + return this; + } + + public CacheKeyBuilder appendFloatArray(float[] input) + { + appendItem(floatArrayToByteArray(input)); + return this; + } + + public CacheKeyBuilder appendCacheable(@Nullable Cacheable input) + { + appendItem(input == null ? EMPTY_BYTES : input.getCacheKey()); + return this; + } + + public CacheKeyBuilder appendCacheableList(List inputs) + { + for (Cacheable input : inputs) { + appendItem(input.getCacheKey()); + } + return this; + } + + private void appendItem(byte[] item) + { + items.add(item); + size += item.length; + } + + public byte[] build() + { + final ByteBuffer buffer = ByteBuffer.allocate(size + separator.length * (items.size() - 1)); + buffer.put(id); + + for (int i = 0; i < items.size(); i++) { + if (i == items.size() - 1) { + buffer.put(items.get(i)); + } else { + buffer.put(items.get(i)).put(separator); + } + } + return buffer.array(); + } +} diff --git a/processing/src/main/java/io/druid/query/QueryCacheHelper.java b/processing/src/main/java/io/druid/query/cache/Cacheable.java similarity index 52% rename from processing/src/main/java/io/druid/query/QueryCacheHelper.java rename to processing/src/main/java/io/druid/query/cache/Cacheable.java index f99c4ee0c85b..b058fe0699ff 100644 --- a/processing/src/main/java/io/druid/query/QueryCacheHelper.java +++ b/processing/src/main/java/io/druid/query/cache/Cacheable.java @@ -17,34 +17,14 @@ * under the License. */ -package io.druid.query; +package io.druid.query.cache; -import com.google.common.collect.Lists; -import io.druid.query.aggregation.AggregatorFactory; - -import java.nio.ByteBuffer; -import java.util.List; - -/** - */ -public class QueryCacheHelper +public interface Cacheable { - public static byte[] computeAggregatorBytes(List aggregatorSpecs) - { - List cacheKeySet = Lists.newArrayListWithCapacity(aggregatorSpecs.size()); - - int totalSize = 0; - for (AggregatorFactory spec : aggregatorSpecs) { - final byte[] cacheKey = spec.getCacheKey(); - cacheKeySet.add(cacheKey); - totalSize += cacheKey.length; - } - - ByteBuffer retVal = ByteBuffer.allocate(totalSize); - for (byte[] bytes : cacheKeySet) { - retVal.put(bytes); - } - return retVal.array(); - } - + /** + * Get a byte array used as a cache key. + * + * @return a cache key + */ + byte[] getCacheKey(); } diff --git a/processing/src/main/java/io/druid/query/dimension/DimensionSpec.java b/processing/src/main/java/io/druid/query/dimension/DimensionSpec.java index 21b9201d8c1c..a0928d11121f 100644 --- a/processing/src/main/java/io/druid/query/dimension/DimensionSpec.java +++ b/processing/src/main/java/io/druid/query/dimension/DimensionSpec.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; +import io.druid.query.cache.Cacheable; import io.druid.query.extraction.ExtractionFn; import io.druid.segment.DimensionSelector; @@ -34,7 +35,7 @@ @JsonSubTypes.Type(name = "listFiltered", value = ListFilteredDimensionSpec.class), @JsonSubTypes.Type(name = "lookup", value = LookupDimensionSpec.class) }) -public interface DimensionSpec +public interface DimensionSpec extends Cacheable { String getDimension(); @@ -46,7 +47,5 @@ public interface DimensionSpec DimensionSelector decorate(DimensionSelector selector); - byte[] getCacheKey(); - boolean preservesOrdering(); } diff --git a/processing/src/main/java/io/druid/query/dimension/LookupDimensionSpec.java b/processing/src/main/java/io/druid/query/dimension/LookupDimensionSpec.java index fd41aeeba03c..335b2b39e1c5 100644 --- a/processing/src/main/java/io/druid/query/dimension/LookupDimensionSpec.java +++ b/processing/src/main/java/io/druid/query/dimension/LookupDimensionSpec.java @@ -174,7 +174,7 @@ public byte[] getCacheKey() .put(DimFilterUtils.STRING_SEPARATOR) .put(replaceWithBytes) .put(DimFilterUtils.STRING_SEPARATOR) - .put(retainMissingValue == true ? (byte) 1 : (byte) 0) + .put(retainMissingValue ? (byte) 1 : (byte) 0) .array(); } diff --git a/processing/src/main/java/io/druid/query/extraction/TimeFormatExtractionFn.java b/processing/src/main/java/io/druid/query/extraction/TimeFormatExtractionFn.java index a5eae36ff6f5..da9d2126ec5d 100644 --- a/processing/src/main/java/io/druid/query/extraction/TimeFormatExtractionFn.java +++ b/processing/src/main/java/io/druid/query/extraction/TimeFormatExtractionFn.java @@ -107,7 +107,7 @@ public boolean isAsMillis() public byte[] getCacheKey() { final byte[] exprBytes = StringUtils.toUtf8(format + "\u0001" + tz.getID() + "\u0001" + locale.toLanguageTag()); - final byte[] granularityCacheKey = granularity.cacheKey(); + final byte[] granularityCacheKey = granularity.getCacheKey(); return ByteBuffer.allocate(4 + exprBytes.length + granularityCacheKey.length) .put(ExtractionCacheHelper.CACHE_TYPE_ID_TIME_FORMAT) .put(exprBytes) diff --git a/processing/src/main/java/io/druid/query/filter/DimFilter.java b/processing/src/main/java/io/druid/query/filter/DimFilter.java index 4de0c061fccf..517b6cd9352c 100644 --- a/processing/src/main/java/io/druid/query/filter/DimFilter.java +++ b/processing/src/main/java/io/druid/query/filter/DimFilter.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.google.common.collect.RangeSet; +import io.druid.query.cache.Cacheable; /** */ @@ -41,15 +42,13 @@ @JsonSubTypes.Type(name="interval", value=IntervalDimFilter.class), @JsonSubTypes.Type(name="like", value=LikeDimFilter.class) }) -public interface DimFilter +public interface DimFilter extends Cacheable { - public byte[] getCacheKey(); - /** * @return Returns an optimized filter. * returning the same filter can be a straightforward default implementation. */ - public DimFilter optimize(); + DimFilter optimize(); /** * Returns a Filter that implements this DimFilter. This does not generally involve optimizing the DimFilter, @@ -57,7 +56,7 @@ public interface DimFilter * * @return a Filter that implements this DimFilter, or null if this DimFilter is a no-op. */ - public Filter toFilter(); + Filter toFilter(); /** * Returns a RangeSet that represents the possible range of the input dimension for this DimFilter.This is @@ -72,5 +71,5 @@ public interface DimFilter * @return a RangeSet that represent the possible range of the input dimension, or null if it is not possible to * determine for this DimFilter. */ - public RangeSet getDimensionRangeSet(String dimension); + RangeSet getDimensionRangeSet(String dimension); } diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java index b9c9e8ba13bd..8b196b037d3b 100644 --- a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java @@ -46,7 +46,6 @@ import io.druid.query.DruidMetrics; import io.druid.query.IntervalChunkingQueryRunnerDecorator; import io.druid.query.Query; -import io.druid.query.QueryCacheHelper; import io.druid.query.QueryDataSource; import io.druid.query.QueryRunner; import io.druid.query.QueryToolChest; @@ -54,10 +53,10 @@ import io.druid.query.aggregation.AggregatorFactory; import io.druid.query.aggregation.MetricManipulationFn; import io.druid.query.aggregation.MetricManipulatorFns; +import io.druid.query.cache.CacheKeyBuilder; import io.druid.query.dimension.DefaultDimensionSpec; import io.druid.query.dimension.DimensionSpec; import io.druid.query.extraction.ExtractionFn; -import io.druid.query.filter.DimFilter; import io.druid.query.groupby.strategy.GroupByStrategySelector; import org.joda.time.DateTime; @@ -356,45 +355,13 @@ public CacheStrategy getCacheStrategy(final GroupByQu @Override public byte[] computeCacheKey(GroupByQuery query) { - final DimFilter dimFilter = query.getDimFilter(); - final byte[] filterBytes = dimFilter == null ? new byte[]{} : dimFilter.getCacheKey(); - final byte[] aggregatorBytes = QueryCacheHelper.computeAggregatorBytes(query.getAggregatorSpecs()); - final byte[] granularityBytes = query.getGranularity().cacheKey(); - final byte[][] dimensionsBytes = new byte[query.getDimensions().size()][]; - int dimensionsBytesSize = 0; - int index = 0; - for (DimensionSpec dimension : query.getDimensions()) { - dimensionsBytes[index] = dimension.getCacheKey(); - dimensionsBytesSize += dimensionsBytes[index].length; - ++index; - } - final byte[] havingBytes = query.getHavingSpec() == null ? new byte[]{} : query.getHavingSpec().getCacheKey(); - final byte[] limitBytes = query.getLimitSpec().getCacheKey(); - - ByteBuffer buffer = ByteBuffer - .allocate( - 2 - + granularityBytes.length - + filterBytes.length - + aggregatorBytes.length - + dimensionsBytesSize - + havingBytes.length - + limitBytes.length - ) - .put(GROUPBY_QUERY) - .put(CACHE_STRATEGY_VERSION) - .put(granularityBytes) - .put(filterBytes) - .put(aggregatorBytes); - - for (byte[] dimensionsByte : dimensionsBytes) { - buffer.put(dimensionsByte); - } - - return buffer - .put(havingBytes) - .put(limitBytes) - .array(); + return new CacheKeyBuilder(GROUPBY_QUERY, CacheKeyBuilder.EMPTY_BYTES) + .appendByte(CACHE_STRATEGY_VERSION) + .appendCacheable(query.getGranularity()) + .appendCacheable(query.getDimFilter()) + .appendCacheableList(query.getAggregatorSpecs()) + .appendCacheableList(query.getDimensions()) + .build(); } @Override diff --git a/processing/src/main/java/io/druid/query/search/SearchQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/search/SearchQueryQueryToolChest.java index 666263db2251..4a0103226301 100644 --- a/processing/src/main/java/io/druid/query/search/SearchQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/search/SearchQueryQueryToolChest.java @@ -159,7 +159,7 @@ public byte[] computeCacheKey(SearchQuery query) final DimFilter dimFilter = query.getDimensionsFilter(); final byte[] filterBytes = dimFilter == null ? new byte[]{} : dimFilter.getCacheKey(); final byte[] querySpecBytes = query.getQuery().getCacheKey(); - final byte[] granularityBytes = query.getGranularity().cacheKey(); + final byte[] granularityBytes = query.getGranularity().getCacheKey(); final List dimensionSpecs = query.getDimensions() != null ? query.getDimensions() : Collections.emptyList(); diff --git a/processing/src/main/java/io/druid/query/select/SelectQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/select/SelectQueryQueryToolChest.java index 2bb009835e2d..f65918846966 100644 --- a/processing/src/main/java/io/druid/query/select/SelectQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/select/SelectQueryQueryToolChest.java @@ -164,7 +164,7 @@ public byte[] computeCacheKey(SelectQuery query) { final DimFilter dimFilter = query.getDimensionsFilter(); final byte[] filterBytes = dimFilter == null ? new byte[]{} : dimFilter.getCacheKey(); - final byte[] granularityBytes = query.getGranularity().cacheKey(); + final byte[] granularityBytes = query.getGranularity().getCacheKey(); final List dimensionSpecs = query.getDimensions() != null ? query.getDimensions() : Collections.emptyList(); diff --git a/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java index 090220e11e9e..e0ab7931e240 100644 --- a/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java @@ -33,7 +33,6 @@ import io.druid.query.DruidMetrics; import io.druid.query.IntervalChunkingQueryRunnerDecorator; import io.druid.query.Query; -import io.druid.query.QueryCacheHelper; import io.druid.query.QueryRunner; import io.druid.query.QueryToolChest; import io.druid.query.Result; @@ -42,11 +41,10 @@ import io.druid.query.aggregation.AggregatorFactory; import io.druid.query.aggregation.MetricManipulationFn; import io.druid.query.aggregation.PostAggregator; -import io.druid.query.filter.DimFilter; +import io.druid.query.cache.CacheKeyBuilder; import org.joda.time.DateTime; import javax.annotation.Nullable; -import java.nio.ByteBuffer; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -132,22 +130,13 @@ public CacheStrategy, Object, TimeseriesQuery> get @Override public byte[] computeCacheKey(TimeseriesQuery query) { - final DimFilter dimFilter = query.getDimensionsFilter(); - final byte[] filterBytes = dimFilter == null ? new byte[]{} : dimFilter.getCacheKey(); - final byte[] aggregatorBytes = QueryCacheHelper.computeAggregatorBytes(query.getAggregatorSpecs()); - final byte[] granularityBytes = query.getGranularity().cacheKey(); - final byte descending = query.isDescending() ? (byte) 1 : 0; - final byte skipEmptyBuckets = query.isSkipEmptyBuckets() ? (byte) 1 : 0; - - return ByteBuffer - .allocate(3 + granularityBytes.length + filterBytes.length + aggregatorBytes.length) - .put(TIMESERIES_QUERY) - .put(descending) - .put(skipEmptyBuckets) - .put(granularityBytes) - .put(filterBytes) - .put(aggregatorBytes) - .array(); + return new CacheKeyBuilder(TIMESERIES_QUERY, CacheKeyBuilder.EMPTY_BYTES) + .appendBoolean(query.isDescending()) + .appendBoolean(query.isSkipEmptyBuckets()) + .appendCacheable(query.getGranularity()) + .appendCacheable(query.getDimensionsFilter()) + .appendCacheableList(query.getAggregatorSpecs()) + .build(); } @Override diff --git a/processing/src/main/java/io/druid/query/topn/TopNMetricSpec.java b/processing/src/main/java/io/druid/query/topn/TopNMetricSpec.java index 2c2894125974..9bed6cdcdb19 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNMetricSpec.java +++ b/processing/src/main/java/io/druid/query/topn/TopNMetricSpec.java @@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo; import io.druid.query.aggregation.AggregatorFactory; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.cache.Cacheable; import io.druid.query.dimension.DimensionSpec; import org.joda.time.DateTime; @@ -39,13 +40,13 @@ @JsonSubTypes.Type(name = "inverted", value = InvertedTopNMetricSpec.class), @JsonSubTypes.Type(name = "dimension", value = DimensionTopNMetricSpec.class), }) -public interface TopNMetricSpec +public interface TopNMetricSpec extends Cacheable { - public void verifyPreconditions(List aggregatorSpecs, List postAggregatorSpecs); + void verifyPreconditions(List aggregatorSpecs, List postAggregatorSpecs); - public Comparator getComparator(List aggregatorSpecs, List postAggregatorSpecs); + Comparator getComparator(List aggregatorSpecs, List postAggregatorSpecs); - public TopNResultBuilder getResultBuilder( + TopNResultBuilder getResultBuilder( DateTime timestamp, DimensionSpec dimSpec, int threshold, @@ -54,13 +55,11 @@ public TopNResultBuilder getResultBuilder( List postAggs ); - public byte[] getCacheKey(); + TopNMetricSpecBuilder configureOptimizer(TopNMetricSpecBuilder builder); - public TopNMetricSpecBuilder configureOptimizer(TopNMetricSpecBuilder builder); + void initTopNAlgorithmSelector(TopNAlgorithmSelector selector); - public void initTopNAlgorithmSelector(TopNAlgorithmSelector selector); + String getMetricName(DimensionSpec dimSpec); - public String getMetricName(DimensionSpec dimSpec); - - public boolean canBeOptimizedUnordered(); + boolean canBeOptimizedUnordered(); } diff --git a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java index 052f14710d5c..51ca39eb2941 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java @@ -25,7 +25,6 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Ordering; -import com.google.common.primitives.Ints; import com.google.inject.Inject; import com.metamx.emitter.service.ServiceMetricEvent; import io.druid.granularity.QueryGranularity; @@ -39,7 +38,6 @@ import io.druid.query.DruidMetrics; import io.druid.query.IntervalChunkingQueryRunnerDecorator; import io.druid.query.Query; -import io.druid.query.QueryCacheHelper; import io.druid.query.QueryRunner; import io.druid.query.QueryToolChest; import io.druid.query.Result; @@ -49,13 +47,12 @@ import io.druid.query.aggregation.AggregatorUtil; import io.druid.query.aggregation.MetricManipulationFn; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.cache.CacheKeyBuilder; import io.druid.query.dimension.DefaultDimensionSpec; import io.druid.query.dimension.DimensionSpec; -import io.druid.query.filter.DimFilter; import org.joda.time.DateTime; import javax.annotation.Nullable; -import java.nio.ByteBuffer; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -304,29 +301,17 @@ public CacheStrategy, Object, TopNQuery> getCacheStrateg ); @Override - public byte[] computeCacheKey(TopNQuery query) + public byte[] computeCacheKey(TopNQuery query) // always called { - final byte[] dimensionSpecBytes = query.getDimensionSpec().getCacheKey(); - final byte[] metricSpecBytes = query.getTopNMetricSpec().getCacheKey(); - - final DimFilter dimFilter = query.getDimensionsFilter(); - final byte[] filterBytes = dimFilter == null ? new byte[]{} : dimFilter.getCacheKey(); - final byte[] aggregatorBytes = QueryCacheHelper.computeAggregatorBytes(query.getAggregatorSpecs()); - final byte[] granularityBytes = query.getGranularity().cacheKey(); - - return ByteBuffer - .allocate( - 1 + dimensionSpecBytes.length + metricSpecBytes.length + 4 + - granularityBytes.length + filterBytes.length + aggregatorBytes.length - ) - .put(TOPN_QUERY) - .put(dimensionSpecBytes) - .put(metricSpecBytes) - .put(Ints.toByteArray(query.getThreshold())) - .put(granularityBytes) - .put(filterBytes) - .put(aggregatorBytes) - .array(); + return new CacheKeyBuilder(TOPN_QUERY, CacheKeyBuilder.EMPTY_BYTES) + .appendCacheable(query.getDimensionSpec()) + .appendCacheable(query.getTopNMetricSpec()) + .appendInt(query.getThreshold()) + .appendCacheable(query.getGranularity()) + .appendCacheable(query.getDimensionsFilter()) + .appendCacheableList(query.getAggregatorSpecs()) + .appendCacheableList(query.getPostAggregatorSpecs()) + .build(); } @Override @@ -336,7 +321,7 @@ public TypeReference getCacheObjectClazz() } @Override - public Function, Object> prepareForCache() + public Function, Object> prepareForCache() // prepare for cache population. called on cache misses { return new Function, Object>() { @@ -364,7 +349,7 @@ public Object apply(final Result input) } @Override - public Function> pullFromCache() + public Function> pullFromCache() // called on cache hits { return new Function>() { diff --git a/processing/src/test/java/io/druid/query/aggregation/post/JavaScriptPostAggregatorTest.java b/processing/src/test/java/io/druid/query/aggregation/post/JavaScriptPostAggregatorTest.java index f4aa330be325..3b5e190aea5c 100644 --- a/processing/src/test/java/io/druid/query/aggregation/post/JavaScriptPostAggregatorTest.java +++ b/processing/src/test/java/io/druid/query/aggregation/post/JavaScriptPostAggregatorTest.java @@ -58,19 +58,14 @@ public void testCompute() @Test public void testComputeJavaScriptNotAllowed() { - JavaScriptPostAggregator javaScriptPostAggregator; - String absPercentFunction = "function(delta, total) { return 100 * Math.abs(delta) / total; }"; - javaScriptPostAggregator = new JavaScriptPostAggregator( + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage("JavaScript is disabled"); + new JavaScriptPostAggregator( "absPercent", Lists.newArrayList("delta", "total"), absPercentFunction, new JavaScriptConfig(true) ); - - expectedException.expect(IllegalStateException.class); - expectedException.expectMessage("JavaScript is disabled"); - javaScriptPostAggregator.compute(Maps.newHashMap()); - Assert.assertTrue(false); } } diff --git a/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java b/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java new file mode 100644 index 000000000000..21bbbf4d6d0e --- /dev/null +++ b/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java @@ -0,0 +1,121 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets 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 io.druid.query.cache; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; +import com.google.common.primitives.Doubles; +import com.google.common.primitives.Floats; +import com.google.common.primitives.Ints; +import io.druid.common.utils.StringUtils; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +@RunWith(Parameterized.class) +public class CacheKeyBuilderTest +{ + @Parameters + public static List params() + { + return ImmutableList.of( + new Object[]{CacheKeyBuilder.DEFAULT_SEPARATOR}, + new Object[]{CacheKeyBuilder.EMPTY_BYTES}, + new Object[]{new byte[] {'\001'}} + ); + } + + private final byte[] separator; + + public CacheKeyBuilderTest(byte[] separator) + { + this.separator = separator; + } + + @Test + public void testCacheKeyBuilder() + { + final Cacheable cacheable = new Cacheable() + { + @Override + public byte[] getCacheKey() + { + return new byte[]{10, 20}; + } + }; + + final byte[] actual = new CacheKeyBuilder((byte) 10, separator) + .appendBoolean(false) + .appendString("test") + .appendInt(10) + .appendFloat(0.1f) + .appendDouble(2.3) + .appendFloatArray(new float[]{10.0f, 11.0f}) + .appendStringList(Lists.newArrayList("test1", "test2")) + .appendCacheable(cacheable) + .appendCacheable(null) + .build(); + + final int expectedSize = 1 // id + + 1 // bool + + 4 // 'test' + + Ints.BYTES // 10 + + Floats.BYTES // 0.1f + + Doubles.BYTES // 2.3 + + Floats.BYTES * 2 // 10.0f, 11.0f + + 5 * 2 // 'test1' 'test2' + + cacheable.getCacheKey().length // cacheable + + 9 * separator.length; // separators + assertEquals(expectedSize, actual.length); + + final byte[] expected = ByteBuffer.allocate(expectedSize) + .put((byte) 10) + .put((byte) 0) + .put(separator) + .put(StringUtils.toUtf8("test")) + .put(separator) + .putInt(10) + .put(separator) + .putFloat(0.1f) + .put(separator) + .putDouble(2.3) + .put(separator) + .putFloat(10.0f) + .putFloat(11.0f) + .put(separator) + .put(StringUtils.toUtf8("test1")) + .put(separator) + .put(StringUtils.toUtf8("test2")) + .put(separator) + .put(cacheable.getCacheKey()) + .put(separator) + .array(); + + assertTrue(Arrays.equals(expected, actual)); + } +} diff --git a/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java b/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java index e651bbedee64..02d28c0b000f 100644 --- a/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java +++ b/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java @@ -36,6 +36,10 @@ import io.druid.query.TestQueryRunners; import io.druid.query.aggregation.AggregatorFactory; import io.druid.query.aggregation.CountAggregatorFactory; +import io.druid.query.aggregation.PostAggregator; +import io.druid.query.aggregation.post.ArithmeticPostAggregator; +import io.druid.query.aggregation.post.ConstantPostAggregator; +import io.druid.query.aggregation.post.FieldAccessPostAggregator; import io.druid.query.dimension.DefaultDimensionSpec; import io.druid.query.spec.MultipleIntervalSegmentSpec; import io.druid.segment.IncrementalIndexSegment; @@ -73,7 +77,7 @@ public void testCacheStrategy() throws Exception null, QueryGranularities.ALL, ImmutableList.of(new CountAggregatorFactory("metric1")), - null, + ImmutableList.of(new ConstantPostAggregator("post", 10)), null ) ); @@ -106,6 +110,75 @@ public void testCacheStrategy() throws Exception Assert.assertEquals(result, fromCacheResult); } + @Test + public void testComputeCacheKeyWithDifferentPostAgg() throws Exception + { + final TopNQuery query1 = new TopNQuery( + new TableDataSource("dummy"), + new DefaultDimensionSpec("test", "test"), + new NumericTopNMetricSpec("post"), + 3, + new MultipleIntervalSegmentSpec( + ImmutableList.of( + new Interval( + "2015-01-01/2015-01-02" + ) + ) + ), + null, + QueryGranularities.ALL, + ImmutableList.of(new CountAggregatorFactory("metric1")), + ImmutableList.of(new ConstantPostAggregator("post", 10)), + null + ); + + final TopNQuery query2 = new TopNQuery( + new TableDataSource("dummy"), + new DefaultDimensionSpec("test", "test"), + new NumericTopNMetricSpec("post"), + 3, + new MultipleIntervalSegmentSpec( + ImmutableList.of( + new Interval( + "2015-01-01/2015-01-02" + ) + ) + ), + null, + QueryGranularities.ALL, + ImmutableList.of(new CountAggregatorFactory("metric1")), + ImmutableList.of( + new ArithmeticPostAggregator( + "post", + "+", + ImmutableList.of( + new FieldAccessPostAggregator( + null, + "metric1" + ), + new FieldAccessPostAggregator( + null, + "metric1" + ) + ) + ) + ), + null + ); + + final CacheStrategy, Object, TopNQuery> strategy1 = new TopNQueryQueryToolChest( + null, + null + ).getCacheStrategy(query1); + + final CacheStrategy, Object, TopNQuery> strategy2 = new TopNQueryQueryToolChest( + null, + null + ).getCacheStrategy(query2); + + Assert.assertFalse(Arrays.equals(strategy1.computeCacheKey(query1), strategy2.computeCacheKey(query2))); + } + @Test public void testMinTopNThreshold() throws Exception { diff --git a/server/src/test/java/io/druid/client/CachingClusteredClientTest.java b/server/src/test/java/io/druid/client/CachingClusteredClientTest.java index 9a9e29a8ce38..f1ccfc5d4cf2 100644 --- a/server/src/test/java/io/druid/client/CachingClusteredClientTest.java +++ b/server/src/test/java/io/druid/client/CachingClusteredClientTest.java @@ -24,6 +24,7 @@ import com.fasterxml.jackson.dataformat.smile.SmileFactory; import com.google.common.base.Charsets; import com.google.common.base.Function; +import com.google.common.base.Preconditions; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -41,7 +42,6 @@ import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.SettableFuture; - import io.druid.client.cache.Cache; import io.druid.client.cache.CacheConfig; import io.druid.client.cache.MapCache; @@ -52,8 +52,8 @@ import io.druid.data.input.MapBasedRow; import io.druid.data.input.Row; import io.druid.granularity.PeriodGranularity; -import io.druid.granularity.QueryGranularity; import io.druid.granularity.QueryGranularities; +import io.druid.granularity.QueryGranularity; import io.druid.jackson.DefaultObjectMapper; import io.druid.java.util.common.ISE; import io.druid.java.util.common.Pair; @@ -205,7 +205,7 @@ public class CachingClusteredClientTest ) ); private static final List RENAMED_AGGS = Arrays.asList( - new CountAggregatorFactory("rows2"), + new CountAggregatorFactory("rows"), new LongSumAggregatorFactory("imps", "imps"), new LongSumAggregatorFactory("impers2", "imps") ); @@ -775,13 +775,13 @@ client, new TopNQueryQueryToolChest( runner, builder.build(), new Interval("2011-01-01/2011-01-02"), - makeTopNResults(new DateTime("2011-01-01"), "a", 50, 5000, "b", 50, 4999, "c", 50, 4998), + makeTopNResultsWithoutRename(new DateTime("2011-01-01"), "a", 50, 5000, "b", 50, 4999, "c", 50, 4998), new Interval("2011-01-02/2011-01-03"), - makeTopNResults(new DateTime("2011-01-02"), "a", 50, 4997, "b", 50, 4996, "c", 50, 4995), + makeTopNResultsWithoutRename(new DateTime("2011-01-02"), "a", 50, 4997, "b", 50, 4996, "c", 50, 4995), new Interval("2011-01-05/2011-01-10"), - makeTopNResults( + makeTopNResultsWithoutRename( new DateTime("2011-01-05"), "a", 50, 4994, "b", 50, 4993, "c", 50, 4992, new DateTime("2011-01-06"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, new DateTime("2011-01-07"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, @@ -790,7 +790,7 @@ client, new TopNQueryQueryToolChest( ), new Interval("2011-01-05/2011-01-10"), - makeTopNResults( + makeTopNResultsWithoutRename( new DateTime("2011-01-05T01"), "a", 50, 4994, "b", 50, 4993, "c", 50, 4992, new DateTime("2011-01-06T01"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, new DateTime("2011-01-07T01"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, @@ -818,7 +818,7 @@ client, new TopNQueryQueryToolChest( builder.intervals("2011-01-01/2011-01-10") .metric("imps") .aggregators(RENAMED_AGGS) - .postAggregators(RENAMED_POST_AGGS) + .postAggregators(POST_AGGS) .build(), context ) @@ -852,7 +852,7 @@ client, new TopNQueryQueryToolChest( runner, builder.build(), new Interval("2011-11-04/2011-11-08"), - makeTopNResults( + makeTopNResultsWithoutRename( new DateTime("2011-11-04", TIMEZONE), "a", 50, 4994, "b", 50, 4993, "c", 50, 4992, new DateTime("2011-11-05", TIMEZONE), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, new DateTime("2011-11-06", TIMEZONE), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, @@ -872,7 +872,7 @@ client, new TopNQueryQueryToolChest( builder.intervals("2011-11-04/2011-11-08") .metric("imps") .aggregators(RENAMED_AGGS) - .postAggregators(RENAMED_POST_AGGS) + .postAggregators(POST_AGGS) .build(), context ) @@ -885,14 +885,14 @@ public void testOutOfOrderSequenceMerging() throws Exception List>> sequences = ImmutableList.of( Sequences.simple( - makeTopNResults( + makeTopNResultsWithoutRename( new DateTime("2011-01-07"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, new DateTime("2011-01-08"), "a", 50, 4988, "b", 50, 4987, "c", 50, 4986, new DateTime("2011-01-09"), "a", 50, 4985, "b", 50, 4984, "c", 50, 4983 ) ), Sequences.simple( - makeTopNResults( + makeTopNResultsWithoutRename( new DateTime("2011-01-06T01"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, new DateTime("2011-01-07T01"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, new DateTime("2011-01-08T01"), "a", 50, 4988, "b", 50, 4987, "c", 50, 4986, @@ -902,7 +902,7 @@ public void testOutOfOrderSequenceMerging() throws Exception ); TestHelper.assertExpectedResults( - makeTopNResults( + makeTopNResultsWithoutRename( new DateTime("2011-01-06T01"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, new DateTime("2011-01-07"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, new DateTime("2011-01-07T01"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, @@ -952,13 +952,13 @@ client, new TopNQueryQueryToolChest( runner, builder.build(), new Interval("2011-01-01/2011-01-02"), - makeTopNResults(), + makeTopNResultsWithoutRename(), new Interval("2011-01-02/2011-01-03"), - makeTopNResults(), + makeTopNResultsWithoutRename(), new Interval("2011-01-05/2011-01-10"), - makeTopNResults( + makeTopNResultsWithoutRename( new DateTime("2011-01-05"), "a", 50, 4994, "b", 50, 4993, "c", 50, 4992, new DateTime("2011-01-06"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, new DateTime("2011-01-07"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, @@ -967,7 +967,7 @@ client, new TopNQueryQueryToolChest( ), new Interval("2011-01-05/2011-01-10"), - makeTopNResults( + makeTopNResultsWithoutRename( new DateTime("2011-01-05T01"), "a", 50, 4994, "b", 50, 4993, "c", 50, 4992, new DateTime("2011-01-06T01"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, new DateTime("2011-01-07T01"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, @@ -994,7 +994,7 @@ client, new TopNQueryQueryToolChest( builder.intervals("2011-01-01/2011-01-10") .metric("imps") .aggregators(RENAMED_AGGS) - .postAggregators(RENAMED_POST_AGGS) + .postAggregators(POST_AGGS) .build(), context ) @@ -1026,13 +1026,13 @@ client, new TopNQueryQueryToolChest( runner, builder.build(), new Interval("2011-01-01/2011-01-02"), - makeTopNResults(), + makeTopNResultsWithoutRename(), new Interval("2011-01-02/2011-01-03"), - makeTopNResults(), + makeTopNResultsWithoutRename(), new Interval("2011-01-05/2011-01-10"), - makeTopNResults( + makeTopNResultsWithoutRename( new DateTime("2011-01-05"), "a", 50, 4994, "b", 50, 4993, "c", 50, 4992, new DateTime("2011-01-06"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, new DateTime("2011-01-07"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, @@ -1041,7 +1041,7 @@ client, new TopNQueryQueryToolChest( ), new Interval("2011-01-05/2011-01-10"), - makeTopNResults( + makeTopNResultsWithoutRename( new DateTime("2011-01-05T01"), "a", 50, 4994, "b", 50, 4993, "c", 50, 4992, new DateTime("2011-01-06T01"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, new DateTime("2011-01-07T01"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, @@ -1052,7 +1052,7 @@ client, new TopNQueryQueryToolChest( HashMap context = new HashMap(); TestHelper.assertExpectedResults( - makeTopNResults( + makeTopNResultsWithoutRename( new DateTime("2011-01-05"), "a", 50, 4994, "b", 50, 4993, "c", 50, 4992, new DateTime("2011-01-05T01"), "a", 50, 4994, "b", 50, 4993, "c", 50, 4992, new DateTime("2011-01-06"), "a", 50, 4991, "b", 50, 4990, "c", 50, 4989, @@ -2527,7 +2527,7 @@ public Result apply( (DateTime) objects[i], new TimeseriesResultValue( ImmutableMap.of( - "rows2", objects[i + 1], + "rows", objects[i + 1], "imps", objects[i + 2], "impers2", objects[i + 2] ) @@ -2538,9 +2538,27 @@ public Result apply( return retVal; } - private Iterable> makeTopNResults + private Iterable> makeTopNResultsWithoutRename (Object... objects) { + return makeTopNResults( + Lists.newArrayList( + TOP_DIM, + "rows", + "imps", + "impers", + "avg_imps_per_row", + "avg_imps_per_row_double", + "avg_imps_per_row_half" + ), + objects + ); + } + + private Iterable> makeTopNResults + (List names, Object... objects) + { + Preconditions.checkArgument(names.size() == 7); List> retVal = Lists.newArrayList(); int index = 0; while (index < objects.length) { @@ -2557,14 +2575,14 @@ public Result apply( final double rows = ((Number) objects[index + 1]).doubleValue(); values.add( ImmutableMap.builder() - .put(TOP_DIM, objects[index]) - .put("rows", rows) - .put("imps", imps) - .put("impers", imps) - .put("avg_imps_per_row", imps / rows) - .put("avg_imps_per_row_double", ((imps * 2) / rows)) - .put("avg_imps_per_row_half", (imps / (rows * 2))) - .build() + .put(names.get(0), objects[index]) + .put(names.get(1), rows) + .put(names.get(2), imps) + .put(names.get(3), imps) + .put(names.get(4), imps / rows) + .put(names.get(5), ((imps * 2) / rows)) + .put(names.get(6), (imps / (rows * 2))) + .build() ); index += 3; } @@ -2577,34 +2595,18 @@ public Result apply( private Iterable> makeRenamedTopNResults (Object... objects) { - List> retVal = Lists.newArrayList(); - int index = 0; - while (index < objects.length) { - DateTime timestamp = (DateTime) objects[index++]; - - List> values = Lists.newArrayList(); - while (index < objects.length && !(objects[index] instanceof DateTime)) { - if (objects.length - index < 3) { - throw new ISE( - "expect 3 values for each entry in the top list, had %d values left.", objects.length - index - ); - } - final double imps = ((Number) objects[index + 2]).doubleValue(); - final double rows = ((Number) objects[index + 1]).doubleValue(); - values.add( - ImmutableMap.of( - TOP_DIM, objects[index], - "rows2", rows, - "imps", imps, - "impers2", imps - ) - ); - index += 3; - } - - retVal.add(new Result<>(timestamp, new TopNResultValue(values))); - } - return retVal; + return makeTopNResults( + Lists.newArrayList( + TOP_DIM, + "rows", + "imps", + "impers2", + "avg_imps_per_row", + "avg_imps_per_row_double", + "avg_imps_per_row_half" + ), + objects + ); } private Iterable> makeSearchResults @@ -3087,16 +3089,16 @@ public void testGroupByCachingRenamedAggs() throws Exception TestHelper.assertExpectedObjects( makeGroupByResults( - new DateTime("2011-01-05T"), ImmutableMap.of("output2", "c", "rows2", 3, "imps", 3, "impers2", 3), - new DateTime("2011-01-05T01"), ImmutableMap.of("output2", "c", "rows2", 3, "imps", 3, "impers2", 3), - new DateTime("2011-01-06T"), ImmutableMap.of("output2", "d", "rows2", 4, "imps", 4, "impers2", 4), - new DateTime("2011-01-06T01"), ImmutableMap.of("output2", "d", "rows2", 4, "imps", 4, "impers2", 4), - new DateTime("2011-01-07T"), ImmutableMap.of("output2", "e", "rows2", 5, "imps", 5, "impers2", 5), - new DateTime("2011-01-07T01"), ImmutableMap.of("output2", "e", "rows2", 5, "imps", 5, "impers2", 5), - new DateTime("2011-01-08T"), ImmutableMap.of("output2", "f", "rows2", 6, "imps", 6, "impers2", 6), - new DateTime("2011-01-08T01"), ImmutableMap.of("output2", "f", "rows2", 6, "imps", 6, "impers2", 6), - new DateTime("2011-01-09T"), ImmutableMap.of("output2", "g", "rows2", 7, "imps", 7, "impers2", 7), - new DateTime("2011-01-09T01"), ImmutableMap.of("output2", "g", "rows2", 7, "imps", 7, "impers2", 7) + new DateTime("2011-01-05T"), ImmutableMap.of("output2", "c", "rows", 3, "imps", 3, "impers2", 3), + new DateTime("2011-01-05T01"), ImmutableMap.of("output2", "c", "rows", 3, "imps", 3, "impers2", 3), + new DateTime("2011-01-06T"), ImmutableMap.of("output2", "d", "rows", 4, "imps", 4, "impers2", 4), + new DateTime("2011-01-06T01"), ImmutableMap.of("output2", "d", "rows", 4, "imps", 4, "impers2", 4), + new DateTime("2011-01-07T"), ImmutableMap.of("output2", "e", "rows", 5, "imps", 5, "impers2", 5), + new DateTime("2011-01-07T01"), ImmutableMap.of("output2", "e", "rows", 5, "imps", 5, "impers2", 5), + new DateTime("2011-01-08T"), ImmutableMap.of("output2", "f", "rows", 6, "imps", 6, "impers2", 6), + new DateTime("2011-01-08T01"), ImmutableMap.of("output2", "f", "rows", 6, "imps", 6, "impers2", 6), + new DateTime("2011-01-09T"), ImmutableMap.of("output2", "g", "rows", 7, "imps", 7, "impers2", 7), + new DateTime("2011-01-09T01"), ImmutableMap.of("output2", "g", "rows", 7, "imps", 7, "impers2", 7) ), runner.run( builder.setInterval("2011-01-05/2011-01-10") From 44e21ce3ca8e576f7824c16b370d259e0e66c97d Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Thu, 2 Feb 2017 14:57:35 +0900 Subject: [PATCH 02/10] Add tests for strings --- .../query/cache/CacheKeyBuilderTest.java | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java b/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java index 21bbbf4d6d0e..650cba39f146 100644 --- a/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java +++ b/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java @@ -35,6 +35,7 @@ import java.util.List; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @RunWith(Parameterized.class) @@ -46,7 +47,8 @@ public static List params() return ImmutableList.of( new Object[]{CacheKeyBuilder.DEFAULT_SEPARATOR}, new Object[]{CacheKeyBuilder.EMPTY_BYTES}, - new Object[]{new byte[] {'\001'}} + new Object[]{new byte[] {'\001'}}, + new Object[]{StringUtils.toUtf8("string")} ); } @@ -118,4 +120,46 @@ public byte[] getCacheKey() assertTrue(Arrays.equals(expected, actual)); } + + @Test + public void testStrings() + { + final byte[] actual = new CacheKeyBuilder((byte) 10, separator) + .appendString("test") + .appendString("test") + .appendStringList(Lists.newArrayList("test", "test")) + .build(); + + final byte[] expected = ByteBuffer.allocate(actual.length) + .put((byte) 10) + .put(StringUtils.toUtf8("test")) + .put(separator) + .put(StringUtils.toUtf8("test")) + .put(separator) + .put(StringUtils.toUtf8("test")) + .put(separator) + .put(StringUtils.toUtf8("test")) + .array(); + + assertTrue(Arrays.equals(expected, actual)); + } + + @Test + public void testNotEqualStrings() + { + final byte[] key1 = new CacheKeyBuilder((byte) 10, separator) + .appendString("test") + .appendString("test") + .build(); + + final byte[] key2 = new CacheKeyBuilder((byte) 10, separator) + .appendString("testtest") + .build(); + + if (Arrays.equals(separator, CacheKeyBuilder.EMPTY_BYTES)) { + assertTrue(Arrays.equals(key1, key2)); + } else { + assertFalse(Arrays.equals(key1, key2)); + } + } } From ddb4eee8859b1bd5d433b2df92e742e2a2ee9dc0 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Fri, 3 Feb 2017 08:30:57 +0900 Subject: [PATCH 03/10] Remove debug comments --- .../java/io/druid/query/topn/TopNQueryQueryToolChest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java index 51ca39eb2941..d0ceb98df049 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java @@ -301,7 +301,7 @@ public CacheStrategy, Object, TopNQuery> getCacheStrateg ); @Override - public byte[] computeCacheKey(TopNQuery query) // always called + public byte[] computeCacheKey(TopNQuery query) { return new CacheKeyBuilder(TOPN_QUERY, CacheKeyBuilder.EMPTY_BYTES) .appendCacheable(query.getDimensionSpec()) @@ -321,7 +321,7 @@ public TypeReference getCacheObjectClazz() } @Override - public Function, Object> prepareForCache() // prepare for cache population. called on cache misses + public Function, Object> prepareForCache() { return new Function, Object>() { @@ -349,7 +349,7 @@ public Object apply(final Result input) } @Override - public Function> pullFromCache() // called on cache hits + public Function> pullFromCache() { return new Function>() { From f30c724177adac145f1d22184de6ae4f4cc12337 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Tue, 7 Feb 2017 21:41:03 +0900 Subject: [PATCH 04/10] Add type keys and list sizes to cache key --- .../io/druid/common/utils/StringUtils.java | 2 +- .../theta/SketchEstimatePostAggregator.java | 2 +- .../theta/SketchSetPostAggregator.java | 2 +- .../histogram/BucketsPostAggregator.java | 2 +- .../CustomBucketsPostAggregator.java | 2 +- .../histogram/EqualBucketsPostAggregator.java | 2 +- .../histogram/MaxPostAggregator.java | 2 +- .../histogram/MinPostAggregator.java | 2 +- .../histogram/QuantilePostAggregator.java | 2 +- .../histogram/QuantilesPostAggregator.java | 2 +- .../StandardDeviationPostAggregator.java | 2 +- .../HyperUniqueFinalizingPostAggregator.java | 2 +- .../aggregation/post/PostAggregatorIds.java | 38 +-- .../io/druid/query/cache/CacheKeyBuilder.java | 161 +++++++---- .../groupby/GroupByQueryQueryToolChest.java | 4 +- .../query/groupby/having/HavingSpec.java | 13 +- .../query/groupby/orderby/LimitSpec.java | 9 +- .../TimeseriesQueryQueryToolChest.java | 2 +- .../query/topn/TopNQueryQueryToolChest.java | 2 +- .../query/cache/CacheKeyBuilderTest.java | 269 +++++++++++++----- 20 files changed, 356 insertions(+), 166 deletions(-) diff --git a/common/src/main/java/io/druid/common/utils/StringUtils.java b/common/src/main/java/io/druid/common/utils/StringUtils.java index 4ea9c7a5729e..3c1971f7866c 100644 --- a/common/src/main/java/io/druid/common/utils/StringUtils.java +++ b/common/src/main/java/io/druid/common/utils/StringUtils.java @@ -23,7 +23,7 @@ */ public class StringUtils extends io.druid.java.util.common.StringUtils { - private static final byte[] EMPTY_BYTES = new byte[0]; + public static final byte[] EMPTY_BYTES = new byte[0]; // should be used only for estimation // returns the same result with StringUtils.fromUtf8(value).length for valid string values diff --git a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchEstimatePostAggregator.java b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchEstimatePostAggregator.java index 215acc4f8db6..e5b2415ad708 100644 --- a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchEstimatePostAggregator.java +++ b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchEstimatePostAggregator.java @@ -153,7 +153,7 @@ public int hashCode() @Override public byte[] getCacheKey() { - final CacheKeyBuilder builder = new CacheKeyBuilder(PostAggregatorIds.SKETCH_ESTIMATE) + final CacheKeyBuilder builder = new CacheKeyBuilder(PostAggregatorIds.DATA_SKETCHES_SKETCH_ESTIMATE) .appendCacheable(field); return errorBoundsStdDev == null ? builder.build() : builder.appendInt(errorBoundsStdDev).build(); } diff --git a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java index 5120ed91dd8c..df8c1fa2cf32 100644 --- a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java +++ b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java @@ -165,7 +165,7 @@ public int hashCode() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.SKETCH_SET) + return new CacheKeyBuilder(PostAggregatorIds.DATA_SKETCHES_SKETCH_SET) .appendString(getFunc()) .appendCacheableList(fields) .appendInt(maxSketchSize) diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/BucketsPostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/BucketsPostAggregator.java index 86f54eb78afe..8c53146d8ad8 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/BucketsPostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/BucketsPostAggregator.java @@ -91,7 +91,7 @@ public String toString() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.BUCKETS) + return new CacheKeyBuilder(PostAggregatorIds.HISTOGRAM_BUCKETS) .appendString(fieldName) .appendFloat(bucketSize) .appendFloat(offset) diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/CustomBucketsPostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/CustomBucketsPostAggregator.java index ee47f22d9138..3433a73f6ee2 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/CustomBucketsPostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/CustomBucketsPostAggregator.java @@ -80,7 +80,7 @@ public String toString() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.CUSTOM_BUCKETS) + return new CacheKeyBuilder(PostAggregatorIds.HISTOGRAM_CUSTOM_BUCKETS) .appendString(fieldName) .appendFloatArray(breaks) .build(); diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/EqualBucketsPostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/EqualBucketsPostAggregator.java index 5156aa97bc0e..f4e9a2b9dc64 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/EqualBucketsPostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/EqualBucketsPostAggregator.java @@ -81,7 +81,7 @@ public String toString() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.EQUAL_BUCKETS) + return new CacheKeyBuilder(PostAggregatorIds.HISTOGRAM_EQUAL_BUCKETS) .appendString(fieldName) .appendInt(numBuckets) .build(); diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MaxPostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MaxPostAggregator.java index 86aa0f6843c2..7d69331b8b8b 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MaxPostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MaxPostAggregator.java @@ -81,7 +81,7 @@ public String toString() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.MAX) + return new CacheKeyBuilder(PostAggregatorIds.HISTOGRAM_MAX) .appendString(fieldName) .build(); } diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MinPostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MinPostAggregator.java index 3badc55b5792..90afc6803ccd 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MinPostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/MinPostAggregator.java @@ -81,7 +81,7 @@ public String toString() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.MIN) + return new CacheKeyBuilder(PostAggregatorIds.HISTOGRAM_MIN) .appendString(fieldName) .build(); } diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilePostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilePostAggregator.java index 09ef7abbc2df..d375308b867f 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilePostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilePostAggregator.java @@ -125,7 +125,7 @@ public String toString() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.QUANTILE) + return new CacheKeyBuilder(PostAggregatorIds.HISTOGRAM_QUANTILE) .appendString(fieldName) .appendFloat(probability) .build(); diff --git a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilesPostAggregator.java b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilesPostAggregator.java index b7ad2e84b836..9411361734b7 100644 --- a/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilesPostAggregator.java +++ b/extensions-core/histogram/src/main/java/io/druid/query/aggregation/histogram/QuantilesPostAggregator.java @@ -93,7 +93,7 @@ public String toString() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.QUANTILES) + return new CacheKeyBuilder(PostAggregatorIds.HISTOGRAM_QUANTILES) .appendString(fieldName) .appendFloatArray(probabilities) .build(); diff --git a/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/StandardDeviationPostAggregator.java b/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/StandardDeviationPostAggregator.java index 400fef370388..ca4e35b379fd 100644 --- a/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/StandardDeviationPostAggregator.java +++ b/extensions-core/stats/src/main/java/io/druid/query/aggregation/variance/StandardDeviationPostAggregator.java @@ -107,7 +107,7 @@ public String toString() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.STANDARD_DEVIATION) + return new CacheKeyBuilder(PostAggregatorIds.VARIANCE_STANDARD_DEVIATION) .appendString(fieldName) .appendBoolean(isVariancePop) .build(); diff --git a/processing/src/main/java/io/druid/query/aggregation/hyperloglog/HyperUniqueFinalizingPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/hyperloglog/HyperUniqueFinalizingPostAggregator.java index c197cfb925f3..5b78b50007d4 100644 --- a/processing/src/main/java/io/druid/query/aggregation/hyperloglog/HyperUniqueFinalizingPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/hyperloglog/HyperUniqueFinalizingPostAggregator.java @@ -130,7 +130,7 @@ public String toString() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.HYPER_UNIQUE_FINALIZING) + return new CacheKeyBuilder(PostAggregatorIds.HLL_HYPER_UNIQUE_FINALIZING) .appendString(fieldName) .build(); } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/PostAggregatorIds.java b/processing/src/main/java/io/druid/query/aggregation/post/PostAggregatorIds.java index 4da712891489..dcfbc4fbf3c4 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/PostAggregatorIds.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/PostAggregatorIds.java @@ -22,23 +22,23 @@ public class PostAggregatorIds { public static final byte ARITHMETIC = 0; - public static final byte BUCKETS = 1; - public static final byte CONSTANT = 2; - public static final byte CUSTOM_BUCKETS = 3; - public static final byte DOUBLE_GREATEST = 4; - public static final byte DOUBLE_LEAST = 5; - public static final byte EQUAL_BUCKETS = 6; - public static final byte EXPRESSION = 7; - public static final byte FIELD_ACCESS = 8; - public static final byte HYPER_UNIQUE_FINALIZING = 9; - public static final byte JAVA_SCRIPT = 10; - public static final byte LONG_GREATEST = 11; - public static final byte LONG_LEAST = 12; - public static final byte MAX = 13; - public static final byte MIN = 14; - public static final byte QUANTILE = 15; - public static final byte QUANTILES = 16; - public static final byte SKETCH_ESTIMATE = 17; - public static final byte SKETCH_SET = 18; - public static final byte STANDARD_DEVIATION = 19; + public static final byte CONSTANT = 1; + public static final byte DOUBLE_GREATEST = 2; + public static final byte DOUBLE_LEAST = 3; + public static final byte EXPRESSION = 4; + public static final byte FIELD_ACCESS = 5; + public static final byte JAVA_SCRIPT = 6; + public static final byte LONG_GREATEST = 7; + public static final byte LONG_LEAST = 8; + public static final byte HLL_HYPER_UNIQUE_FINALIZING = 9; + public static final byte HISTOGRAM_BUCKETS = 10; + public static final byte HISTOGRAM_CUSTOM_BUCKETS = 11; + public static final byte HISTOGRAM_EQUAL_BUCKETS = 12; + public static final byte HISTOGRAM_MAX = 13; + public static final byte HISTOGRAM_MIN = 14; + public static final byte HISTOGRAM_QUANTILE = 15; + public static final byte HISTOGRAM_QUANTILES = 16; + public static final byte DATA_SKETCHES_SKETCH_ESTIMATE = 17; + public static final byte DATA_SKETCHES_SKETCH_SET = 18; + public static final byte VARIANCE_STANDARD_DEVIATION = 19; } diff --git a/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java b/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java index 48b253f9b8d7..db4eb4663cfa 100644 --- a/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java +++ b/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java @@ -19,6 +19,7 @@ package io.druid.query.cache; +import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.common.primitives.Doubles; import com.google.common.primitives.Floats; @@ -27,36 +28,76 @@ import javax.annotation.Nullable; import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.Collection; +import java.util.Iterator; import java.util.List; public class CacheKeyBuilder { - static final byte[] DEFAULT_SEPARATOR = new byte[]{(byte) 0xFF}; - public static final byte[] EMPTY_BYTES = new byte[0]; + static final byte BYTE_KEY = 0; + static final byte BYTE_ARRAY_KEY = 1; + static final byte BOOLEAN_KEY = 2; + static final byte INT_KEY = 3; + static final byte FLOAT_KEY = 4; + static final byte FLOAT_ARRAY_KEY = 5; + static final byte DOUBLE_KEY = 6; + static final byte STRING_KEY = 7; + static final byte STRING_LIST_KEY = 8; + static final byte CACHEABLE_KEY = 9; + static final byte CACHEABLE_LIST_KEY = 10; + + static final byte[] STRING_SEPARATOR = new byte[]{(byte) 0xFF}; + public static final byte[] EMPTY_BYTES = StringUtils.EMPTY_BYTES; + + private static class Item + { + private final byte typeKey; + private final byte[] item; + + Item(byte typeKey, byte[] item) + { + this.typeKey = typeKey; + this.item = item; + } - private static byte[] byteToByteArray(byte input) - { - return new byte[]{input}; + public int byteSize() + { + return 1 + item.length; + } } - private static byte[] stringToByteArray(String input) + private static byte[] stringListToByteArray(List input) { - return StringUtils.toUtf8WithNullToEmpty(input); + if (input.size() > 0) { + List byteArrayList = Lists.newArrayListWithCapacity(input.size()); + int totalByteLength = 0; + for (String eachStr : input) { + final byte[] byteArray = StringUtils.toUtf8WithNullToEmpty(eachStr); + totalByteLength += byteArray.length; + byteArrayList.add(byteArray); + } + return joinByteArrayList(byteArrayList, STRING_SEPARATOR, totalByteLength); + } else { + return EMPTY_BYTES; + } } - private static byte[] booleanToByteArray(boolean input) + private static byte[] joinByteArrayList(Collection byteArrayList, byte[] separator, int totalByteLength) { - return new byte[]{(byte) (input ? 1 : 0)}; - } + final Iterator iterator = byteArrayList.iterator(); + Preconditions.checkArgument(iterator.hasNext()); - private static byte[] intToByteArray(int input) - { - return Ints.toByteArray(input); - } + final int bufSize = Ints.BYTES + separator.length * (byteArrayList.size() - 1) + totalByteLength; + final ByteBuffer buffer = ByteBuffer.allocate(bufSize) + .putInt(byteArrayList.size()) + .put(iterator.next()); - private static byte[] floatToByteArray(float input) - { - return ByteBuffer.allocate(Floats.BYTES).putFloat(input).array(); + while (iterator.hasNext()) { + buffer.put(separator).put(iterator.next()); + } + + return buffer.array(); } private static byte[] floatArrayToByteArray(float[] input) @@ -66,110 +107,128 @@ private static byte[] floatArrayToByteArray(float[] input) return buffer.array(); } - private static byte[] doubleToByteArray(double input) + private static byte[] cacheableToByteArray(@Nullable Cacheable cacheable) + { + if (cacheable == null) { + return EMPTY_BYTES; + } else { + final byte[] key = cacheable.getCacheKey(); + Preconditions.checkArgument(!Arrays.equals(key, EMPTY_BYTES), "cache key is equal to the empty key"); + return key; + } + } + + private static byte[] cacheableListToByteArray(List input) { - return ByteBuffer.allocate(Doubles.BYTES).putDouble(input).array(); + final int inputSize = input.size(); + if (inputSize > 0) { + final List byteArrayList = Lists.newArrayListWithCapacity(inputSize); + int totalByteLength = 0; + for (Cacheable eachCacheable : input) { + byte[] key = cacheableToByteArray(eachCacheable); + totalByteLength += key.length; + byteArrayList.add(key); + } + + return joinByteArrayList(byteArrayList, EMPTY_BYTES, totalByteLength); + } else { + return EMPTY_BYTES; + } } - private final List items = Lists.newArrayList(); + private final List items = Lists.newArrayList(); private final byte id; - private final byte[] separator; private int size; public CacheKeyBuilder(byte id) - { - this(id, DEFAULT_SEPARATOR); - } - - public CacheKeyBuilder(byte id, byte[] separator) { this.id = id; this.size = 1; - this.separator = separator; } public CacheKeyBuilder appendByte(byte input) { - appendItem(byteToByteArray(input)); + appendItem(BYTE_KEY, new byte[]{input}); + return this; + } + + public CacheKeyBuilder appendByteArray(byte[] input) + { + // TODO: check it is ok to not add input.length for fixed-size types + appendItem(BYTE_ARRAY_KEY, input); return this; } public CacheKeyBuilder appendString(@Nullable String input) { - appendItem(stringToByteArray(input)); + appendItem(STRING_KEY, StringUtils.toUtf8WithNullToEmpty(input)); return this; } public CacheKeyBuilder appendStringList(List input) { - for (String eachStr : input) { - appendItem(stringToByteArray(eachStr)); - } + appendItem(STRING_LIST_KEY, stringListToByteArray(input)); return this; } public CacheKeyBuilder appendBoolean(boolean input) { - appendItem(booleanToByteArray(input)); + appendItem(BOOLEAN_KEY, new byte[]{(byte) (input ? 1 : 0)}); return this; } public CacheKeyBuilder appendInt(int input) { - appendItem(intToByteArray(input)); + appendItem(INT_KEY, Ints.toByteArray(input)); return this; } public CacheKeyBuilder appendFloat(float input) { - appendItem(floatToByteArray(input)); + appendItem(FLOAT_KEY, ByteBuffer.allocate(Floats.BYTES).putFloat(input).array()); return this; } public CacheKeyBuilder appendDouble(double input) { - appendItem(doubleToByteArray(input)); + appendItem(DOUBLE_KEY, ByteBuffer.allocate(Doubles.BYTES).putDouble(input).array()); return this; } public CacheKeyBuilder appendFloatArray(float[] input) { - appendItem(floatArrayToByteArray(input)); + appendItem(FLOAT_ARRAY_KEY, floatArrayToByteArray(input)); return this; } public CacheKeyBuilder appendCacheable(@Nullable Cacheable input) { - appendItem(input == null ? EMPTY_BYTES : input.getCacheKey()); + appendItem(CACHEABLE_KEY, cacheableToByteArray(input)); return this; } - public CacheKeyBuilder appendCacheableList(List inputs) + public CacheKeyBuilder appendCacheableList(List input) { - for (Cacheable input : inputs) { - appendItem(input.getCacheKey()); - } + appendItem(CACHEABLE_LIST_KEY, cacheableListToByteArray(input)); return this; } - private void appendItem(byte[] item) + private void appendItem(byte typeKey, byte[] input) { + final Item item = new Item(typeKey, input); items.add(item); - size += item.length; + size += item.byteSize(); } public byte[] build() { - final ByteBuffer buffer = ByteBuffer.allocate(size + separator.length * (items.size() - 1)); + final ByteBuffer buffer = ByteBuffer.allocate(size); buffer.put(id); - for (int i = 0; i < items.size(); i++) { - if (i == items.size() - 1) { - buffer.put(items.get(i)); - } else { - buffer.put(items.get(i)).put(separator); - } + for (Item item : items) { + buffer.put(item.typeKey).put(item.item); } + return buffer.array(); } } diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java index 8b196b037d3b..526a66a47e1c 100644 --- a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java @@ -355,12 +355,14 @@ public CacheStrategy getCacheStrategy(final GroupByQu @Override public byte[] computeCacheKey(GroupByQuery query) { - return new CacheKeyBuilder(GROUPBY_QUERY, CacheKeyBuilder.EMPTY_BYTES) + return new CacheKeyBuilder(GROUPBY_QUERY) .appendByte(CACHE_STRATEGY_VERSION) .appendCacheable(query.getGranularity()) .appendCacheable(query.getDimFilter()) .appendCacheableList(query.getAggregatorSpecs()) .appendCacheableList(query.getDimensions()) + .appendCacheable(query.getHavingSpec()) + .appendCacheable(query.getLimitSpec()) .build(); } diff --git a/processing/src/main/java/io/druid/query/groupby/having/HavingSpec.java b/processing/src/main/java/io/druid/query/groupby/having/HavingSpec.java index 8e20cbb08ef4..aa112bd479e7 100644 --- a/processing/src/main/java/io/druid/query/groupby/having/HavingSpec.java +++ b/processing/src/main/java/io/druid/query/groupby/having/HavingSpec.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; import io.druid.data.input.Row; +import io.druid.query.cache.Cacheable; import io.druid.segment.column.ValueType; import java.util.Map; @@ -43,12 +44,12 @@ @JsonSubTypes.Type(name = "always", value = AlwaysHavingSpec.class), @JsonSubTypes.Type(name = "filter", value = DimFilterHavingSpec.class) }) -public interface HavingSpec +public interface HavingSpec extends Cacheable { // Atoms for easy combination, but for now they are mostly useful // for testing. - public static final HavingSpec NEVER = new NeverHavingSpec(); - public static final HavingSpec ALWAYS = new AlwaysHavingSpec(); + HavingSpec NEVER = new NeverHavingSpec(); + HavingSpec ALWAYS = new AlwaysHavingSpec(); /** * Informs this HavingSpec that rows passed to "eval" will have a certain signature. Will be called @@ -56,7 +57,7 @@ public interface HavingSpec * * @param rowSignature signature of the rows */ - public void setRowSignature(Map rowSignature); + void setRowSignature(Map rowSignature); /** * Evaluates if a given row satisfies the having spec. @@ -65,7 +66,5 @@ public interface HavingSpec * * @return true if the given row satisfies the having spec. False otherwise. */ - public boolean eval(Row row); - - public byte[] getCacheKey(); + boolean eval(Row row); } diff --git a/processing/src/main/java/io/druid/query/groupby/orderby/LimitSpec.java b/processing/src/main/java/io/druid/query/groupby/orderby/LimitSpec.java index 7d88cf8687ea..fd5d202f9980 100644 --- a/processing/src/main/java/io/druid/query/groupby/orderby/LimitSpec.java +++ b/processing/src/main/java/io/druid/query/groupby/orderby/LimitSpec.java @@ -26,6 +26,7 @@ import io.druid.java.util.common.guava.Sequence; import io.druid.query.aggregation.AggregatorFactory; import io.druid.query.aggregation.PostAggregator; +import io.druid.query.cache.Cacheable; import io.druid.query.dimension.DimensionSpec; import java.util.List; @@ -36,7 +37,7 @@ @JsonSubTypes(value = { @JsonSubTypes.Type(name = "default", value = DefaultLimitSpec.class) }) -public interface LimitSpec +public interface LimitSpec extends Cacheable { /** * Returns a function that applies a limit to an input sequence that is assumed to be sorted on dimensions. @@ -47,13 +48,11 @@ public interface LimitSpec * * @return limit function */ - public Function, Sequence> build( + Function, Sequence> build( List dimensions, List aggs, List postAggs ); - public LimitSpec merge(LimitSpec other); - - public byte[] getCacheKey(); + LimitSpec merge(LimitSpec other); } diff --git a/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java index e0ab7931e240..f5c2167cb441 100644 --- a/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java @@ -130,7 +130,7 @@ public CacheStrategy, Object, TimeseriesQuery> get @Override public byte[] computeCacheKey(TimeseriesQuery query) { - return new CacheKeyBuilder(TIMESERIES_QUERY, CacheKeyBuilder.EMPTY_BYTES) + return new CacheKeyBuilder(TIMESERIES_QUERY) .appendBoolean(query.isDescending()) .appendBoolean(query.isSkipEmptyBuckets()) .appendCacheable(query.getGranularity()) diff --git a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java index d0ceb98df049..2122a2ac6846 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java @@ -303,7 +303,7 @@ public CacheStrategy, Object, TopNQuery> getCacheStrateg @Override public byte[] computeCacheKey(TopNQuery query) { - return new CacheKeyBuilder(TOPN_QUERY, CacheKeyBuilder.EMPTY_BYTES) + return new CacheKeyBuilder(TOPN_QUERY) .appendCacheable(query.getDimensionSpec()) .appendCacheable(query.getTopNMetricSpec()) .appendInt(query.getThreshold()) diff --git a/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java b/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java index 650cba39f146..1fd946ee4f8e 100644 --- a/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java +++ b/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java @@ -26,9 +26,6 @@ import com.google.common.primitives.Ints; import io.druid.common.utils.StringUtils; import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameters; import java.nio.ByteBuffer; import java.util.Arrays; @@ -38,27 +35,8 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -@RunWith(Parameterized.class) public class CacheKeyBuilderTest { - @Parameters - public static List params() - { - return ImmutableList.of( - new Object[]{CacheKeyBuilder.DEFAULT_SEPARATOR}, - new Object[]{CacheKeyBuilder.EMPTY_BYTES}, - new Object[]{new byte[] {'\001'}}, - new Object[]{StringUtils.toUtf8("string")} - ); - } - - private final byte[] separator; - - public CacheKeyBuilderTest(byte[] separator) - { - this.separator = separator; - } - @Test public void testCacheKeyBuilder() { @@ -71,95 +49,248 @@ public byte[] getCacheKey() } }; - final byte[] actual = new CacheKeyBuilder((byte) 10, separator) + final byte[] actual = new CacheKeyBuilder((byte) 10) .appendBoolean(false) .appendString("test") .appendInt(10) .appendFloat(0.1f) .appendDouble(2.3) + .appendByteArray(CacheKeyBuilder.STRING_SEPARATOR) // test when an item is same with the separator .appendFloatArray(new float[]{10.0f, 11.0f}) .appendStringList(Lists.newArrayList("test1", "test2")) .appendCacheable(cacheable) .appendCacheable(null) + .appendCacheableList(Lists.newArrayList((Cacheable) null)) .build(); - final int expectedSize = 1 // id - + 1 // bool - + 4 // 'test' - + Ints.BYTES // 10 - + Floats.BYTES // 0.1f - + Doubles.BYTES // 2.3 - + Floats.BYTES * 2 // 10.0f, 11.0f - + 5 * 2 // 'test1' 'test2' - + cacheable.getCacheKey().length // cacheable - + 9 * separator.length; // separators + final int expectedSize = 1 // id + + 1 // bool + + 4 // 'test' + + Ints.BYTES // 10 + + Floats.BYTES // 0.1f + + Doubles.BYTES // 2.3 + + CacheKeyBuilder.STRING_SEPARATOR.length // byte array + + Floats.BYTES * 2 // 10.0f, 11.0f + + Ints.BYTES + 5 * 2 + 1 // 'test1' 'test2' + + cacheable.getCacheKey().length // cacheable + + Ints.BYTES // cacheable list + + 11; // type keys assertEquals(expectedSize, actual.length); final byte[] expected = ByteBuffer.allocate(expectedSize) .put((byte) 10) + .put(CacheKeyBuilder.BOOLEAN_KEY) .put((byte) 0) - .put(separator) + .put(CacheKeyBuilder.STRING_KEY) .put(StringUtils.toUtf8("test")) - .put(separator) + .put(CacheKeyBuilder.INT_KEY) .putInt(10) - .put(separator) + .put(CacheKeyBuilder.FLOAT_KEY) .putFloat(0.1f) - .put(separator) + .put(CacheKeyBuilder.DOUBLE_KEY) .putDouble(2.3) - .put(separator) + .put(CacheKeyBuilder.BYTE_ARRAY_KEY) + .put(CacheKeyBuilder.STRING_SEPARATOR) + .put(CacheKeyBuilder.FLOAT_ARRAY_KEY) .putFloat(10.0f) .putFloat(11.0f) - .put(separator) + .put(CacheKeyBuilder.STRING_LIST_KEY) + .putInt(2) .put(StringUtils.toUtf8("test1")) - .put(separator) + .put(CacheKeyBuilder.STRING_SEPARATOR) .put(StringUtils.toUtf8("test2")) - .put(separator) + .put(CacheKeyBuilder.CACHEABLE_KEY) .put(cacheable.getCacheKey()) - .put(separator) + .put(CacheKeyBuilder.CACHEABLE_KEY) + .put(CacheKeyBuilder.CACHEABLE_LIST_KEY) + .putInt(1) .array(); assertTrue(Arrays.equals(expected, actual)); } @Test - public void testStrings() + public void testNotEqualStrings() { - final byte[] actual = new CacheKeyBuilder((byte) 10, separator) - .appendString("test") - .appendString("test") - .appendStringList(Lists.newArrayList("test", "test")) + final List keys = Lists.newArrayList(); + keys.add( + new CacheKeyBuilder((byte) 10) + .appendString("test") + .appendString("test") + .build() + ); + + keys.add( + new CacheKeyBuilder((byte) 10) + .appendString("testtest") + .build() + ); + + keys.add( + new CacheKeyBuilder((byte) 10) + .appendString("testtest") + .appendString("") + .build() + ); + + keys.add( + new CacheKeyBuilder((byte) 10) + .appendString("") + .appendString("testtest") + .build() + ); + + keys.add( + new CacheKeyBuilder((byte) 10) + .appendStringList(ImmutableList.of("test", "test")) + .build() + ); + + keys.add( + new CacheKeyBuilder((byte) 10) + .appendStringList(ImmutableList.of("testtest")) + .build() + ); + + keys.add( + new CacheKeyBuilder((byte) 10) + .appendStringList(ImmutableList.of("testtest", "")) + .build() + ); + + keys.add( + new CacheKeyBuilder((byte) 10) + .appendStringList(ImmutableList.of("", "testtest")) + .build() + ); + + keys.add( + new CacheKeyBuilder((byte) 10) + .appendStringList(ImmutableList.of("testtest")) + .appendStringList(ImmutableList.of()) + .build() + ); + + keys.add( + new CacheKeyBuilder((byte) 10) + .appendStringList(ImmutableList.of()) + .appendStringList(ImmutableList.of("testtest")) + .build() + ); + + assertNotEqualsEachOther(keys); + } + + @Test + public void testNotEqualCacheables() + { + final Cacheable test = new Cacheable() + { + @Override + public byte[] getCacheKey() + { + return "test".getBytes(); + } + }; + + final Cacheable testtest = new Cacheable() + { + @Override + public byte[] getCacheKey() + { + return "testtest".getBytes(); + } + }; + + final List keys = Lists.newArrayList(); + keys.add( + new CacheKeyBuilder((byte) 10) + .appendCacheable(test) + .appendCacheable(test) + .build() + ); + + keys.add( + new CacheKeyBuilder((byte) 10) + .appendCacheable(testtest) + .build() + ); + + keys.add( + new CacheKeyBuilder((byte) 10) + .appendCacheableList(Lists.newArrayList(test, test)) + .build() + ); + + keys.add( + new CacheKeyBuilder((byte) 10) + .appendCacheableList(Lists.newArrayList(testtest)) + .build() + ); + + keys.add( + new CacheKeyBuilder((byte) 10) + .appendCacheableList(Lists.newArrayList(testtest)) + .appendCacheableList(Lists.newArrayList()) + .build() + ); + + keys.add( + new CacheKeyBuilder((byte) 10) + .appendCacheableList(Lists.newArrayList()) + .appendCacheableList(Lists.newArrayList(testtest)) + .build() + ); + + assertNotEqualsEachOther(keys); + } + + private static void assertNotEqualsEachOther(List keys) + { + for (byte[] k1 : keys) { + for (byte[] k2 : keys) { + if (k1 != k2) { + assertFalse(Arrays.equals(k1, k2)); + } + } + } + } + + @Test + public void testEmptyOrNullStringLists() + { + byte[] key1 = new CacheKeyBuilder((byte) 10) + .appendStringList(Lists.newArrayList("", "")) .build(); - final byte[] expected = ByteBuffer.allocate(actual.length) - .put((byte) 10) - .put(StringUtils.toUtf8("test")) - .put(separator) - .put(StringUtils.toUtf8("test")) - .put(separator) - .put(StringUtils.toUtf8("test")) - .put(separator) - .put(StringUtils.toUtf8("test")) - .array(); + byte[] key2 = new CacheKeyBuilder((byte) 10) + .appendStringList(Lists.newArrayList("")) + .build(); - assertTrue(Arrays.equals(expected, actual)); + assertFalse(Arrays.equals(key1, key2)); + + key1 = new CacheKeyBuilder((byte) 10) + .appendStringList(Lists.newArrayList("")) + .build(); + + key2 = new CacheKeyBuilder((byte) 10) + .appendStringList(Lists.newArrayList((String) null)) + .build(); + + assertTrue(Arrays.equals(key1, key2)); } @Test - public void testNotEqualStrings() + public void testEmptyOrNullCacheables() { - final byte[] key1 = new CacheKeyBuilder((byte) 10, separator) - .appendString("test") - .appendString("test") + byte[] key1 = new CacheKeyBuilder((byte) 10) + .appendCacheableList(Lists.newArrayList()) .build(); - final byte[] key2 = new CacheKeyBuilder((byte) 10, separator) - .appendString("testtest") + byte[] key2 = new CacheKeyBuilder((byte) 10) + .appendCacheableList(Lists.newArrayList((Cacheable) null)) .build(); - if (Arrays.equals(separator, CacheKeyBuilder.EMPTY_BYTES)) { - assertTrue(Arrays.equals(key1, key2)); - } else { - assertFalse(Arrays.equals(key1, key2)); - } + assertFalse(Arrays.equals(key1, key2)); } } From f5caefcf6261e979f5fd4860fce2b0103e3ce478 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Wed, 8 Feb 2017 12:49:18 +0900 Subject: [PATCH 05/10] Make post aggregators used for sort are considered for cache key generation --- .../theta/SketchSetPostAggregator.java | 2 +- .../post/ArithmeticPostAggregator.java | 2 +- .../post/DoubleGreatestPostAggregator.java | 2 +- .../post/DoubleLeastPostAggregator.java | 2 +- .../post/JavaScriptPostAggregator.java | 2 +- .../post/LongGreatestPostAggregator.java | 2 +- .../post/LongLeastPostAggregator.java | 2 +- .../io/druid/query/cache/CacheKeyBuilder.java | 117 ++++++++++-------- .../groupby/GroupByQueryQueryToolChest.java | 6 +- .../TimeseriesQueryQueryToolChest.java | 2 +- .../query/topn/TopNQueryQueryToolChest.java | 72 ++++++++++- .../query/cache/CacheKeyBuilderTest.java | 96 +++++++++----- .../topn/TopNQueryQueryToolChestTest.java | 117 ++++++++++++++++++ 13 files changed, 328 insertions(+), 96 deletions(-) diff --git a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java index df8c1fa2cf32..c1060d7fb934 100644 --- a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java +++ b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java @@ -167,7 +167,7 @@ public byte[] getCacheKey() { return new CacheKeyBuilder(PostAggregatorIds.DATA_SKETCHES_SKETCH_SET) .appendString(getFunc()) - .appendCacheableList(fields) + .appendCacheables(fields) .appendInt(maxSketchSize) .build(); } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java index bc800c0eedae..ff3906feb5ea 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java @@ -129,7 +129,7 @@ public byte[] getCacheKey() { return new CacheKeyBuilder(PostAggregatorIds.ARITHMETIC) .appendString(fnName) - .appendCacheableList(fields) + .appendCacheables(fields) .appendString(ordering) .build(); } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/DoubleGreatestPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/DoubleGreatestPostAggregator.java index 8342dfa2cec6..6bee3ad49c81 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/DoubleGreatestPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/DoubleGreatestPostAggregator.java @@ -148,7 +148,7 @@ public int hashCode() public byte[] getCacheKey() { return new CacheKeyBuilder(PostAggregatorIds.DOUBLE_GREATEST) - .appendCacheableList(fields) + .appendCacheables(fields) .build(); } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/DoubleLeastPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/DoubleLeastPostAggregator.java index a1ac6688242f..dd7f55851b7d 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/DoubleLeastPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/DoubleLeastPostAggregator.java @@ -148,7 +148,7 @@ public int hashCode() public byte[] getCacheKey() { return new CacheKeyBuilder(PostAggregatorIds.DOUBLE_LEAST) - .appendCacheableList(fields) + .appendCacheables(fields) .build(); } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/JavaScriptPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/JavaScriptPostAggregator.java index 83040ef16878..b5f3e92968d3 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/JavaScriptPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/JavaScriptPostAggregator.java @@ -132,7 +132,7 @@ public Object compute(Map combinedAggregators) public byte[] getCacheKey() { return new CacheKeyBuilder(PostAggregatorIds.JAVA_SCRIPT) - .appendStringList(fieldNames) + .appendStrings(fieldNames) .appendString(function) .build(); } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/LongGreatestPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/LongGreatestPostAggregator.java index 0b00e69fb1a7..786d2a1d370f 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/LongGreatestPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/LongGreatestPostAggregator.java @@ -149,7 +149,7 @@ public int hashCode() public byte[] getCacheKey() { return new CacheKeyBuilder(PostAggregatorIds.LONG_GREATEST) - .appendCacheableList(fields) + .appendCacheables(fields) .build(); } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/LongLeastPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/LongLeastPostAggregator.java index 64621bf6a52b..9c4063fbb3f5 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/LongLeastPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/LongLeastPostAggregator.java @@ -149,7 +149,7 @@ public int hashCode() public byte[] getCacheKey() { return new CacheKeyBuilder(PostAggregatorIds.LONG_LEAST) - .appendCacheableList(fields) + .appendCacheables(fields) .build(); } } diff --git a/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java b/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java index db4eb4663cfa..165cdbe10f53 100644 --- a/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java +++ b/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java @@ -19,17 +19,20 @@ package io.druid.query.cache; +import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.common.primitives.Doubles; import com.google.common.primitives.Floats; import com.google.common.primitives.Ints; +import com.google.common.primitives.UnsignedBytes; import io.druid.common.utils.StringUtils; import javax.annotation.Nullable; import java.nio.ByteBuffer; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Iterator; import java.util.List; @@ -48,7 +51,7 @@ public class CacheKeyBuilder static final byte CACHEABLE_LIST_KEY = 10; static final byte[] STRING_SEPARATOR = new byte[]{(byte) 0xFF}; - public static final byte[] EMPTY_BYTES = StringUtils.EMPTY_BYTES; + static final byte[] EMPTY_BYTES = StringUtils.EMPTY_BYTES; private static class Item { @@ -61,45 +64,12 @@ private static class Item this.item = item; } - public int byteSize() + int byteSize() { return 1 + item.length; } } - private static byte[] stringListToByteArray(List input) - { - if (input.size() > 0) { - List byteArrayList = Lists.newArrayListWithCapacity(input.size()); - int totalByteLength = 0; - for (String eachStr : input) { - final byte[] byteArray = StringUtils.toUtf8WithNullToEmpty(eachStr); - totalByteLength += byteArray.length; - byteArrayList.add(byteArray); - } - return joinByteArrayList(byteArrayList, STRING_SEPARATOR, totalByteLength); - } else { - return EMPTY_BYTES; - } - } - - private static byte[] joinByteArrayList(Collection byteArrayList, byte[] separator, int totalByteLength) - { - final Iterator iterator = byteArrayList.iterator(); - Preconditions.checkArgument(iterator.hasNext()); - - final int bufSize = Ints.BYTES + separator.length * (byteArrayList.size() - 1) + totalByteLength; - final ByteBuffer buffer = ByteBuffer.allocate(bufSize) - .putInt(byteArrayList.size()) - .put(iterator.next()); - - while (iterator.hasNext()) { - buffer.put(separator).put(iterator.next()); - } - - return buffer.array(); - } - private static byte[] floatArrayToByteArray(float[] input) { final ByteBuffer buffer = ByteBuffer.allocate(Floats.BYTES * input.length); @@ -118,19 +88,67 @@ private static byte[] cacheableToByteArray(@Nullable Cacheable cacheable) } } - private static byte[] cacheableListToByteArray(List input) - { - final int inputSize = input.size(); - if (inputSize > 0) { - final List byteArrayList = Lists.newArrayListWithCapacity(inputSize); + private static byte[] stringCollectionToByteArray(Collection input) + { + return collectionToByteArray( + input, + new Function() + { + @Override + public byte[] apply(@Nullable String input) + { + return StringUtils.toUtf8WithNullToEmpty(input); + } + }, + STRING_SEPARATOR + ); + } + + private static byte[] cacheableCollectionToByteArray(Collection input) + { + return collectionToByteArray( + input, + new Function() + { + @Override + public byte[] apply(@Nullable Cacheable input) + { + return input == null ? EMPTY_BYTES : input.getCacheKey(); + } + }, + EMPTY_BYTES + ); + } + + private static byte[] collectionToByteArray( + Collection collection, + Function serializeFunction, + byte[] separator + ) + { + if (collection.size() > 0) { + List byteArrayList = Lists.newArrayListWithCapacity(collection.size()); int totalByteLength = 0; - for (Cacheable eachCacheable : input) { - byte[] key = cacheableToByteArray(eachCacheable); - totalByteLength += key.length; - byteArrayList.add(key); + for (T eachItem : collection) { + final byte[] byteArray = serializeFunction.apply(eachItem); + totalByteLength += byteArray.length; + byteArrayList.add(byteArray); + } + + // Sort the byte array list to guarantee that collections of same items but in different orders make the same result + Collections.sort(byteArrayList, UnsignedBytes.lexicographicalComparator()); + + final Iterator iterator = byteArrayList.iterator(); + final int bufSize = Ints.BYTES + separator.length * (byteArrayList.size() - 1) + totalByteLength; + final ByteBuffer buffer = ByteBuffer.allocate(bufSize) + .putInt(byteArrayList.size()) + .put(iterator.next()); + + while (iterator.hasNext()) { + buffer.put(separator).put(iterator.next()); } - return joinByteArrayList(byteArrayList, EMPTY_BYTES, totalByteLength); + return buffer.array(); } else { return EMPTY_BYTES; } @@ -154,7 +172,6 @@ public CacheKeyBuilder appendByte(byte input) public CacheKeyBuilder appendByteArray(byte[] input) { - // TODO: check it is ok to not add input.length for fixed-size types appendItem(BYTE_ARRAY_KEY, input); return this; } @@ -165,9 +182,9 @@ public CacheKeyBuilder appendString(@Nullable String input) return this; } - public CacheKeyBuilder appendStringList(List input) + public CacheKeyBuilder appendStrings(Collection input) { - appendItem(STRING_LIST_KEY, stringListToByteArray(input)); + appendItem(STRING_LIST_KEY, stringCollectionToByteArray(input)); return this; } @@ -207,9 +224,9 @@ public CacheKeyBuilder appendCacheable(@Nullable Cacheable input) return this; } - public CacheKeyBuilder appendCacheableList(List input) + public CacheKeyBuilder appendCacheables(Collection input) { - appendItem(CACHEABLE_LIST_KEY, cacheableListToByteArray(input)); + appendItem(CACHEABLE_LIST_KEY, cacheableCollectionToByteArray(input)); return this; } diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java index 526a66a47e1c..af4d485d06fd 100644 --- a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java @@ -359,10 +359,8 @@ public byte[] computeCacheKey(GroupByQuery query) .appendByte(CACHE_STRATEGY_VERSION) .appendCacheable(query.getGranularity()) .appendCacheable(query.getDimFilter()) - .appendCacheableList(query.getAggregatorSpecs()) - .appendCacheableList(query.getDimensions()) - .appendCacheable(query.getHavingSpec()) - .appendCacheable(query.getLimitSpec()) + .appendCacheables(query.getAggregatorSpecs()) + .appendCacheables(query.getDimensions()) .build(); } diff --git a/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java index f5c2167cb441..1d986fa4845d 100644 --- a/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java @@ -135,7 +135,7 @@ public byte[] computeCacheKey(TimeseriesQuery query) .appendBoolean(query.isSkipEmptyBuckets()) .appendCacheable(query.getGranularity()) .appendCacheable(query.getDimensionsFilter()) - .appendCacheableList(query.getAggregatorSpecs()) + .appendCacheables(query.getAggregatorSpecs()) .build(); } diff --git a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java index 2122a2ac6846..73f7ab4be874 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java @@ -20,7 +20,9 @@ package io.druid.query.topn; import com.fasterxml.jackson.core.type.TypeReference; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -53,9 +55,11 @@ import org.joda.time.DateTime; import javax.annotation.Nullable; +import java.util.Collection; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Map.Entry; /** */ @@ -288,6 +292,59 @@ public TypeReference> getResultTypeReference() return TYPE_REFERENCE; } + @VisibleForTesting + static Collection findPostAggregatorsForSort( + TopNMetricSpec metricSpec, + List postAggregators, + DimensionSpec dimensionSpec + ) + { + PostAggregator found = null; + for (PostAggregator eachAggregator : postAggregators) { + if (eachAggregator.getName().equals(metricSpec.getMetricName(dimensionSpec))) { + found = eachAggregator; + break; + } + } + + if (found != null) { + final Map foundMap = Maps.newHashMap(); + final Map notCheckedMap = Maps.newHashMap(); + + for (PostAggregator eachAggregator : postAggregators) { + if (eachAggregator != found) { + notCheckedMap.put(eachAggregator.getName(), eachAggregator); + } + } + foundMap.put(found.getName(), found); + foundMap.putAll(getDependentPostAggregators(found, notCheckedMap)); + + return foundMap.values(); + } else { + return ImmutableList.of(); + } + } + + private static Map getDependentPostAggregators(PostAggregator cur, Map notChecked) + { + final Map found = Maps.newHashMap(); + + for (String eachDependentField : cur.getDependentFields()) { + for (Entry entry : notChecked.entrySet()) { + if (eachDependentField.equals(entry.getKey())) { + final String foundPostAggName = entry.getKey(); + final PostAggregator foundPostAgg = entry.getValue(); + final Map newNotChecked = Maps.newHashMap(notChecked); + found.put(foundPostAggName, foundPostAgg); + newNotChecked.remove(foundPostAggName); + found.putAll(getDependentPostAggregators(foundPostAgg, newNotChecked)); + } + } + } + + return found; + } + @Override public CacheStrategy, Object, TopNQuery> getCacheStrategy(final TopNQuery query) { @@ -303,15 +360,22 @@ public CacheStrategy, Object, TopNQuery> getCacheStrateg @Override public byte[] computeCacheKey(TopNQuery query) { - return new CacheKeyBuilder(TOPN_QUERY) + final CacheKeyBuilder builder = new CacheKeyBuilder(TOPN_QUERY) .appendCacheable(query.getDimensionSpec()) .appendCacheable(query.getTopNMetricSpec()) .appendInt(query.getThreshold()) .appendCacheable(query.getGranularity()) .appendCacheable(query.getDimensionsFilter()) - .appendCacheableList(query.getAggregatorSpecs()) - .appendCacheableList(query.getPostAggregatorSpecs()) - .build(); + .appendCacheables(query.getAggregatorSpecs()); + + final Collection sortPostAggregators = findPostAggregatorsForSort(query.getTopNMetricSpec(), query.getPostAggregatorSpecs(), query.getDimensionSpec()); + if (!sortPostAggregators.isEmpty()) { + // Append post aggregators only when they are used as sort keys. + // Note that appending an empty list produces a different cache key from not appending it. + builder.appendCacheables(sortPostAggregators); + } + + return builder.build(); } @Override diff --git a/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java b/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java index 1fd946ee4f8e..9908de06cf83 100644 --- a/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java +++ b/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java @@ -57,10 +57,10 @@ public byte[] getCacheKey() .appendDouble(2.3) .appendByteArray(CacheKeyBuilder.STRING_SEPARATOR) // test when an item is same with the separator .appendFloatArray(new float[]{10.0f, 11.0f}) - .appendStringList(Lists.newArrayList("test1", "test2")) + .appendStrings(Lists.newArrayList("test1", "test2")) .appendCacheable(cacheable) .appendCacheable(null) - .appendCacheableList(Lists.newArrayList((Cacheable) null)) + .appendCacheables(Lists.newArrayList((Cacheable) null)) .build(); final int expectedSize = 1 // id @@ -71,7 +71,7 @@ public byte[] getCacheKey() + Doubles.BYTES // 2.3 + CacheKeyBuilder.STRING_SEPARATOR.length // byte array + Floats.BYTES * 2 // 10.0f, 11.0f - + Ints.BYTES + 5 * 2 + 1 // 'test1' 'test2' + + Ints.BYTES + 5 * 2 + 1 // 'test1' 'test2' + cacheable.getCacheKey().length // cacheable + Ints.BYTES // cacheable list + 11; // type keys @@ -109,6 +109,48 @@ public byte[] getCacheKey() assertTrue(Arrays.equals(expected, actual)); } + @Test + public void testDifferentOrderList() + { + byte[] key1 = new CacheKeyBuilder((byte) 10) + .appendStrings(Lists.newArrayList("AB", "BA")) + .build(); + + byte[] key2 = new CacheKeyBuilder((byte) 10) + .appendStrings(Lists.newArrayList("BA", "AB")) + .build(); + + assertTrue(Arrays.equals(key1, key2)); + + final Cacheable cacheable1 = new Cacheable() + { + @Override + public byte[] getCacheKey() + { + return new byte[]{1}; + } + }; + + final Cacheable cacheable2 = new Cacheable() + { + @Override + public byte[] getCacheKey() + { + return new byte[]{2}; + } + }; + + key1 = new CacheKeyBuilder((byte) 10) + .appendCacheables(Lists.newArrayList(cacheable1, cacheable2)) + .build(); + + key2 = new CacheKeyBuilder((byte) 10) + .appendCacheables(Lists.newArrayList(cacheable2, cacheable1)) + .build(); + + assertTrue(Arrays.equals(key1, key2)); + } + @Test public void testNotEqualStrings() { @@ -142,39 +184,33 @@ public void testNotEqualStrings() keys.add( new CacheKeyBuilder((byte) 10) - .appendStringList(ImmutableList.of("test", "test")) - .build() - ); - - keys.add( - new CacheKeyBuilder((byte) 10) - .appendStringList(ImmutableList.of("testtest")) + .appendStrings(ImmutableList.of("test", "test")) .build() ); keys.add( new CacheKeyBuilder((byte) 10) - .appendStringList(ImmutableList.of("testtest", "")) + .appendStrings(ImmutableList.of("testtest")) .build() ); keys.add( new CacheKeyBuilder((byte) 10) - .appendStringList(ImmutableList.of("", "testtest")) + .appendStrings(ImmutableList.of("testtest", "")) .build() ); keys.add( new CacheKeyBuilder((byte) 10) - .appendStringList(ImmutableList.of("testtest")) - .appendStringList(ImmutableList.of()) + .appendStrings(ImmutableList.of("testtest")) + .appendStrings(ImmutableList.of()) .build() ); keys.add( new CacheKeyBuilder((byte) 10) - .appendStringList(ImmutableList.of()) - .appendStringList(ImmutableList.of("testtest")) + .appendStrings(ImmutableList.of()) + .appendStrings(ImmutableList.of("testtest")) .build() ); @@ -218,27 +254,27 @@ public byte[] getCacheKey() keys.add( new CacheKeyBuilder((byte) 10) - .appendCacheableList(Lists.newArrayList(test, test)) + .appendCacheables(Lists.newArrayList(test, test)) .build() ); keys.add( new CacheKeyBuilder((byte) 10) - .appendCacheableList(Lists.newArrayList(testtest)) + .appendCacheables(Lists.newArrayList(testtest)) .build() ); keys.add( new CacheKeyBuilder((byte) 10) - .appendCacheableList(Lists.newArrayList(testtest)) - .appendCacheableList(Lists.newArrayList()) + .appendCacheables(Lists.newArrayList(testtest)) + .appendCacheables(Lists.newArrayList()) .build() ); keys.add( new CacheKeyBuilder((byte) 10) - .appendCacheableList(Lists.newArrayList()) - .appendCacheableList(Lists.newArrayList(testtest)) + .appendCacheables(Lists.newArrayList()) + .appendCacheables(Lists.newArrayList(testtest)) .build() ); @@ -260,21 +296,21 @@ private static void assertNotEqualsEachOther(List keys) public void testEmptyOrNullStringLists() { byte[] key1 = new CacheKeyBuilder((byte) 10) - .appendStringList(Lists.newArrayList("", "")) + .appendStrings(Lists.newArrayList("", "")) .build(); byte[] key2 = new CacheKeyBuilder((byte) 10) - .appendStringList(Lists.newArrayList("")) + .appendStrings(Lists.newArrayList("")) .build(); assertFalse(Arrays.equals(key1, key2)); key1 = new CacheKeyBuilder((byte) 10) - .appendStringList(Lists.newArrayList("")) + .appendStrings(Lists.newArrayList("")) .build(); key2 = new CacheKeyBuilder((byte) 10) - .appendStringList(Lists.newArrayList((String) null)) + .appendStrings(Lists.newArrayList((String) null)) .build(); assertTrue(Arrays.equals(key1, key2)); @@ -283,12 +319,12 @@ public void testEmptyOrNullStringLists() @Test public void testEmptyOrNullCacheables() { - byte[] key1 = new CacheKeyBuilder((byte) 10) - .appendCacheableList(Lists.newArrayList()) + final byte[] key1 = new CacheKeyBuilder((byte) 10) + .appendCacheables(Lists.newArrayList()) .build(); - byte[] key2 = new CacheKeyBuilder((byte) 10) - .appendCacheableList(Lists.newArrayList((Cacheable) null)) + final byte[] key2 = new CacheKeyBuilder((byte) 10) + .appendCacheables(Lists.newArrayList((Cacheable) null)) .build(); assertFalse(Arrays.equals(key1, key2)); diff --git a/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java b/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java index 02d28c0b000f..bb691064bd63 100644 --- a/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java +++ b/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Lists; import com.google.common.collect.Maps; import io.druid.granularity.QueryGranularities; import io.druid.jackson.DefaultObjectMapper; @@ -50,6 +51,9 @@ import org.junit.Test; import java.util.Arrays; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; import java.util.Map; public class TopNQueryQueryToolChestTest @@ -247,4 +251,117 @@ public Sequence> run( return query.run(runner, responseContext); } } + + @Test + public void testFindPostAggregatorsForSort() + { + final TopNQuery query = new TopNQueryBuilder() + .dataSource(new TableDataSource("dummy")) + .dimension("test", "test") + .metric("sum") + .threshold(3) + .granularity(QueryGranularities.ALL) + .intervals(ImmutableList.of(new Interval("2015-01-01/2015-01-02"))) + .aggregators(ImmutableList.of(new CountAggregatorFactory("metric1"))) + .postAggregators( + ImmutableList.of( + new ArithmeticPostAggregator( + "part_sum", + "+", + ImmutableList.of( + new FieldAccessPostAggregator( + null, + "metric1" + ), + new FieldAccessPostAggregator( + null, + "metric1" + ) + ) + ), + new ArithmeticPostAggregator( + "sum", + "+", + ImmutableList.of( + new FieldAccessPostAggregator( + "part_sum", + "part_sum" + ), + new FieldAccessPostAggregator( + null, + "metric1" + ) + ) + ), + new ArithmeticPostAggregator( + "sub", + "-", + ImmutableList.of( + new FieldAccessPostAggregator( + null, + "metric1" + ), + new FieldAccessPostAggregator( + null, + "metric1" + ) + ) + ) + ) + ) + .build(); + + final List actual = Lists.newArrayList( + TopNQueryQueryToolChest.findPostAggregatorsForSort( + query.getTopNMetricSpec(), + query.getPostAggregatorSpecs(), + query.getDimensionSpec() + ) + ); + + final List expected = Lists.newArrayList( + new ArithmeticPostAggregator( + "sum", + "+", + ImmutableList.of( + new FieldAccessPostAggregator( + "part_sum", + "part_sum" + ), + new FieldAccessPostAggregator( + null, + "metric1" + ) + ) + ), + new ArithmeticPostAggregator( + "part_sum", + "+", + ImmutableList.of( + new FieldAccessPostAggregator( + null, + "metric1" + ), + new FieldAccessPostAggregator( + null, + "metric1" + ) + ) + ) + ); + + final Comparator nameComparator = new Comparator() + { + @Override + public int compare(PostAggregator o1, PostAggregator o2) + { + return o1.getName().compareTo(o2.getName()); + } + }; + + Collections.sort(actual, nameComparator); + Collections.sort(expected, nameComparator); + + Assert.assertTrue(expected.equals(actual)); + } } From ee91c3504b82f6185b89eb0c048f27d2e125a427 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Wed, 8 Feb 2017 13:01:44 +0900 Subject: [PATCH 06/10] Use assertArrayEquals() --- .../java/io/druid/query/cache/CacheKeyBuilderTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java b/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java index 9908de06cf83..17eb6630f0f6 100644 --- a/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java +++ b/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java @@ -31,9 +31,9 @@ import java.util.Arrays; import java.util.List; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; public class CacheKeyBuilderTest { @@ -106,7 +106,7 @@ public byte[] getCacheKey() .putInt(1) .array(); - assertTrue(Arrays.equals(expected, actual)); + assertArrayEquals(expected, actual); } @Test @@ -120,7 +120,7 @@ public void testDifferentOrderList() .appendStrings(Lists.newArrayList("BA", "AB")) .build(); - assertTrue(Arrays.equals(key1, key2)); + assertArrayEquals(key1, key2); final Cacheable cacheable1 = new Cacheable() { @@ -148,7 +148,7 @@ public byte[] getCacheKey() .appendCacheables(Lists.newArrayList(cacheable2, cacheable1)) .build(); - assertTrue(Arrays.equals(key1, key2)); + assertArrayEquals(key1, key2); } @Test @@ -313,7 +313,7 @@ public void testEmptyOrNullStringLists() .appendStrings(Lists.newArrayList((String) null)) .build(); - assertTrue(Arrays.equals(key1, key2)); + assertArrayEquals(key1, key2); } @Test From 6faf2ab3e6f0932b08329ef62256ef807ca709b6 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Wed, 8 Feb 2017 13:35:43 +0900 Subject: [PATCH 07/10] Improve findPostAggregatorsForSort() --- .../query/topn/TopNQueryQueryToolChest.java | 38 +++++++++++++------ .../topn/TopNQueryQueryToolChestTest.java | 32 +++++++++++++++- 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java index 73f7ab4be874..c4a8690e6bcc 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java @@ -59,7 +59,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Map.Entry; /** */ @@ -310,14 +309,22 @@ static Collection findPostAggregatorsForSort( if (found != null) { final Map foundMap = Maps.newHashMap(); final Map notCheckedMap = Maps.newHashMap(); + final List postAggregatorNameOrder = Lists.newArrayList(); for (PostAggregator eachAggregator : postAggregators) { if (eachAggregator != found) { notCheckedMap.put(eachAggregator.getName(), eachAggregator); + postAggregatorNameOrder.add(eachAggregator.getName()); + } else { + // Since inner post aggregators always appear first than outer post aggregators, + // it should be fine to break here. + break; } } foundMap.put(found.getName(), found); - foundMap.putAll(getDependentPostAggregators(found, notCheckedMap)); + foundMap.putAll( + getDependentPostAggregators(found, postAggregatorNameOrder, notCheckedMap) + ); return foundMap.values(); } else { @@ -325,19 +332,28 @@ static Collection findPostAggregatorsForSort( } } - private static Map getDependentPostAggregators(PostAggregator cur, Map notChecked) + private static Map getDependentPostAggregators( + PostAggregator cur, + List postAggregatorNameOrder, + Map notChecked + ) { final Map found = Maps.newHashMap(); for (String eachDependentField : cur.getDependentFields()) { - for (Entry entry : notChecked.entrySet()) { - if (eachDependentField.equals(entry.getKey())) { - final String foundPostAggName = entry.getKey(); - final PostAggregator foundPostAgg = entry.getValue(); - final Map newNotChecked = Maps.newHashMap(notChecked); - found.put(foundPostAggName, foundPostAgg); - newNotChecked.remove(foundPostAggName); - found.putAll(getDependentPostAggregators(foundPostAgg, newNotChecked)); + for (int i = 0; i < postAggregatorNameOrder.size(); i++) { + final String postAggregatorName = postAggregatorNameOrder.get(i); + if (eachDependentField.equals(postAggregatorName)) { + final PostAggregator foundPostAgg = notChecked.get(postAggregatorName); + found.put(postAggregatorName, foundPostAgg); + + // Further find nested post aggregators + final List remainIterOrder = postAggregatorNameOrder.subList(0, i); + final Map remainNotChecked = Maps.newHashMapWithExpectedSize(remainIterOrder.size()); + for (String remainPostAggName : remainIterOrder) { + remainNotChecked.put(remainPostAggName, notChecked.get(remainPostAggName)); + } + found.putAll(getDependentPostAggregators(foundPostAgg, remainIterOrder, remainNotChecked)); } } } diff --git a/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java b/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java index bb691064bd63..74befbdd945c 100644 --- a/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java +++ b/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java @@ -266,7 +266,7 @@ public void testFindPostAggregatorsForSort() .postAggregators( ImmutableList.of( new ArithmeticPostAggregator( - "part_sum", + "part_part_sum", "+", ImmutableList.of( new FieldAccessPostAggregator( @@ -279,6 +279,20 @@ public void testFindPostAggregatorsForSort() ) ) ), + new ArithmeticPostAggregator( + "part_sum", + "+", + ImmutableList.of( + new FieldAccessPostAggregator( + null, + "part_part_sum" + ), + new FieldAccessPostAggregator( + null, + "metric1" + ) + ) + ), new ArithmeticPostAggregator( "sum", "+", @@ -335,7 +349,7 @@ public void testFindPostAggregatorsForSort() ) ), new ArithmeticPostAggregator( - "part_sum", + "part_part_sum", "+", ImmutableList.of( new FieldAccessPostAggregator( @@ -347,6 +361,20 @@ public void testFindPostAggregatorsForSort() "metric1" ) ) + ), + new ArithmeticPostAggregator( + "part_sum", + "+", + ImmutableList.of( + new FieldAccessPostAggregator( + null, + "part_part_sum" + ), + new FieldAccessPostAggregator( + null, + "metric1" + ) + ) ) ); From 521780a597708465ba451cece591112ab8be2f2d Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Thu, 9 Feb 2017 10:34:14 +0900 Subject: [PATCH 08/10] Address comments --- .../theta/SketchSetPostAggregator.java | 27 +++- .../post/ArithmeticPostAggregator.java | 29 +++- .../post/DoubleGreatestPostAggregator.java | 2 +- .../post/DoubleLeastPostAggregator.java | 2 +- .../post/LongGreatestPostAggregator.java | 2 +- .../post/LongLeastPostAggregator.java | 2 +- .../io/druid/query/cache/CacheKeyBuilder.java | 35 +++-- .../groupby/GroupByQueryQueryToolChest.java | 4 +- .../TimeseriesQueryQueryToolChest.java | 2 +- .../query/topn/TopNQueryQueryToolChest.java | 81 +--------- .../query/cache/CacheKeyBuilderTest.java | 71 ++++++++- .../topn/TopNQueryQueryToolChestTest.java | 145 ------------------ .../client/CachingClusteredClientTest.java | 45 ++++-- 13 files changed, 190 insertions(+), 257 deletions(-) diff --git a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java index c1060d7fb934..1f1c71a2eeff 100644 --- a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java +++ b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java @@ -165,10 +165,29 @@ public int hashCode() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.DATA_SKETCHES_SKETCH_SET) + final CacheKeyBuilder builder = new CacheKeyBuilder(PostAggregatorIds.DATA_SKETCHES_SKETCH_SET) .appendString(getFunc()) - .appendCacheables(fields) - .appendInt(maxSketchSize) - .build(); + .appendInt(maxSketchSize); + + if (preserveFieldOrder(func)) { + builder.appendCacheables(fields); + } else { + builder.appendCacheablesIgnoringOrder(fields); + } + + return builder.build(); + } + + private static boolean preserveFieldOrder(SketchHolder.Func func) + { + switch (func) { + case NOT: + return true; + case UNION: + case INTERSECT: + return false; + default: + throw new IAE(func.name()); + } } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java index ff3906feb5ea..181dfec2f08d 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java @@ -127,11 +127,17 @@ public String getName() @Override public byte[] getCacheKey() { - return new CacheKeyBuilder(PostAggregatorIds.ARITHMETIC) + final CacheKeyBuilder builder = new CacheKeyBuilder(PostAggregatorIds.ARITHMETIC) .appendString(fnName) - .appendCacheables(fields) - .appendString(ordering) - .build(); + .appendString(ordering); + + if (preserveFieldOrder(op)) { + builder.appendCacheables(fields); + } else { + builder.appendCacheablesIgnoringOrder(fields); + } + + return builder.build(); } @JsonProperty("fn") @@ -163,6 +169,21 @@ public String toString() '}'; } + private static boolean preserveFieldOrder(Ops op) + { + switch (op) { + case PLUS: + case MULT: + return false; + case MINUS: + case DIV: + case QUOTIENT: + return true; + default: + throw new IAE(op.fn); + } + } + private static enum Ops { PLUS("+") diff --git a/processing/src/main/java/io/druid/query/aggregation/post/DoubleGreatestPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/DoubleGreatestPostAggregator.java index 6bee3ad49c81..e9ff5975c2c2 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/DoubleGreatestPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/DoubleGreatestPostAggregator.java @@ -148,7 +148,7 @@ public int hashCode() public byte[] getCacheKey() { return new CacheKeyBuilder(PostAggregatorIds.DOUBLE_GREATEST) - .appendCacheables(fields) + .appendCacheablesIgnoringOrder(fields) .build(); } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/DoubleLeastPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/DoubleLeastPostAggregator.java index dd7f55851b7d..e02d237e721e 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/DoubleLeastPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/DoubleLeastPostAggregator.java @@ -148,7 +148,7 @@ public int hashCode() public byte[] getCacheKey() { return new CacheKeyBuilder(PostAggregatorIds.DOUBLE_LEAST) - .appendCacheables(fields) + .appendCacheablesIgnoringOrder(fields) .build(); } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/LongGreatestPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/LongGreatestPostAggregator.java index 786d2a1d370f..a9b4391dac61 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/LongGreatestPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/LongGreatestPostAggregator.java @@ -149,7 +149,7 @@ public int hashCode() public byte[] getCacheKey() { return new CacheKeyBuilder(PostAggregatorIds.LONG_GREATEST) - .appendCacheables(fields) + .appendCacheablesIgnoringOrder(fields) .build(); } } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/LongLeastPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/LongLeastPostAggregator.java index 9c4063fbb3f5..a9a3fc7e2c01 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/LongLeastPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/LongLeastPostAggregator.java @@ -149,7 +149,7 @@ public int hashCode() public byte[] getCacheKey() { return new CacheKeyBuilder(PostAggregatorIds.LONG_LEAST) - .appendCacheables(fields) + .appendCacheablesIgnoringOrder(fields) .build(); } } diff --git a/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java b/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java index 165cdbe10f53..19c047f91f5a 100644 --- a/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java +++ b/processing/src/main/java/io/druid/query/cache/CacheKeyBuilder.java @@ -88,7 +88,7 @@ private static byte[] cacheableToByteArray(@Nullable Cacheable cacheable) } } - private static byte[] stringCollectionToByteArray(Collection input) + private static byte[] stringCollectionToByteArray(Collection input, boolean preserveOrder) { return collectionToByteArray( input, @@ -100,11 +100,12 @@ public byte[] apply(@Nullable String input) return StringUtils.toUtf8WithNullToEmpty(input); } }, - STRING_SEPARATOR + STRING_SEPARATOR, + preserveOrder ); } - private static byte[] cacheableCollectionToByteArray(Collection input) + private static byte[] cacheableCollectionToByteArray(Collection input, boolean preserveOrder) { return collectionToByteArray( input, @@ -116,14 +117,16 @@ public byte[] apply(@Nullable Cacheable input) return input == null ? EMPTY_BYTES : input.getCacheKey(); } }, - EMPTY_BYTES + EMPTY_BYTES, + preserveOrder ); } private static byte[] collectionToByteArray( Collection collection, Function serializeFunction, - byte[] separator + byte[] separator, + boolean preserveOrder ) { if (collection.size() > 0) { @@ -135,8 +138,10 @@ private static byte[] collectionToByteArray( byteArrayList.add(byteArray); } - // Sort the byte array list to guarantee that collections of same items but in different orders make the same result - Collections.sort(byteArrayList, UnsignedBytes.lexicographicalComparator()); + if (!preserveOrder) { + // Sort the byte array list to guarantee that collections of same items but in different orders make the same result + Collections.sort(byteArrayList, UnsignedBytes.lexicographicalComparator()); + } final Iterator iterator = byteArrayList.iterator(); final int bufSize = Ints.BYTES + separator.length * (byteArrayList.size() - 1) + totalByteLength; @@ -184,7 +189,13 @@ public CacheKeyBuilder appendString(@Nullable String input) public CacheKeyBuilder appendStrings(Collection input) { - appendItem(STRING_LIST_KEY, stringCollectionToByteArray(input)); + appendItem(STRING_LIST_KEY, stringCollectionToByteArray(input, true)); + return this; + } + + public CacheKeyBuilder appendStringsIgnoringOrder(Collection input) + { + appendItem(STRING_LIST_KEY, stringCollectionToByteArray(input, false)); return this; } @@ -226,7 +237,13 @@ public CacheKeyBuilder appendCacheable(@Nullable Cacheable input) public CacheKeyBuilder appendCacheables(Collection input) { - appendItem(CACHEABLE_LIST_KEY, cacheableCollectionToByteArray(input)); + appendItem(CACHEABLE_LIST_KEY, cacheableCollectionToByteArray(input, true)); + return this; + } + + public CacheKeyBuilder appendCacheablesIgnoringOrder(Collection input) + { + appendItem(CACHEABLE_LIST_KEY, cacheableCollectionToByteArray(input, false)); return this; } diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java index af4d485d06fd..57979634becb 100644 --- a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java @@ -359,8 +359,8 @@ public byte[] computeCacheKey(GroupByQuery query) .appendByte(CACHE_STRATEGY_VERSION) .appendCacheable(query.getGranularity()) .appendCacheable(query.getDimFilter()) - .appendCacheables(query.getAggregatorSpecs()) - .appendCacheables(query.getDimensions()) + .appendCacheablesIgnoringOrder(query.getAggregatorSpecs()) + .appendCacheablesIgnoringOrder(query.getDimensions()) .build(); } diff --git a/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java index 1d986fa4845d..a399af557e76 100644 --- a/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/timeseries/TimeseriesQueryQueryToolChest.java @@ -135,7 +135,7 @@ public byte[] computeCacheKey(TimeseriesQuery query) .appendBoolean(query.isSkipEmptyBuckets()) .appendCacheable(query.getGranularity()) .appendCacheable(query.getDimensionsFilter()) - .appendCacheables(query.getAggregatorSpecs()) + .appendCacheablesIgnoringOrder(query.getAggregatorSpecs()) .build(); } diff --git a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java index c4a8690e6bcc..bc394b869f54 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java @@ -20,9 +20,7 @@ package io.druid.query.topn; import com.fasterxml.jackson.core.type.TypeReference; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; -import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -55,7 +53,6 @@ import org.joda.time.DateTime; import javax.annotation.Nullable; -import java.util.Collection; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -100,7 +97,7 @@ public String apply(@Nullable AggregatorFactory input) ).toArray(new String[0]); } - private static List prunePostAggregators(TopNQuery query) + public static List prunePostAggregators(TopNQuery query) { return AggregatorUtil.pruneDependentPostAgg( query.getPostAggregatorSpecs(), @@ -291,75 +288,7 @@ public TypeReference> getResultTypeReference() return TYPE_REFERENCE; } - @VisibleForTesting - static Collection findPostAggregatorsForSort( - TopNMetricSpec metricSpec, - List postAggregators, - DimensionSpec dimensionSpec - ) - { - PostAggregator found = null; - for (PostAggregator eachAggregator : postAggregators) { - if (eachAggregator.getName().equals(metricSpec.getMetricName(dimensionSpec))) { - found = eachAggregator; - break; - } - } - - if (found != null) { - final Map foundMap = Maps.newHashMap(); - final Map notCheckedMap = Maps.newHashMap(); - final List postAggregatorNameOrder = Lists.newArrayList(); - for (PostAggregator eachAggregator : postAggregators) { - if (eachAggregator != found) { - notCheckedMap.put(eachAggregator.getName(), eachAggregator); - postAggregatorNameOrder.add(eachAggregator.getName()); - } else { - // Since inner post aggregators always appear first than outer post aggregators, - // it should be fine to break here. - break; - } - } - foundMap.put(found.getName(), found); - foundMap.putAll( - getDependentPostAggregators(found, postAggregatorNameOrder, notCheckedMap) - ); - - return foundMap.values(); - } else { - return ImmutableList.of(); - } - } - - private static Map getDependentPostAggregators( - PostAggregator cur, - List postAggregatorNameOrder, - Map notChecked - ) - { - final Map found = Maps.newHashMap(); - - for (String eachDependentField : cur.getDependentFields()) { - for (int i = 0; i < postAggregatorNameOrder.size(); i++) { - final String postAggregatorName = postAggregatorNameOrder.get(i); - if (eachDependentField.equals(postAggregatorName)) { - final PostAggregator foundPostAgg = notChecked.get(postAggregatorName); - found.put(postAggregatorName, foundPostAgg); - - // Further find nested post aggregators - final List remainIterOrder = postAggregatorNameOrder.subList(0, i); - final Map remainNotChecked = Maps.newHashMapWithExpectedSize(remainIterOrder.size()); - for (String remainPostAggName : remainIterOrder) { - remainNotChecked.put(remainPostAggName, notChecked.get(remainPostAggName)); - } - found.putAll(getDependentPostAggregators(foundPostAgg, remainIterOrder, remainNotChecked)); - } - } - } - - return found; - } @Override public CacheStrategy, Object, TopNQuery> getCacheStrategy(final TopNQuery query) @@ -382,13 +311,13 @@ public byte[] computeCacheKey(TopNQuery query) .appendInt(query.getThreshold()) .appendCacheable(query.getGranularity()) .appendCacheable(query.getDimensionsFilter()) - .appendCacheables(query.getAggregatorSpecs()); + .appendCacheablesIgnoringOrder(query.getAggregatorSpecs()); - final Collection sortPostAggregators = findPostAggregatorsForSort(query.getTopNMetricSpec(), query.getPostAggregatorSpecs(), query.getDimensionSpec()); - if (!sortPostAggregators.isEmpty()) { + final List postAggregators = prunePostAggregators(query); + if (!postAggregators.isEmpty()) { // Append post aggregators only when they are used as sort keys. // Note that appending an empty list produces a different cache key from not appending it. - builder.appendCacheables(sortPostAggregators); + builder.appendCacheablesIgnoringOrder(postAggregators); } return builder.build(); diff --git a/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java b/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java index 17eb6630f0f6..d0a857a943d3 100644 --- a/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java +++ b/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java @@ -60,7 +60,7 @@ public byte[] getCacheKey() .appendStrings(Lists.newArrayList("test1", "test2")) .appendCacheable(cacheable) .appendCacheable(null) - .appendCacheables(Lists.newArrayList((Cacheable) null)) + .appendCacheables(Lists.newArrayList(cacheable, null, cacheable)) .build(); final int expectedSize = 1 // id @@ -73,7 +73,7 @@ public byte[] getCacheKey() + Floats.BYTES * 2 // 10.0f, 11.0f + Ints.BYTES + 5 * 2 + 1 // 'test1' 'test2' + cacheable.getCacheKey().length // cacheable - + Ints.BYTES // cacheable list + + Ints.BYTES + 4 // cacheable list + 11; // type keys assertEquals(expectedSize, actual.length); @@ -103,7 +103,9 @@ public byte[] getCacheKey() .put(cacheable.getCacheKey()) .put(CacheKeyBuilder.CACHEABLE_KEY) .put(CacheKeyBuilder.CACHEABLE_LIST_KEY) - .putInt(1) + .putInt(3) + .put(cacheable.getCacheKey()) + .put(cacheable.getCacheKey()) .array(); assertArrayEquals(expected, actual); @@ -329,4 +331,67 @@ public void testEmptyOrNullCacheables() assertFalse(Arrays.equals(key1, key2)); } + + @Test + public void testIgnoringOrder() + { + byte[] actual = new CacheKeyBuilder((byte) 10) + .appendStringsIgnoringOrder(Lists.newArrayList("test2", "test1", "te")) + .build(); + + byte[] expected = ByteBuffer.allocate(20) + .put((byte) 10) + .put(CacheKeyBuilder.STRING_LIST_KEY) + .putInt(3) + .put(StringUtils.toUtf8("te")) + .put(CacheKeyBuilder.STRING_SEPARATOR) + .put(StringUtils.toUtf8("test1")) + .put(CacheKeyBuilder.STRING_SEPARATOR) + .put(StringUtils.toUtf8("test2")) + .array(); + + assertArrayEquals(expected, actual); + + final Cacheable c1 = new Cacheable() + { + @Override + public byte[] getCacheKey() + { + return "te".getBytes(); + } + }; + + final Cacheable c2 = new Cacheable() + { + @Override + public byte[] getCacheKey() + { + return "test1".getBytes(); + } + }; + + final Cacheable c3 = new Cacheable() + { + @Override + public byte[] getCacheKey() + { + return "test2".getBytes(); + } + }; + + actual = new CacheKeyBuilder((byte) 10) + .appendCacheablesIgnoringOrder(Lists.newArrayList(c3, c2, c1)) + .build(); + + expected = ByteBuffer.allocate(18) + .put((byte) 10) + .put(CacheKeyBuilder.CACHEABLE_LIST_KEY) + .putInt(3) + .put(c1.getCacheKey()) + .put(c2.getCacheKey()) + .put(c3.getCacheKey()) + .array(); + + assertArrayEquals(expected, actual); + } } diff --git a/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java b/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java index 74befbdd945c..02d28c0b000f 100644 --- a/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java +++ b/processing/src/test/java/io/druid/query/topn/TopNQueryQueryToolChestTest.java @@ -22,7 +22,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Lists; import com.google.common.collect.Maps; import io.druid.granularity.QueryGranularities; import io.druid.jackson.DefaultObjectMapper; @@ -51,9 +50,6 @@ import org.junit.Test; import java.util.Arrays; -import java.util.Collections; -import java.util.Comparator; -import java.util.List; import java.util.Map; public class TopNQueryQueryToolChestTest @@ -251,145 +247,4 @@ public Sequence> run( return query.run(runner, responseContext); } } - - @Test - public void testFindPostAggregatorsForSort() - { - final TopNQuery query = new TopNQueryBuilder() - .dataSource(new TableDataSource("dummy")) - .dimension("test", "test") - .metric("sum") - .threshold(3) - .granularity(QueryGranularities.ALL) - .intervals(ImmutableList.of(new Interval("2015-01-01/2015-01-02"))) - .aggregators(ImmutableList.of(new CountAggregatorFactory("metric1"))) - .postAggregators( - ImmutableList.of( - new ArithmeticPostAggregator( - "part_part_sum", - "+", - ImmutableList.of( - new FieldAccessPostAggregator( - null, - "metric1" - ), - new FieldAccessPostAggregator( - null, - "metric1" - ) - ) - ), - new ArithmeticPostAggregator( - "part_sum", - "+", - ImmutableList.of( - new FieldAccessPostAggregator( - null, - "part_part_sum" - ), - new FieldAccessPostAggregator( - null, - "metric1" - ) - ) - ), - new ArithmeticPostAggregator( - "sum", - "+", - ImmutableList.of( - new FieldAccessPostAggregator( - "part_sum", - "part_sum" - ), - new FieldAccessPostAggregator( - null, - "metric1" - ) - ) - ), - new ArithmeticPostAggregator( - "sub", - "-", - ImmutableList.of( - new FieldAccessPostAggregator( - null, - "metric1" - ), - new FieldAccessPostAggregator( - null, - "metric1" - ) - ) - ) - ) - ) - .build(); - - final List actual = Lists.newArrayList( - TopNQueryQueryToolChest.findPostAggregatorsForSort( - query.getTopNMetricSpec(), - query.getPostAggregatorSpecs(), - query.getDimensionSpec() - ) - ); - - final List expected = Lists.newArrayList( - new ArithmeticPostAggregator( - "sum", - "+", - ImmutableList.of( - new FieldAccessPostAggregator( - "part_sum", - "part_sum" - ), - new FieldAccessPostAggregator( - null, - "metric1" - ) - ) - ), - new ArithmeticPostAggregator( - "part_part_sum", - "+", - ImmutableList.of( - new FieldAccessPostAggregator( - null, - "metric1" - ), - new FieldAccessPostAggregator( - null, - "metric1" - ) - ) - ), - new ArithmeticPostAggregator( - "part_sum", - "+", - ImmutableList.of( - new FieldAccessPostAggregator( - null, - "part_part_sum" - ), - new FieldAccessPostAggregator( - null, - "metric1" - ) - ) - ) - ); - - final Comparator nameComparator = new Comparator() - { - @Override - public int compare(PostAggregator o1, PostAggregator o2) - { - return o1.getName().compareTo(o2.getName()); - } - }; - - Collections.sort(actual, nameComparator); - Collections.sort(expected, nameComparator); - - Assert.assertTrue(expected.equals(actual)); - } } diff --git a/server/src/test/java/io/druid/client/CachingClusteredClientTest.java b/server/src/test/java/io/druid/client/CachingClusteredClientTest.java index 277e6ece9732..61cb2e5e404e 100644 --- a/server/src/test/java/io/druid/client/CachingClusteredClientTest.java +++ b/server/src/test/java/io/druid/client/CachingClusteredClientTest.java @@ -209,6 +209,32 @@ public class CachingClusteredClientTest new LongSumAggregatorFactory("imps", "imps"), new LongSumAggregatorFactory("impers2", "imps") ); + private static final List DIFF_ORDER_POST_AGGS = Arrays.asList( + new ArithmeticPostAggregator( + "avg_imps_per_row", + "/", + Arrays.asList( + new FieldAccessPostAggregator("imps", "imps"), + new FieldAccessPostAggregator("rows", "rows") + ) + ), + new ArithmeticPostAggregator( + "avg_imps_per_row_half", + "/", + Arrays.asList( + new FieldAccessPostAggregator("avg_imps_per_row", "avg_imps_per_row"), + new ConstantPostAggregator("constant", 2) + ) + ), + new ArithmeticPostAggregator( + "avg_imps_per_row_double", + "*", + Arrays.asList( + new FieldAccessPostAggregator("avg_imps_per_row", "avg_imps_per_row"), + new ConstantPostAggregator("constant", 2) + ) + ) + ); private static final DimFilter DIM_FILTER = null; private static final List RENAMED_POST_AGGS = ImmutableList.of(); private static final QueryGranularity GRANULARITY = QueryGranularities.DAY; @@ -271,7 +297,7 @@ public static Iterable constructorFeeder() throws IOException new Function() { @Override - public Object[] apply(@Nullable Integer input) + public Object[] apply(Integer input) { return new Object[]{input}; } @@ -765,10 +791,11 @@ public void testTopNCaching() throws Exception .context(CONTEXT); QueryRunner runner = new FinalizeResultsQueryRunner( - client, new TopNQueryQueryToolChest( - new TopNQueryConfig(), - QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator() - ) + client, + new TopNQueryQueryToolChest( + new TopNQueryConfig(), + QueryRunnerTestHelper.NoopIntervalChunkingQueryRunnerDecorator() + ) ); testQueryCaching( @@ -818,7 +845,7 @@ client, new TopNQueryQueryToolChest( builder.intervals("2011-01-01/2011-01-10") .metric("imps") .aggregators(RENAMED_AGGS) - .postAggregators(POST_AGGS) + .postAggregators(DIFF_ORDER_POST_AGGS) .build(), context ) @@ -872,7 +899,7 @@ client, new TopNQueryQueryToolChest( builder.intervals("2011-11-04/2011-11-08") .metric("imps") .aggregators(RENAMED_AGGS) - .postAggregators(POST_AGGS) + .postAggregators(DIFF_ORDER_POST_AGGS) .build(), context ) @@ -994,7 +1021,7 @@ client, new TopNQueryQueryToolChest( builder.intervals("2011-01-01/2011-01-10") .metric("imps") .aggregators(RENAMED_AGGS) - .postAggregators(POST_AGGS) + .postAggregators(DIFF_ORDER_POST_AGGS) .build(), context ) @@ -1068,7 +1095,7 @@ client, new TopNQueryQueryToolChest( builder.intervals("2011-01-01/2011-01-10") .metric("avg_imps_per_row_double") .aggregators(AGGS) - .postAggregators(POST_AGGS) + .postAggregators(DIFF_ORDER_POST_AGGS) .build(), context ) From a49cdaf46c92c4a43bfaac9e61c4d9d294108d1f Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Thu, 9 Feb 2017 11:15:58 +0900 Subject: [PATCH 09/10] fix test failure --- .../java/io/druid/query/cache/CacheKeyBuilderTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java b/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java index d0a857a943d3..5a8e277eb19f 100644 --- a/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java +++ b/processing/src/test/java/io/druid/query/cache/CacheKeyBuilderTest.java @@ -115,11 +115,11 @@ public byte[] getCacheKey() public void testDifferentOrderList() { byte[] key1 = new CacheKeyBuilder((byte) 10) - .appendStrings(Lists.newArrayList("AB", "BA")) + .appendStringsIgnoringOrder(Lists.newArrayList("AB", "BA")) .build(); byte[] key2 = new CacheKeyBuilder((byte) 10) - .appendStrings(Lists.newArrayList("BA", "AB")) + .appendStringsIgnoringOrder(Lists.newArrayList("BA", "AB")) .build(); assertArrayEquals(key1, key2); @@ -143,11 +143,11 @@ public byte[] getCacheKey() }; key1 = new CacheKeyBuilder((byte) 10) - .appendCacheables(Lists.newArrayList(cacheable1, cacheable2)) + .appendCacheablesIgnoringOrder(Lists.newArrayList(cacheable1, cacheable2)) .build(); key2 = new CacheKeyBuilder((byte) 10) - .appendCacheables(Lists.newArrayList(cacheable2, cacheable1)) + .appendCacheablesIgnoringOrder(Lists.newArrayList(cacheable2, cacheable1)) .build(); assertArrayEquals(key1, key2); From e186d672a6cf94b96b75a058b3f34a00916f6233 Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Thu, 9 Feb 2017 13:13:38 +0900 Subject: [PATCH 10/10] address comments --- .../datasketches/theta/SketchSetPostAggregator.java | 4 ++-- .../query/aggregation/post/ArithmeticPostAggregator.java | 4 ++-- .../java/io/druid/query/topn/TopNQueryQueryToolChest.java | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java index 1f1c71a2eeff..7ebd5dd097f8 100644 --- a/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java +++ b/extensions-core/datasketches/src/main/java/io/druid/query/aggregation/datasketches/theta/SketchSetPostAggregator.java @@ -169,7 +169,7 @@ public byte[] getCacheKey() .appendString(getFunc()) .appendInt(maxSketchSize); - if (preserveFieldOrder(func)) { + if (preserveFieldOrderInCacheKey(func)) { builder.appendCacheables(fields); } else { builder.appendCacheablesIgnoringOrder(fields); @@ -178,7 +178,7 @@ public byte[] getCacheKey() return builder.build(); } - private static boolean preserveFieldOrder(SketchHolder.Func func) + private static boolean preserveFieldOrderInCacheKey(SketchHolder.Func func) { switch (func) { case NOT: diff --git a/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java index 181dfec2f08d..42cfcedaeb3c 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java @@ -131,7 +131,7 @@ public byte[] getCacheKey() .appendString(fnName) .appendString(ordering); - if (preserveFieldOrder(op)) { + if (preserveFieldOrderInCacheKey(op)) { builder.appendCacheables(fields); } else { builder.appendCacheablesIgnoringOrder(fields); @@ -169,7 +169,7 @@ public String toString() '}'; } - private static boolean preserveFieldOrder(Ops op) + private static boolean preserveFieldOrderInCacheKey(Ops op) { switch (op) { case PLUS: diff --git a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java index bc394b869f54..6f876de37e25 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java @@ -97,7 +97,7 @@ public String apply(@Nullable AggregatorFactory input) ).toArray(new String[0]); } - public static List prunePostAggregators(TopNQuery query) + private static List prunePostAggregators(TopNQuery query) { return AggregatorUtil.pruneDependentPostAgg( query.getPostAggregatorSpecs(),