Skip to content

Commit 39da629

Browse files
pdabre12aditi-pandit
pdabre12
authored and
aditi-pandit
committed
Add tests for presto-plan-checker-router-plugin and CI job
1 parent 5c3560a commit 39da629

File tree

5 files changed

+356
-1
lines changed

5 files changed

+356
-1
lines changed

‎.github/workflows/prestocpp-linux-build-and-unit-test.yml‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,3 +311,60 @@ jobs:
311311
-DDATA_DIR=${RUNNER_TEMP} \
312312
-Duser.timezone=America/Bahia_Banderas \
313313
-T1C
314+
315+
prestocpp-linux-presto-plan-checker-router-plugin-tests:
316+
needs: prestocpp-linux-build-for-test
317+
runs-on: ubuntu-22.04
318+
container:
319+
image: prestodb/presto-native-dependency:0.293-20250522140509-484b00e
320+
env:
321+
MAVEN_OPTS: "-Xmx4G -XX:+ExitOnOutOfMemoryError"
322+
MAVEN_FAST_INSTALL: "-B -V --quiet -T 1C -DskipTests -Dair.check.skip-all -Dmaven.javadoc.skip=true"
323+
MAVEN_TEST: "-B -Dair.check.skip-all -Dmaven.javadoc.skip=true -DLogTestDurationListener.enabled=true --fail-at-end"
324+
steps:
325+
- uses: actions/checkout@v4
326+
- name: Fix git permissions
327+
# Usually actions/checkout does this but as we run in a container
328+
# it doesn't work
329+
run: git config --global --add safe.directory ${GITHUB_WORKSPACE}
330+
331+
- name: Download artifacts
332+
uses: actions/download-artifact@v4
333+
with:
334+
name: presto-native-build
335+
path: presto-native-execution/_build/release
336+
337+
# Permissions are lost when uploading. Details here: https://github.com/actions/upload-artifact/issues/38
338+
- name: Restore execute permissions and library path
339+
run: |
340+
chmod +x ${GITHUB_WORKSPACE}/presto-native-execution/_build/release/presto_cpp/main/presto_server
341+
chmod +x ${GITHUB_WORKSPACE}/presto-native-execution/_build/release/velox/velox/functions/remote/server/velox_functions_remote_server_main
342+
# Ensure transitive dependency libboost-iostreams is found.
343+
ldconfig /usr/local/lib
344+
345+
- name: Install OpenJDK8
346+
uses: actions/setup-java@v4
347+
with:
348+
distribution: 'temurin'
349+
java-version: '8.0.442'
350+
cache: 'maven'
351+
- name: Download nodejs to maven cache
352+
run: .github/bin/download_nodejs
353+
354+
- name: Maven install
355+
env:
356+
# Use different Maven options to install.
357+
MAVEN_OPTS: "-Xmx2G -XX:+ExitOnOutOfMemoryError"
358+
run: |
359+
for i in $(seq 1 3); do ./mvnw clean install $MAVEN_FAST_INSTALL -pl 'presto-plan-checker-router-plugin' -am && s=0 && break || s=$? && sleep 10; done; (exit $s)
360+
361+
- name: Run presto-native plan checker router plugin tests
362+
run: |
363+
export PRESTO_SERVER_PATH="${GITHUB_WORKSPACE}/presto-native-execution/_build/release/presto_cpp/main/presto_server"
364+
mvn test \
365+
${MAVEN_TEST} \
366+
-pl 'presto-plan-checker-router-plugin' \
367+
-DPRESTO_SERVER=${PRESTO_SERVER_PATH} \
368+
-DDATA_DIR=${RUNNER_TEMP} \
369+
-Duser.timezone=America/Bahia_Banderas \
370+
-T1C

‎.github/workflows/test-other-modules.yml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,4 +91,5 @@ jobs:
9191
!presto-iceberg,
9292
!presto-singlestore,
9393
!presto-native-sidecar-plugin,
94-
!presto-base-arrow-flight'
94+
!presto-base-arrow-flight,
95+
!presto-plan-checker-router-plugin'

‎presto-plan-checker-router-plugin/pom.xml‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@
7575
<artifactId>bootstrap</artifactId>
7676
</dependency>
7777

78+
<dependency>
79+
<groupId>com.facebook.airlift</groupId>
80+
<artifactId>log-manager</artifactId>
81+
</dependency>
82+
7883
<dependency>
7984
<groupId>com.facebook.airlift</groupId>
8085
<artifactId>json</artifactId>
@@ -138,6 +143,64 @@
138143
<artifactId>presto-common</artifactId>
139144
<scope>provided</scope>
140145
</dependency>
146+
147+
<!-- test dependencies-->
148+
<dependency>
149+
<groupId>org.testng</groupId>
150+
<artifactId>testng</artifactId>
151+
<scope>test</scope>
152+
</dependency>
153+
154+
<dependency>
155+
<groupId>com.facebook.presto</groupId>
156+
<artifactId>presto-tpcds</artifactId>
157+
<scope>test</scope>
158+
</dependency>
159+
160+
<dependency>
161+
<groupId>com.facebook.presto</groupId>
162+
<artifactId>presto-tests</artifactId>
163+
<scope>test</scope>
164+
</dependency>
165+
166+
167+
<dependency>
168+
<groupId>com.facebook.presto</groupId>
169+
<artifactId>presto-native-execution</artifactId>
170+
<scope>test</scope>
171+
<version>${project.version}</version>
172+
</dependency>
173+
174+
<dependency>
175+
<groupId>com.facebook.presto</groupId>
176+
<artifactId>presto-native-execution</artifactId>
177+
<scope>test</scope>
178+
<type>test-jar</type>
179+
<version>${project.version}</version>
180+
</dependency>
181+
182+
<dependency>
183+
<groupId>com.facebook.presto</groupId>
184+
<artifactId>presto-native-sidecar-plugin</artifactId>
185+
<scope>test</scope>
186+
<version>${project.version}</version>
187+
</dependency>
188+
189+
<dependency>
190+
<groupId>com.facebook.presto</groupId>
191+
<artifactId>presto-native-sidecar-plugin</artifactId>
192+
<scope>test</scope>
193+
<type>test-jar</type>
194+
<version>${project.version}</version>
195+
</dependency>
196+
197+
<dependency>
198+
<groupId>com.facebook.presto</groupId>
199+
<artifactId>presto-main-base</artifactId>
200+
<scope>test</scope>
201+
<type>test-jar</type>
202+
<version>${project.version}</version>
203+
</dependency>
141204
</dependencies>
142205

143206
<!-- Build configurations -->

‎presto-plan-checker-router-plugin/src/test/java/com/facebook/presto/router/scheduler/TestPlanCheckerProviderRouterPluginConfig.java‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
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.router.scheduler;
15+
16+
import com.google.common.collect.ImmutableMap;
17+
import io.airlift.units.Duration;
18+
import org.testng.annotations.Test;
19+
20+
import java.net.URI;
21+
import java.net.URISyntaxException;
22+
import java.util.Map;
23+
24+
import static com.facebook.airlift.configuration.testing.ConfigAssertions.assertFullMapping;
25+
import static com.facebook.airlift.configuration.testing.ConfigAssertions.assertRecordedDefaults;
26+
import static com.facebook.airlift.configuration.testing.ConfigAssertions.recordDefaults;
27+
import static java.util.concurrent.TimeUnit.MINUTES;
28+
29+
public class TestPlanCheckerProviderRouterPluginConfig
30+
{
31+
@Test
32+
public void testDefault()
33+
{
34+
assertRecordedDefaults(recordDefaults(PlanCheckerRouterPluginConfig.class)
35+
.setJavaRouterURI(null)
36+
.setNativeRouterURI(null)
37+
.setPlanCheckClustersURIs(null)
38+
.setClientRequestTimeout(new Duration(2, MINUTES)));
39+
}
40+
41+
@Test
42+
public void testExplicitPropertyMappings()
43+
throws URISyntaxException
44+
{
45+
Map<String, String> properties = new ImmutableMap.Builder<String, String>()
46+
.put("router-java-url", "192.168.0.1")
47+
.put("router-native-url", "192.168.0.2")
48+
.put("plan-check-clusters-uris", "192.168.0.3, 192.168.0.4")
49+
.put("client-request-timeout", "5m")
50+
.build();
51+
PlanCheckerRouterPluginConfig expected = new PlanCheckerRouterPluginConfig()
52+
.setJavaRouterURI(new URI("192.168.0.1"))
53+
.setNativeRouterURI(new URI("192.168.0.2"))
54+
.setPlanCheckClustersURIs("192.168.0.3, 192.168.0.4")
55+
.setClientRequestTimeout(new Duration(5, MINUTES));
56+
assertFullMapping(properties, expected);
57+
}
58+
}

‎presto-plan-checker-router-plugin/src/test/java/com/facebook/presto/router/scheduler/TestPlanCheckerRouterPlugin.java‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
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.router.scheduler;
15+
16+
import com.facebook.airlift.json.JsonCodec;
17+
import com.facebook.airlift.log.Logging;
18+
import com.facebook.presto.client.QueryError;
19+
import com.facebook.presto.client.QueryResults;
20+
import com.facebook.presto.common.ErrorType;
21+
import com.facebook.presto.nativeworker.PrestoNativeQueryRunnerUtils;
22+
import com.facebook.presto.server.MockHttpServletRequest;
23+
import com.facebook.presto.spi.PrestoException;
24+
import com.facebook.presto.spi.router.RouterRequestInfo;
25+
import com.facebook.presto.spi.router.Scheduler;
26+
import com.facebook.presto.testing.QueryRunner;
27+
import com.facebook.presto.tests.AbstractTestQueryFramework;
28+
import com.facebook.presto.tests.DistributedQueryRunner;
29+
import com.google.common.collect.ImmutableListMultimap;
30+
import com.google.common.collect.ListMultimap;
31+
import org.testng.annotations.BeforeClass;
32+
import org.testng.annotations.Test;
33+
34+
import java.net.URI;
35+
import java.util.Optional;
36+
37+
import static com.facebook.airlift.json.JsonCodec.jsonCodec;
38+
import static com.facebook.presto.client.PrestoHeaders.PRESTO_CATALOG;
39+
import static com.facebook.presto.client.PrestoHeaders.PRESTO_SCHEMA;
40+
import static com.facebook.presto.client.PrestoHeaders.PRESTO_TIME_ZONE;
41+
import static com.facebook.presto.client.PrestoHeaders.PRESTO_USER;
42+
import static com.facebook.presto.common.ErrorType.USER_ERROR;
43+
import static com.facebook.presto.nativeworker.NativeQueryRunnerUtils.createLineitem;
44+
import static com.facebook.presto.nativeworker.NativeQueryRunnerUtils.createRegion;
45+
import static com.facebook.presto.sidecar.NativeSidecarPluginQueryRunnerUtils.setupNativeSidecarPlugin;
46+
import static java.util.Collections.emptyList;
47+
import static org.testng.Assert.assertEquals;
48+
import static org.testng.Assert.assertNotNull;
49+
import static org.testng.Assert.assertSame;
50+
import static org.testng.Assert.assertTrue;
51+
52+
public class TestPlanCheckerRouterPlugin
53+
extends AbstractTestQueryFramework
54+
{
55+
private static final JsonCodec<QueryResults> QUERY_RESULTS_CODEC = jsonCodec(QueryResults.class);
56+
private PlanCheckerRouterPluginConfig planCheckerRouterConfig;
57+
58+
@BeforeClass
59+
public void setup()
60+
throws Exception
61+
{
62+
Logging.initialize();
63+
64+
URI javaRouterUri = new URI("192.168.0.1");
65+
URI nativeRouterUri = new URI("192.168.0.2");
66+
67+
URI planCheckerClusters = ((DistributedQueryRunner) getQueryRunner()).getCoordinator().getBaseUrl();
68+
planCheckerRouterConfig =
69+
new PlanCheckerRouterPluginConfig()
70+
.setJavaRouterURI(javaRouterUri)
71+
.setNativeRouterURI(nativeRouterUri)
72+
.setPlanCheckClustersURIs(planCheckerClusters.toString());
73+
}
74+
75+
@Override
76+
protected void createTables()
77+
{
78+
// TODO : We create a Java query runner and use it create tables, this fails in CI with the native query runner. Need to fix this
79+
QueryRunner queryRunner = (QueryRunner) getExpectedQueryRunner();
80+
createLineitem(queryRunner);
81+
createRegion(queryRunner);
82+
}
83+
84+
@Override
85+
protected QueryRunner createQueryRunner()
86+
throws Exception
87+
{
88+
DistributedQueryRunner queryRunner = (DistributedQueryRunner) PrestoNativeQueryRunnerUtils.nativeHiveQueryRunnerBuilder()
89+
.setAddStorageFormatToPath(true)
90+
.setFailOnNestedLoopJoin(true)
91+
.setCoordinatorSidecarEnabled(true)
92+
.build();
93+
setupNativeSidecarPlugin(queryRunner);
94+
return queryRunner;
95+
}
96+
97+
@Override
98+
protected QueryRunner createExpectedQueryRunner()
99+
throws Exception
100+
{
101+
return PrestoNativeQueryRunnerUtils.javaHiveQueryRunnerBuilder()
102+
.setAddStorageFormatToPath(true)
103+
.build();
104+
}
105+
106+
@Test
107+
public void testPlanCheckerPluginWithNativeCompatibleQueries()
108+
{
109+
Scheduler scheduler = new PlanCheckerRouterPluginScheduler(planCheckerRouterConfig);
110+
scheduler.setCandidates(planCheckerRouterConfig.getPlanCheckClustersURIs());
111+
112+
// native compatible query
113+
Optional<URI> target = scheduler.getDestination(
114+
getMockRouterRequestInfo(
115+
ImmutableListMultimap.of(
116+
PRESTO_USER, "test",
117+
PRESTO_TIME_ZONE, "America/Bahia_Banderas",
118+
PRESTO_CATALOG, "tpch",
119+
PRESTO_SCHEMA, "tiny"),
120+
"SELECT lower(comment) from region"));
121+
assertTrue(target.isPresent());
122+
assertEquals(target.get(), planCheckerRouterConfig.getNativeRouterURI());
123+
}
124+
125+
@Test
126+
public void testPlanCheckerPluginWithNativeIncompatibleQueries()
127+
{
128+
Scheduler scheduler = new PlanCheckerRouterPluginScheduler(planCheckerRouterConfig);
129+
scheduler.setCandidates(planCheckerRouterConfig.getPlanCheckClustersURIs());
130+
131+
// native incompatible query
132+
Optional<URI> target = scheduler.getDestination(
133+
getMockRouterRequestInfo(
134+
ImmutableListMultimap.of(
135+
PRESTO_USER, "test",
136+
PRESTO_TIME_ZONE, "America/Bahia_Banderas",
137+
PRESTO_CATALOG, "tpch",
138+
PRESTO_SCHEMA, "tiny"),
139+
"SELECT EXISTS(SELECT 1 WHERE l.orderkey > 0 OR l.orderkey != 3) FROM lineitem l LIMIT 1"));
140+
assertTrue(target.isPresent());
141+
assertEquals(target.get(), planCheckerRouterConfig.getJavaRouterURI());
142+
}
143+
144+
@Test
145+
public void testPlanCheckerPluginWithUserErrors()
146+
{
147+
Scheduler scheduler = new PlanCheckerRouterPluginScheduler(planCheckerRouterConfig);
148+
scheduler.setCandidates(planCheckerRouterConfig.getPlanCheckClustersURIs());
149+
150+
// queries with user error, the below query does not have a defined catalog and schema
151+
try {
152+
scheduler.getDestination(
153+
getMockRouterRequestInfo(
154+
ImmutableListMultimap.of(
155+
PRESTO_USER, "test",
156+
PRESTO_TIME_ZONE, "America/Bahia_Banderas"),
157+
"SELECT lower(comment) from region"));
158+
}
159+
catch (PrestoException e) {
160+
verifyQueryError(e, "line 1:55: Schema must be specified when session schema is not set");
161+
}
162+
}
163+
164+
private static RouterRequestInfo getMockRouterRequestInfo(ListMultimap<String, String> headers, String query)
165+
{
166+
return new RouterRequestInfo("test", Optional.empty(), emptyList(), query, new MockHttpServletRequest(headers));
167+
}
168+
169+
private static void verifyQueryError(PrestoException e, String message)
170+
{
171+
QueryError error = QUERY_RESULTS_CODEC.fromJson(e.getMessage()).getError();
172+
assertNotNull(error);
173+
assertTrue(error.getMessage().equalsIgnoreCase(message));
174+
assertSame(ErrorType.valueOf(error.getErrorType()), USER_ERROR);
175+
}
176+
}

0 commit comments

Comments
 (0)