Skip to content

Commit 49a2ccb

Browse files
Lukas WegmannLuegg
Lukas Wegmann
authored andcommitted
SQL: swap JDBC page.timeout and query.timeout properties in query requests (elastic#79491)
* SQL: swap JDBC page.timeout and query.timeout properties in query requests resolves elastic#79430 * rm reference to elastic#79480 from spec
1 parent be29fe1 commit 49a2ccb

File tree

3 files changed

+53
-12
lines changed

3 files changed

+53
-12
lines changed

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcHttpClient.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
*/
77
package org.elasticsearch.xpack.sql.jdbc;
88

9-
import org.elasticsearch.core.Tuple;
109
import org.elasticsearch.core.TimeValue;
10+
import org.elasticsearch.core.Tuple;
1111
import org.elasticsearch.xpack.sql.client.ClientVersion;
1212
import org.elasticsearch.xpack.sql.client.HttpClient;
1313
import org.elasticsearch.xpack.sql.proto.ColumnInfo;
@@ -63,8 +63,8 @@ Cursor query(String sql, List<SqlTypedParamValue> params, RequestMeta meta) thro
6363
SqlQueryRequest sqlRequest = new SqlQueryRequest(sql, params, conCfg.zoneId(),
6464
jdbcConn.getCatalog(),
6565
fetch,
66-
TimeValue.timeValueMillis(meta.timeoutInMs()),
6766
TimeValue.timeValueMillis(meta.queryTimeoutInMs()),
67+
TimeValue.timeValueMillis(meta.pageTimeoutInMs()),
6868
null,
6969
Boolean.FALSE,
7070
null,
@@ -82,8 +82,13 @@ Cursor query(String sql, List<SqlTypedParamValue> params, RequestMeta meta) thro
8282
* the scroll id to use to fetch the next page.
8383
*/
8484
Tuple<String, List<List<Object>>> nextPage(String cursor, RequestMeta meta) throws SQLException {
85-
SqlQueryRequest sqlRequest = new SqlQueryRequest(cursor, TimeValue.timeValueMillis(meta.timeoutInMs()),
86-
TimeValue.timeValueMillis(meta.queryTimeoutInMs()), new RequestInfo(Mode.JDBC), conCfg.binaryCommunication());
85+
SqlQueryRequest sqlRequest = new SqlQueryRequest(
86+
cursor,
87+
TimeValue.timeValueMillis(meta.queryTimeoutInMs()),
88+
TimeValue.timeValueMillis(meta.pageTimeoutInMs()),
89+
new RequestInfo(Mode.JDBC),
90+
conCfg.binaryCommunication()
91+
);
8792
SqlQueryResponse response = httpClient.query(sqlRequest);
8893
return new Tuple<>(response.cursor(), response.rows());
8994
}

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/RequestMeta.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@
99
class RequestMeta {
1010

1111
private int fetchSize;
12-
private long timeoutInMs;
12+
private long pageTimeoutInMs;
1313
private long queryTimeoutInMs;
1414

15-
RequestMeta(int fetchSize, long timeout, long queryTimeoutInMs) {
15+
RequestMeta(int fetchSize, long pageTimeoutInMs, long queryTimeoutInMs) {
1616
this.fetchSize = fetchSize;
17-
this.timeoutInMs = timeout;
17+
this.pageTimeoutInMs = pageTimeoutInMs;
1818
this.queryTimeoutInMs = queryTimeoutInMs;
1919
}
2020

@@ -23,8 +23,8 @@ RequestMeta queryTimeout(long timeout) {
2323
return this;
2424
}
2525

26-
RequestMeta timeout(long timeout) {
27-
this.timeoutInMs = timeout;
26+
RequestMeta pageTimeout(long timeout) {
27+
this.pageTimeoutInMs = timeout;
2828
return this;
2929
}
3030

@@ -37,8 +37,8 @@ int fetchSize() {
3737
return fetchSize;
3838
}
3939

40-
long timeoutInMs() {
41-
return timeoutInMs;
40+
long pageTimeoutInMs() {
41+
return pageTimeoutInMs;
4242
}
4343

4444
long queryTimeoutInMs() {

x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfigurationDataSourceTests.java

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,15 @@
88
package org.elasticsearch.xpack.sql.jdbc;
99

1010
import org.elasticsearch.common.xcontent.XContentHelper;
11-
import org.elasticsearch.xcontent.XContentType;
11+
import org.elasticsearch.test.http.MockRequest;
1212
import org.elasticsearch.test.http.MockResponse;
13+
import org.elasticsearch.xcontent.XContentType;
14+
import org.elasticsearch.xcontent.json.JsonXContent;
1315

1416
import java.io.IOException;
1517
import java.net.URISyntaxException;
18+
import java.sql.Connection;
19+
import java.sql.PreparedStatement;
1620
import java.sql.SQLException;
1721
import java.util.Map;
1822
import java.util.Properties;
@@ -43,4 +47,36 @@ public void testDataSourceConfigurationWithSSLInURL() throws SQLException, URISy
4347
assertEquals(address, connection.getURL());
4448
JdbcConfigurationTests.assertSslConfig(allProps, connection.cfg.sslConfig());
4549
}
50+
51+
public void testTimeoutsInUrl() throws IOException, SQLException {
52+
int queryTimeout = 10;
53+
int pageTimeout = 20;
54+
55+
webServer().enqueue(
56+
new MockResponse().setResponseCode(200)
57+
.addHeader("Content-Type", "application/json")
58+
.setBody(XContentHelper.toXContent(createCurrentVersionMainResponse(), XContentType.JSON, false).utf8ToString())
59+
);
60+
61+
EsDataSource dataSource = new EsDataSource();
62+
String address = "jdbc:es://" + webServerAddress()
63+
+ "/?binary.format=false&query.timeout=" + queryTimeout
64+
+ "&page.timeout=" + pageTimeout;
65+
dataSource.setUrl(address);
66+
Connection connection = dataSource.getConnection();
67+
webServer().takeRequest();
68+
69+
webServer().enqueue(
70+
new MockResponse().setResponseCode(200)
71+
.addHeader("Content-Type", "application/json")
72+
.setBody("{\"rows\":[],\"columns\":[]}")
73+
);
74+
PreparedStatement statement = connection.prepareStatement("SELECT 1");
75+
statement.execute();
76+
MockRequest request = webServer().takeRequest();
77+
78+
Map<String, Object> sqlQueryRequest = XContentHelper.convertToMap(JsonXContent.jsonXContent, request.getBody(), false);
79+
assertEquals(queryTimeout + "ms", sqlQueryRequest.get("request_timeout"));
80+
assertEquals(pageTimeout + "ms", sqlQueryRequest.get("page_timeout"));
81+
}
4682
}

0 commit comments

Comments
 (0)