[CALCITE-1808] JaninoRelMetadataProvider loading cache might cause OutOfMemoryError
Limit the size of the cache, controlled by property "saffron.metadata.handler.cache.maximum.size" (default 1,000). Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/3763abfa Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/3763abfa Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/3763abfa Branch: refs/heads/master Commit: 3763abfab2675faeabd2bca9803e86d52aa50cca Parents: 189aad1 Author: Julian Hyde <[email protected]> Authored: Thu Oct 5 13:37:30 2017 -0700 Committer: Julian Hyde <[email protected]> Committed: Fri Dec 1 18:05:17 2017 -0800 ---------------------------------------------------------------------- .../rel/metadata/JaninoRelMetadataProvider.java | 28 ++++++++++++----- .../apache/calcite/util/SaffronProperties.java | 13 ++++++++ .../java/org/apache/calcite/test/Matchers.java | 6 ++-- .../apache/calcite/test/RelMetadataTest.java | 33 +++++++++++++++++--- 4 files changed, 65 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/3763abfa/core/src/main/java/org/apache/calcite/rel/metadata/JaninoRelMetadataProvider.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/JaninoRelMetadataProvider.java b/core/src/main/java/org/apache/calcite/rel/metadata/JaninoRelMetadataProvider.java index e3c89fb..634da05 100644 --- a/core/src/main/java/org/apache/calcite/rel/metadata/JaninoRelMetadataProvider.java +++ b/core/src/main/java/org/apache/calcite/rel/metadata/JaninoRelMetadataProvider.java @@ -54,6 +54,7 @@ import org.apache.calcite.rel.stream.LogicalDelta; import org.apache.calcite.rex.RexNode; import org.apache.calcite.util.ControlFlowException; import org.apache.calcite.util.Pair; +import org.apache.calcite.util.SaffronProperties; import org.apache.calcite.util.Util; import com.google.common.cache.CacheBuilder; @@ -106,14 +107,16 @@ public class JaninoRelMetadataProvider implements RelMetadataProvider { * For the cache to be effective, providers should implement identity * correctly. */ private static final LoadingCache<Key, MetadataHandler> HANDLERS = - CacheBuilder.newBuilder().build( - new CacheLoader<Key, MetadataHandler>() { - public MetadataHandler load(@Nonnull Key key) { - //noinspection unchecked - return load3(key.def, key.provider.handlers(key.def), - key.relClasses); - } - }); + maxSize(CacheBuilder.newBuilder(), + SaffronProperties.INSTANCE.metadataHandlerCacheMaximumSize().get()) + .build( + new CacheLoader<Key, MetadataHandler>() { + public MetadataHandler load(@Nonnull Key key) { + //noinspection unchecked + return load3(key.def, key.provider.handlers(key.def), + key.relClasses); + } + }); // Pre-register the most common relational operators, to reduce the number of // times we re-generate. @@ -168,6 +171,15 @@ public class JaninoRelMetadataProvider implements RelMetadataProvider { return new JaninoRelMetadataProvider(provider); } + // helper for initialization + private static <K, V> CacheBuilder<K, V> maxSize(CacheBuilder<K, V> builder, + int size) { + if (size >= 0) { + builder.maximumSize(size); + } + return builder; + } + @Override public boolean equals(Object obj) { return obj == this || obj instanceof JaninoRelMetadataProvider http://git-wip-us.apache.org/repos/asf/calcite/blob/3763abfa/core/src/main/java/org/apache/calcite/util/SaffronProperties.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/util/SaffronProperties.java b/core/src/main/java/org/apache/calcite/util/SaffronProperties.java index 1397871..7ae8682 100644 --- a/core/src/main/java/org/apache/calcite/util/SaffronProperties.java +++ b/core/src/main/java/org/apache/calcite/util/SaffronProperties.java @@ -19,6 +19,7 @@ package org.apache.calcite.util; import org.apache.calcite.runtime.Resources; import org.apache.calcite.runtime.Resources.BooleanProp; import org.apache.calcite.runtime.Resources.Default; +import org.apache.calcite.runtime.Resources.IntProp; import org.apache.calcite.runtime.Resources.Resource; import org.apache.calcite.runtime.Resources.StringProp; @@ -96,6 +97,18 @@ public interface SaffronProperties { @Default("primary") StringProp defaultCollationStrength(); + /** + * The string property "saffron.metadata.handler.cache.maximum.size" is the + * maximum size of the cache of metadata handlers. A typical value is + * the number of queries being concurrently prepared multiplied by the number + * of types of metadata. + * + * <p>If the value is less than 0, there is no limit. The default is 1,000. + */ + @Resource("saffron.metadata.handler.cache.maximum.size") + @Default("1000") + IntProp metadataHandlerCacheMaximumSize(); + SaffronProperties INSTANCE = Helper.instance(); /** Helper class. */ http://git-wip-us.apache.org/repos/asf/calcite/blob/3763abfa/core/src/test/java/org/apache/calcite/test/Matchers.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/Matchers.java b/core/src/test/java/org/apache/calcite/test/Matchers.java index 08733a2..76d994f 100644 --- a/core/src/test/java/org/apache/calcite/test/Matchers.java +++ b/core/src/test/java/org/apache/calcite/test/Matchers.java @@ -19,6 +19,7 @@ package org.apache.calcite.test; import org.apache.calcite.util.Util; import com.google.common.base.Functions; +import com.google.common.base.Preconditions; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; @@ -134,19 +135,20 @@ public class Matchers { private final double epsilon; public IsWithin(T expectedValue, double epsilon) { + Preconditions.checkArgument(epsilon >= 0D); this.expectedValue = expectedValue; this.epsilon = epsilon; } public boolean matches(Object actualValue) { - return areEqual(actualValue, expectedValue, epsilon); + return isWithin(actualValue, expectedValue, epsilon); } public void describeTo(Description description) { description.appendValue(expectedValue + " +/-" + epsilon); } - private static boolean areEqual(Object actual, Number expected, + private static boolean isWithin(Object actual, Number expected, double epsilon) { if (actual == null) { return expected == null; http://git-wip-us.apache.org/repos/asf/calcite/blob/3763abfa/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java index 09c8a50..d6b1568 100644 --- a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java +++ b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java @@ -88,6 +88,7 @@ import org.apache.calcite.tools.Frameworks; import org.apache.calcite.tools.RelBuilder; import org.apache.calcite.util.ImmutableBitSet; import org.apache.calcite.util.ImmutableIntList; +import org.apache.calcite.util.SaffronProperties; import com.google.common.base.Function; import com.google.common.collect.ImmutableList; @@ -120,6 +121,8 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import static org.apache.calcite.test.Matchers.within; + import static org.hamcrest.CoreMatchers.endsWith; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; @@ -166,10 +169,6 @@ public class RelMetadataTest extends SqlToRelTestBase { //~ Methods ---------------------------------------------------------------- - private static Matcher<? super Number> nearTo(Number v, Number epsilon) { - return equalTo(v); // TODO: use epsilon - } - // ---------------------------------------------------------------------- // Tests for getPercentageOriginalRows // ---------------------------------------------------------------------- @@ -780,7 +779,31 @@ public class RelMetadataTest extends SqlToRelTestBase { final RelMetadataQuery mq = RelMetadataQuery.instance(); Double result = mq.getSelectivity(rel, null); assertThat(result, - nearTo(DEFAULT_COMP_SELECTIVITY * DEFAULT_EQUAL_SELECTIVITY, EPSILON)); + within(DEFAULT_COMP_SELECTIVITY * DEFAULT_EQUAL_SELECTIVITY, EPSILON)); + } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-1808";>[CALCITE-1808] + * JaninoRelMetadataProvider loading cache might cause + * OutOfMemoryError</a>. */ + @Test public void testMetadataHandlerCacheLimit() { + Assume.assumeTrue("If cache size is too large, this test may fail and the " + + "test won't be to blame", + SaffronProperties.INSTANCE.metadataHandlerCacheMaximumSize().get() + < 10_000); + final int iterationCount = 2_000; + final RelNode rel = convertSql("select * from emp"); + final RelMetadataProvider metadataProvider = + rel.getCluster().getMetadataProvider(); + final RelOptPlanner planner = rel.getCluster().getPlanner(); + for (int i = 0; i < iterationCount; i++) { + RelMetadataQuery.THREAD_PROVIDERS.set( + JaninoRelMetadataProvider.of( + new CachingRelMetadataProvider(metadataProvider, planner))); + final RelMetadataQuery mq = RelMetadataQuery.instance(); + final Double result = mq.getRowCount(rel); + assertThat(result, within(14d, 0.1d)); + } } @Test public void testDistinctRowCountTable() {
