prestodb / presto

The official home of the Presto distributed SQL query engine for big data
http://prestodb.io
Apache License 2.0
15.74k stars 5.28k forks source link

Make TestNodeMemoryConfig success independent of running heap size #23080

Closed ghelmling closed 4 days ago

ghelmling commented 5 days ago

Description

There is a mismatch between the default values in com.facebook.presto.memory.NodeMemoryConfig, which are a factor of the runtime heap size, and the test override values in com.facebook.presto.memory.TestNodeMemoryConfig, which are hard-coded. This can cause the tests to fail when: (heap size) * (default factor) == (test override).

Motivation and Context

When run with a 5GB heap, tests will fail due to the hard-coded values:

mvn test -Dtest=com.facebook.presto.memory.TestNodeMemoryConfig -Dsurefire.failIfNoSpecifiedTests=false -pl presto-main -am -Dair.test.jvmsize=5g

[ERROR] Tests run: 4, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 1.305 s <<< FAILURE! - in com.facebook.presto.memory.TestNodeMemoryConfig
[ERROR] com.facebook.presto.memory.TestNodeMemoryConfig.testOutOfRangeBroadcastMemoryLimit  Time elapsed: 0.381 s  <<< FAILURE!
java.lang.AssertionError: SoftMaxQueryMemoryPerNode did not expect [536870912B] but found [512MB]
    at org.testng.Assert.fail(Assert.java:110)
    at org.testng.Assert.failEquals(Assert.java:1417)
    at org.testng.Assert.assertNotEqualsImpl(Assert.java:156)
    at org.testng.Assert.assertNotEquals(Assert.java:1986)
    at com.facebook.airlift.configuration.testing.ConfigAssertions.assertAttributesNotEqual(ConfigAssertions.java:228)
    at com.facebook.airlift.configuration.testing.ConfigAssertions.assertFullMapping(ConfigAssertions.java:131)
    at com.facebook.presto.memory.TestNodeMemoryConfig.testOutOfRangeBroadcastMemoryLimit(TestNodeMemoryConfig.java:97)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.testng.internal.invokers.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:135)
    at org.testng.internal.invokers.TestInvoker.invokeMethod(TestInvoker.java:673)
    at org.testng.internal.invokers.TestInvoker.invokeTestMethod(TestInvoker.java:220)
    at org.testng.internal.invokers.MethodRunner.runInSequence(MethodRunner.java:50)
    at org.testng.internal.invokers.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:945)
    at org.testng.internal.invokers.TestInvoker.invokeTestMethods(TestInvoker.java:193)
    at org.testng.internal.invokers.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
    at org.testng.internal.invokers.TestMethodWorker.run(TestMethodWorker.java:128)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at java.base/java.lang.Thread.run(Thread.java:829)

[ERROR] com.facebook.presto.memory.TestNodeMemoryConfig.testExplicitPropertyMappings  Time elapsed: 0.382 s  <<< FAILURE!
java.lang.AssertionError: SoftMaxQueryMemoryPerNode did not expect [536870912B] but found [512MB]
    at org.testng.Assert.fail(Assert.java:110)
    at org.testng.Assert.failEquals(Assert.java:1417)
    at org.testng.Assert.assertNotEqualsImpl(Assert.java:156)
    at org.testng.Assert.assertNotEquals(Assert.java:1986)
    at com.facebook.airlift.configuration.testing.ConfigAssertions.assertAttributesNotEqual(ConfigAssertions.java:228)
    at com.facebook.airlift.configuration.testing.ConfigAssertions.assertFullMapping(ConfigAssertions.java:131)
    at com.facebook.presto.memory.TestNodeMemoryConfig.testExplicitPropertyMappings(TestNodeMemoryConfig.java:70)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.testng.internal.invokers.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:135)
    at org.testng.internal.invokers.TestInvoker.invokeMethod(TestInvoker.java:673)
    at org.testng.internal.invokers.TestInvoker.invokeTestMethod(TestInvoker.java:220)
    at org.testng.internal.invokers.MethodRunner.runInSequence(MethodRunner.java:50)
    at org.testng.internal.invokers.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:945)
    at org.testng.internal.invokers.TestInvoker.invokeTestMethods(TestInvoker.java:193)
    at org.testng.internal.invokers.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
    at org.testng.internal.invokers.TestMethodWorker.run(TestMethodWorker.java:128)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at java.base/java.lang.Thread.run(Thread.java:829)

Impact

Test Plan

mvn test -Dtest=com.facebook.presto.memory.TestNodeMemoryConfig -Dsurefire.failIfNoSpecifiedTests=false -pl presto-main -am -Dair.test.jvmsize=5g

Contributor checklist

Release Notes

== NO RELEASE NOTE ==
linux-foundation-easycla[bot] commented 5 days ago

CLA Signed


The committers listed above are authorized under a signed CLA.