Skip to content

Commit 0e84a02

Browse files
committed
fix: Truncate query in exceptions and add test coverage for protocol limits
With this change we are checking limits around: - headers - request size - parameter sizes The driver already passed all limits but the tests uncoverd a significant inefficiency around exception creation. With multi megabyte queries this was causing multi second hangs. Thus we now truncate the query in the exception message.
1 parent a1e5d3c commit 0e84a02

File tree

7 files changed

+171
-7
lines changed

7 files changed

+171
-7
lines changed

src/main/java/com/salesforce/datacloud/jdbc/core/HyperGrpcClientExecutor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
@Slf4j
5050
@Builder(toBuilder = true)
5151
public class HyperGrpcClientExecutor implements AutoCloseable {
52-
private static final int GRPC_INBOUND_MESSAGE_MAX_SIZE = 128 * 1024 * 1024;
52+
private static final int GRPC_INBOUND_MESSAGE_MAX_SIZE = 64 * 1024 * 1024;
5353

5454
@NonNull private final ManagedChannel channel;
5555

src/main/java/com/salesforce/datacloud/jdbc/core/StreamingResultSet.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public static StreamingResultSet of(String sql, QueryStatusListener listener) {
6969

7070
return result;
7171
} catch (Exception ex) {
72-
throw QueryExceptionHandler.createException(QUERY_FAILURE + sql, ex);
72+
throw QueryExceptionHandler.createQueryException(sql, ex);
7373
}
7474
}
7575

@@ -87,6 +87,4 @@ public String getStatus() {
8787
public boolean isReady() {
8888
return listener.isReady();
8989
}
90-
91-
private static final String QUERY_FAILURE = "Failed to execute query: ";
9290
}

src/main/java/com/salesforce/datacloud/jdbc/core/listener/AdaptiveQueryStatusListener.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public static AdaptiveQueryStatusListener of(String query, HyperGrpcClientExecut
7878
new AdaptiveQueryStatusPoller(queryId, client),
7979
new AsyncQueryStatusPoller(queryId, client));
8080
} catch (StatusRuntimeException ex) {
81-
throw QueryExceptionHandler.createException("Failed to execute query: " + query, ex);
81+
throw QueryExceptionHandler.createQueryException(query, ex);
8282
}
8383
}
8484

src/main/java/com/salesforce/datacloud/jdbc/core/listener/AsyncQueryStatusListener.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public static AsyncQueryStatusListener of(String query, HyperGrpcClientExecutor
6161
.client(client)
6262
.build();
6363
} catch (StatusRuntimeException ex) {
64-
throw QueryExceptionHandler.createException("Failed to execute query: " + query, ex);
64+
throw QueryExceptionHandler.createQueryException(query, ex);
6565
}
6666
}
6767

src/main/java/com/salesforce/datacloud/jdbc/core/listener/SyncQueryStatusListener.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public static SyncQueryStatusListener of(String query, HyperGrpcClientExecutor c
6262
.initial(result)
6363
.build();
6464
} catch (StatusRuntimeException ex) {
65-
throw QueryExceptionHandler.createException("Failed to execute query: " + query, ex);
65+
throw QueryExceptionHandler.createQueryException(query, ex);
6666
}
6767
}
6868

src/main/java/com/salesforce/datacloud/jdbc/exception/QueryExceptionHandler.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,17 @@
2828
@Slf4j
2929
@UtilityClass
3030
public class QueryExceptionHandler {
31+
// We introduce a limit to avoid truncating important details from the log due to large queries.
32+
// When testing with 60 MB queries the exception formatting also took multi second hangs.
33+
private static final int MAX_QUERY_LENGTH_IN_EXCEPTION = 16 * 1024;
34+
35+
public static DataCloudJDBCException createQueryException(String query, Exception e) {
36+
String exceptionQuery = query;
37+
if (exceptionQuery.length() > MAX_QUERY_LENGTH_IN_EXCEPTION) {
38+
exceptionQuery = exceptionQuery.substring(0, MAX_QUERY_LENGTH_IN_EXCEPTION) + "<truncated>";
39+
}
40+
return QueryExceptionHandler.createException("Failed to execute query: " + exceptionQuery, e);
41+
}
3142

3243
public static DataCloudJDBCException createException(String message, Exception e) {
3344
if (e instanceof StatusRuntimeException) {
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
/*
2+
* Copyright (c) 2024, Salesforce, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.salesforce.datacloud.jdbc.core;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
20+
21+
import com.google.common.collect.Maps;
22+
import com.salesforce.datacloud.jdbc.exception.DataCloudJDBCException;
23+
import com.salesforce.datacloud.jdbc.hyper.HyperTestBase;
24+
import lombok.SneakyThrows;
25+
import lombok.val;
26+
import org.apache.commons.lang3.StringUtils;
27+
import org.junit.jupiter.api.Test;
28+
29+
public class JDBCLimitsTest extends HyperTestBase {
30+
@Test
31+
@SneakyThrows
32+
public void testLargeQuery() {
33+
String query = "SELECT 'a', /*" + StringUtils.repeat('x', 62 * 1024 * 1024) + "*/ 'b'";
34+
// Verify that the full SQL string is submitted by checking that the value before and after the large
35+
// comment are returned
36+
assertWithStatement(statement -> {
37+
val result = statement.executeQuery(query);
38+
result.next();
39+
assertThat(result.getString(1)).isEqualTo("a");
40+
assertThat(result.getString(2)).isEqualTo("b");
41+
});
42+
}
43+
44+
@Test
45+
@SneakyThrows
46+
public void testTooLargeQuery() {
47+
String query = "SELECT 'a', /*" + StringUtils.repeat('x', 65 * 1024 * 1024) + "*/ 'b'";
48+
assertWithStatement(statement -> {
49+
assertThatExceptionOfType(DataCloudJDBCException.class).isThrownBy(() -> {
50+
statement.executeQuery(query);
51+
});
52+
});
53+
}
54+
55+
@Test
56+
@SneakyThrows
57+
public void testLargeRowResponse() {
58+
// 31 MB is the expected max row size configured in Hyper
59+
String value = StringUtils.repeat('x', 31 * 1024 * 1024);
60+
String query = "SELECT rpad('', 31*1024*1024, 'x')";
61+
// Verify that large responses are supported
62+
assertWithStatement(statement -> {
63+
val result = statement.executeQuery(query);
64+
result.next();
65+
assertThat(result.getString(1)).isEqualTo(value);
66+
});
67+
}
68+
69+
@Test
70+
@SneakyThrows
71+
public void testTooLargeRowResponse() {
72+
// 31 MB is the expected max row size configured in Hyper, thus 33 MB should be too large
73+
String query = "SELECT rpad('', 33*1024*1024, 'x')";
74+
assertWithStatement(statement -> {
75+
assertThatExceptionOfType(DataCloudJDBCException.class)
76+
.isThrownBy(() -> {
77+
statement.executeQuery(query);
78+
})
79+
.withMessageContaining("tuple size limit exceeded");
80+
});
81+
}
82+
83+
@Test
84+
@SneakyThrows
85+
public void testLargeParameterRoundtrip() {
86+
// 31 MB is the expected max row size configured in Hyper
87+
String value = StringUtils.repeat('x', 31 * 1024 * 1024);
88+
// Verify that large responses are supported
89+
assertWithConnection(connection -> {
90+
val stmt = connection.prepareStatement("SELECT ?");
91+
stmt.setString(1, value);
92+
val result = stmt.executeQuery();
93+
result.next();
94+
assertThat(result.getString(1)).isEqualTo(value);
95+
});
96+
}
97+
98+
@Test
99+
@SneakyThrows
100+
public void testLargeParameter() {
101+
// We can send requests of up to 64MB so this parameter should still be accepted
102+
String value = StringUtils.repeat('x', 63 * 1024 * 1024);
103+
// Verify that large responses are supported
104+
assertWithConnection(connection -> {
105+
val stmt = connection.prepareStatement("SELECT length(?)");
106+
stmt.setString(1, value);
107+
val result = stmt.executeQuery();
108+
result.next();
109+
assertThat(result.getInt(1)).isEqualTo(value.length());
110+
});
111+
}
112+
113+
@Test
114+
@SneakyThrows
115+
public void testTooLargeParameter() {
116+
// We can send requests of up to 64MB so this parameter should fail
117+
String value = StringUtils.repeat('x', 64 * 1024 * 1024);
118+
assertWithConnection(connection -> {
119+
assertThatExceptionOfType(DataCloudJDBCException.class).isThrownBy(() -> {
120+
val stmt = connection.prepareStatement("SELECT length(?)");
121+
stmt.setString(1, value);
122+
stmt.executeQuery();
123+
});
124+
});
125+
}
126+
127+
@Test
128+
@SneakyThrows
129+
public void testLargeHeaders() {
130+
// We expect that under 1 MB total header size should be fine, we use workload as it'll get injected into the
131+
// header
132+
val settings = Maps.immutableEntry("workload", StringUtils.repeat('x', 1000 * 1024));
133+
assertWithStatement(
134+
statement -> {
135+
val result = statement.executeQuery("SELECT 'A'");
136+
result.next();
137+
assertThat(result.getString(1)).isEqualTo("A");
138+
},
139+
settings);
140+
}
141+
142+
@Test
143+
@SneakyThrows
144+
public void testTooLargeHeaders() {
145+
// We expect that due to 1 MB total header size limit, setting such a large workload should fail
146+
val settings = Maps.immutableEntry("workload", StringUtils.repeat('x', 1024 * 1024));
147+
assertWithStatement(
148+
statement -> {
149+
assertThatExceptionOfType(DataCloudJDBCException.class).isThrownBy(() -> {
150+
statement.executeQuery("SELECT 'A'");
151+
});
152+
},
153+
settings);
154+
}
155+
}

0 commit comments

Comments
 (0)