Skip to content

Commit 02f8f9f

Browse files
pdabre12pdabre12
committed
[native] Add a config property for excluding invalid worker session properties
1 parent be5fe8c commit 02f8f9f

File tree

8 files changed

+163
-13
lines changed

8 files changed

+163
-13
lines changed

‎presto-docs/src/main/sphinx/admin/properties.rst

+11
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,17 @@ be executed within a single node.
8282

8383
The corresponding session property is :ref:`admin/properties-session:\`\`single_node_execution_enabled\`\``.
8484

85+
``exclude-invalid-worker-session-properties``
86+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
87+
88+
* **Type:** ``boolean``
89+
* **Default value:** ``false``
90+
91+
When ``exclude-invalid-worker-session-properties`` is ``true``, worker session properties that are
92+
incompatible with the cluster type are excluded. For example, when ``native-execution-enabled``
93+
is ``true``, java-worker only session properties are excluded and the native-worker only
94+
session properties are included.
95+
8596
.. _tuning-memory:
8697

8798
Memory Management Properties

‎presto-main/src/main/java/com/facebook/presto/server/ServerMainModule.java

+13-3
Original file line numberDiff line numberDiff line change
@@ -811,9 +811,19 @@ public ListeningExecutorService createResourceManagerExecutor(ResourceManagerCon
811811
// Worker session property providers
812812
MapBinder<String, WorkerSessionPropertyProvider> mapBinder =
813813
newMapBinder(binder, String.class, WorkerSessionPropertyProvider.class);
814-
mapBinder.addBinding("java-worker").to(JavaWorkerSessionPropertyProvider.class).in(Scopes.SINGLETON);
815-
if (!serverConfig.isCoordinatorSidecarEnabled()) {
816-
mapBinder.addBinding("native-worker").to(NativeWorkerSessionPropertyProvider.class).in(Scopes.SINGLETON);
814+
if (featuresConfig.isNativeExecutionEnabled()) {
815+
if (!serverConfig.isCoordinatorSidecarEnabled()) {
816+
mapBinder.addBinding("native-worker").to(NativeWorkerSessionPropertyProvider.class).in(Scopes.SINGLETON);
817+
}
818+
if (!featuresConfig.isExcludeInvalidWorkerSessionProperties()) {
819+
mapBinder.addBinding("java-worker").to(JavaWorkerSessionPropertyProvider.class).in(Scopes.SINGLETON);
820+
}
821+
}
822+
else {
823+
mapBinder.addBinding("java-worker").to(JavaWorkerSessionPropertyProvider.class).in(Scopes.SINGLETON);
824+
if (!featuresConfig.isExcludeInvalidWorkerSessionProperties()) {
825+
mapBinder.addBinding("native-worker").to(NativeWorkerSessionPropertyProvider.class).in(Scopes.SINGLETON);
826+
}
817827
}
818828

819829
// Node manager binding

‎presto-main/src/main/java/com/facebook/presto/sql/analyzer/FeaturesConfig.java

+15
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,8 @@ public class FeaturesConfig
289289
private boolean includeValuesNodeInConnectorOptimizer = true;
290290

291291
private boolean eagerPlanValidationEnabled;
292+
293+
private boolean setExcludeInvalidWorkerSessionProperties;
292294
private int eagerPlanValidationThreadPoolSize = 20;
293295

294296
private boolean prestoSparkExecutionEnvironment;
@@ -2930,4 +2932,17 @@ public FeaturesConfig setExpressionOptimizerName(String expressionOptimizerName)
29302932
this.expressionOptimizerName = expressionOptimizerName;
29312933
return this;
29322934
}
2935+
2936+
@Config("exclude-invalid-worker-session-properties")
2937+
@ConfigDescription("Exclude worker session properties from invalid clusters")
2938+
public FeaturesConfig setExcludeInvalidWorkerSessionProperties(boolean setExcludeInvalidWorkerSessionProperties)
2939+
{
2940+
this.setExcludeInvalidWorkerSessionProperties = setExcludeInvalidWorkerSessionProperties;
2941+
return this;
2942+
}
2943+
2944+
public boolean isExcludeInvalidWorkerSessionProperties()
2945+
{
2946+
return this.setExcludeInvalidWorkerSessionProperties;
2947+
}
29332948
}

‎presto-main/src/test/java/com/facebook/presto/sql/analyzer/TestFeaturesConfig.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,8 @@ public void testDefaults()
252252
.setSingleNodeExecutionEnabled(false)
253253
.setNativeExecutionScaleWritersThreadsEnabled(false)
254254
.setEnhancedCTESchedulingEnabled(true)
255-
.setExpressionOptimizerName("default"));
255+
.setExpressionOptimizerName("default")
256+
.setExcludeInvalidWorkerSessionProperties(false));
256257
}
257258

258259
@Test
@@ -454,6 +455,7 @@ public void testExplicitPropertyMappings()
454455
.put("native-execution-scale-writer-threads-enabled", "true")
455456
.put("enhanced-cte-scheduling-enabled", "false")
456457
.put("expression-optimizer-name", "custom")
458+
.put("exclude-invalid-worker-session-properties", "true")
457459
.build();
458460

459461
FeaturesConfig expected = new FeaturesConfig()
@@ -652,7 +654,8 @@ public void testExplicitPropertyMappings()
652654
.setSingleNodeExecutionEnabled(true)
653655
.setNativeExecutionScaleWritersThreadsEnabled(true)
654656
.setEnhancedCTESchedulingEnabled(false)
655-
.setExpressionOptimizerName("custom");
657+
.setExpressionOptimizerName("custom")
658+
.setExcludeInvalidWorkerSessionProperties(true);
656659
assertFullMapping(properties, expected);
657660
}
658661

‎presto-native-execution/src/test/java/com/facebook/presto/nativeworker/NativeQueryRunnerUtils.java

+1
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ public static Map<String, String> getNativeSidecarProperties()
5959
{
6060
return ImmutableMap.<String, String>builder()
6161
.put("coordinator-sidecar-enabled", "true")
62+
.put("exclude-invalid-worker-session-properties", "true")
6263
.put("presto.default-namespace", "native.default")
6364
.build();
6465
}

‎presto-native-sidecar-plugin/src/test/java/com/facebook/presto/sidecar/TestNativeSidecarPlugin.java

+1-8
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,7 @@ public void testShowSession()
8585
@Test
8686
public void testSetJavaWorkerSessionProperty()
8787
{
88-
@Language("SQL") String setSession = "SET SESSION aggregation_spill_enabled=false";
89-
MaterializedResult setSessionResult = computeActual(setSession);
90-
assertEquals(
91-
setSessionResult.toString(),
92-
"MaterializedResult{rows=[[true]], " +
93-
"types=[boolean], " +
94-
"setSessionProperties={aggregation_spill_enabled=false}, " +
95-
"resetSessionProperties=[], updateType=SET SESSION}");
88+
assertQueryFails("SET SESSION aggregation_spill_enabled=false", "line 1:1: Session property aggregation_spill_enabled does not exist");
9689
}
9790

9891
@Test

‎presto-tests/src/test/java/com/facebook/presto/tests/TestSetWorkerSessionPropertiesExcludingInvalidProperties.java

+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package com.facebook.presto.tests;
15+
16+
import com.facebook.presto.testing.MaterializedResult;
17+
import com.facebook.presto.testing.QueryRunner;
18+
import org.intellij.lang.annotations.Language;
19+
import org.testng.annotations.Test;
20+
21+
import static com.facebook.presto.testing.TestingSession.testSessionBuilder;
22+
import static org.testng.Assert.assertEquals;
23+
24+
public class TestSetWorkerSessionPropertiesExcludingInvalidProperties
25+
extends AbstractTestQueryFramework
26+
{
27+
@Override
28+
protected QueryRunner createQueryRunner()
29+
throws Exception
30+
{
31+
return DistributedQueryRunner.builder(testSessionBuilder().build())
32+
.setSingleCoordinatorProperty("exclude-invalid-worker-session-properties", "true")
33+
.build();
34+
}
35+
36+
@Test
37+
public void testSetSessionInvalidNativeWorkerSessionProperty()
38+
{
39+
// SET SESSION on a native-worker session property
40+
assertQueryFails("SET SESSION native_expression_max_array_size_in_reduce=50000", "line 1:1: Session property native_expression_max_array_size_in_reduce does not exist");
41+
}
42+
43+
@Test
44+
public void testSetSessionValidJavaWorkerSessionProperty()
45+
{
46+
// SET SESSION on a java-worker session property
47+
@Language("SQL") String setSession = "SET SESSION distinct_aggregation_spill_enabled=false";
48+
MaterializedResult setSessionResult = computeActual(setSession);
49+
assertEquals(
50+
setSessionResult.toString(),
51+
"MaterializedResult{rows=[[true]], " +
52+
"types=[boolean], " +
53+
"setSessionProperties={distinct_aggregation_spill_enabled=false}, " +
54+
"resetSessionProperties=[], updateType=SET SESSION}");
55+
}
56+
}

‎presto-tests/src/test/java/com/facebook/presto/tests/TestSetWorkerSessionPropertiesIncludingInvalidProperties.java

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package com.facebook.presto.tests;
15+
16+
import com.facebook.presto.testing.MaterializedResult;
17+
import com.facebook.presto.testing.QueryRunner;
18+
import org.intellij.lang.annotations.Language;
19+
import org.testng.annotations.Test;
20+
21+
import static com.facebook.presto.testing.TestingSession.testSessionBuilder;
22+
import static org.testng.Assert.assertEquals;
23+
24+
public class TestSetWorkerSessionPropertiesIncludingInvalidProperties
25+
extends AbstractTestQueryFramework
26+
{
27+
@Override
28+
protected QueryRunner createQueryRunner()
29+
throws Exception
30+
{
31+
return DistributedQueryRunner.builder(testSessionBuilder().build()).build();
32+
}
33+
34+
@Test
35+
public void testSetSessionValidNativeWorkerSessionProperty()
36+
{
37+
// SET SESSION on a native-worker session property
38+
@Language("SQL") String setSession = "SET SESSION native_expression_max_array_size_in_reduce=50000";
39+
MaterializedResult setSessionResult = computeActual(setSession);
40+
assertEquals(
41+
setSessionResult.toString(),
42+
"MaterializedResult{rows=[[true]], " +
43+
"types=[boolean], " +
44+
"setSessionProperties={native_expression_max_array_size_in_reduce=50000}, " +
45+
"resetSessionProperties=[], updateType=SET SESSION}");
46+
}
47+
48+
@Test
49+
public void testSetSessionValidJavaWorkerSessionProperty()
50+
{
51+
// SET SESSION on a java-worker session property
52+
@Language("SQL") String setSession = "SET SESSION distinct_aggregation_spill_enabled=false";
53+
MaterializedResult setSessionResult = computeActual(setSession);
54+
assertEquals(
55+
setSessionResult.toString(),
56+
"MaterializedResult{rows=[[true]], " +
57+
"types=[boolean], " +
58+
"setSessionProperties={distinct_aggregation_spill_enabled=false}, " +
59+
"resetSessionProperties=[], updateType=SET SESSION}");
60+
}
61+
}

0 commit comments

Comments
 (0)