From 0c8e9ea55f865e7fe500433ae044b42fe84696de Mon Sep 17 00:00:00 2001 From: qutang1 Date: Wed, 4 Dec 2019 10:47:35 -0800 Subject: [PATCH 1/2] Fix double-checked locking in predicate for SelectorDimFilter --- .../druid/query/filter/SelectorDimFilter.java | 120 +++++++++--------- 1 file changed, 57 insertions(+), 63 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/filter/SelectorDimFilter.java b/processing/src/main/java/org/apache/druid/query/filter/SelectorDimFilter.java index ef351a5c41e0..082422e1cee1 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/SelectorDimFilter.java +++ b/processing/src/main/java/org/apache/druid/query/filter/SelectorDimFilter.java @@ -25,6 +25,8 @@ import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.base.Predicates; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Range; import com.google.common.collect.RangeSet; @@ -58,9 +60,11 @@ public class SelectorDimFilter implements DimFilter private final Object initLock = new Object(); - private DruidLongPredicate longPredicate; - private DruidFloatPredicate floatPredicate; - private DruidDoublePredicate druidDoublePredicate; + private volatile DruidLongPredicate longPredicate; + private volatile DruidFloatPredicate floatPredicate; + private volatile DruidDoublePredicate druidDoublePredicate; + + @JsonCreator public SelectorDimFilter( @@ -71,7 +75,9 @@ public SelectorDimFilter( ) { Preconditions.checkArgument(dimension != null, "dimension must not be null"); - + this.longPredicate = makeLongPredicateSupplier().get(); + this.floatPredicate = makeFloatPredicateSupplier().get(); + this.druidDoublePredicate = makeDoublePredicateSupplier().get(); this.dimension = dimension; this.value = NullHandling.emptyToNullIfNeeded(value); this.extractionFn = extractionFn; @@ -121,21 +127,18 @@ public Predicate makeStringPredicate() @Override public DruidLongPredicate makeLongPredicate() { - initLongPredicate(); return longPredicate; } @Override public DruidFloatPredicate makeFloatPredicate() { - initFloatPredicate(); return floatPredicate; } @Override public DruidDoublePredicate makeDoublePredicate() { - initDoublePredicate(); return druidDoublePredicate; } }; @@ -227,76 +230,67 @@ public Set getRequiredColumns() } - private void initLongPredicate() + private DruidLongPredicate createLongPredicate() { - if (longPredicate != null) { - return; + if (value == null) { + return DruidLongPredicate.MATCH_NULL_ONLY; } - synchronized (initLock) { - if (longPredicate != null) { - return; - } - if (value == null) { - longPredicate = DruidLongPredicate.MATCH_NULL_ONLY; - return; - } - final Long valueAsLong = GuavaUtils.tryParseLong(value); - if (valueAsLong == null) { - longPredicate = DruidLongPredicate.ALWAYS_FALSE; - } else { - // store the primitive, so we don't unbox for every comparison - final long unboxedLong = valueAsLong.longValue(); - longPredicate = input -> input == unboxedLong; - } + final Long valueAsLong = GuavaUtils.tryParseLong(value); + if (valueAsLong == null) { + return DruidLongPredicate.ALWAYS_FALSE; + } else { + // store the primitive, so we don't unbox for every comparison + final long unboxedLong = valueAsLong.longValue(); + return input -> input == unboxedLong; } } + + private Supplier makeLongPredicateSupplier() + { + Supplier longPredicate = () -> createLongPredicate(); + return Suppliers.memoize(longPredicate); + } - private void initFloatPredicate() + private DruidFloatPredicate createFloatPredicate() { - if (floatPredicate != null) { - return; + if (value == null) { + return DruidFloatPredicate.MATCH_NULL_ONLY; } - synchronized (initLock) { - if (floatPredicate != null) { - return; - } - - if (value == null) { - floatPredicate = DruidFloatPredicate.MATCH_NULL_ONLY; - return; - } - final Float valueAsFloat = Floats.tryParse(value); + final Float valueAsFloat = Floats.tryParse(value); - if (valueAsFloat == null) { - floatPredicate = DruidFloatPredicate.ALWAYS_FALSE; - } else { - final int floatBits = Float.floatToIntBits(valueAsFloat); - floatPredicate = input -> Float.floatToIntBits(input) == floatBits; - } + if (valueAsFloat == null) { + return DruidFloatPredicate.ALWAYS_FALSE; + } else { + final int floatBits = Float.floatToIntBits(valueAsFloat); + return input -> Float.floatToIntBits(input) == floatBits; } } - private void initDoublePredicate() + private Supplier makeFloatPredicateSupplier() { - if (druidDoublePredicate != null) { - return; + Supplier floatPredicate = () -> createFloatPredicate(); + return Suppliers.memoize(floatPredicate); + } + + + private DruidDoublePredicate createDoublePredicate() + { + if (value == null) { + return DruidDoublePredicate.MATCH_NULL_ONLY; } - synchronized (initLock) { - if (druidDoublePredicate != null) { - return; - } - if (value == null) { - druidDoublePredicate = DruidDoublePredicate.MATCH_NULL_ONLY; - return; - } - final Double aDouble = Doubles.tryParse(value); + final Double aDouble = Doubles.tryParse(value); - if (aDouble == null) { - druidDoublePredicate = DruidDoublePredicate.ALWAYS_FALSE; - } else { - final long bits = Double.doubleToLongBits(aDouble); - druidDoublePredicate = input -> Double.doubleToLongBits(input) == bits; - } + if (aDouble == null) { + return DruidDoublePredicate.ALWAYS_FALSE; + } else { + final long bits = Double.doubleToLongBits(aDouble); + return input -> Double.doubleToLongBits(input) == bits; } } + + private Supplier makeDoublePredicateSupplier() + { + Supplier doublePredicate = () -> createDoublePredicate(); + return Suppliers.memoize(doublePredicate); + } } From 96cb38c9af34171571a5a71b9bfa20c0bea22940 Mon Sep 17 00:00:00 2001 From: qutang1 Date: Wed, 4 Dec 2019 13:05:05 -0800 Subject: [PATCH 2/2] Remove unused declaration --- .../java/org/apache/druid/query/filter/SelectorDimFilter.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/filter/SelectorDimFilter.java b/processing/src/main/java/org/apache/druid/query/filter/SelectorDimFilter.java index 082422e1cee1..68983076a1bd 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/SelectorDimFilter.java +++ b/processing/src/main/java/org/apache/druid/query/filter/SelectorDimFilter.java @@ -58,8 +58,6 @@ public class SelectorDimFilter implements DimFilter @Nullable private final FilterTuning filterTuning; - private final Object initLock = new Object(); - private volatile DruidLongPredicate longPredicate; private volatile DruidFloatPredicate floatPredicate; private volatile DruidDoublePredicate druidDoublePredicate;