SQL: swap JDBC page.timeout and query.timeout properties in query req…

…uests (#79491) * SQL: swap JDBC page.timeout and query.timeout properties in query requests resolves #79430 * rm reference to #79480 from spec
elastic · Oct 27, 2021 · 76ab406 · 76ab406
1 parent 691961d
commit 76ab406
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 12 deletions.
diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcHttpClient.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcHttpClient.java
@@ -6,8 +6,8 @@
  */
 package org.elasticsearch.xpack.sql.jdbc;
 
-import org.elasticsearch.core.Tuple;
 import org.elasticsearch.core.TimeValue;
+import org.elasticsearch.core.Tuple;
 import org.elasticsearch.xpack.sql.client.ClientVersion;
 import org.elasticsearch.xpack.sql.client.HttpClient;
 import org.elasticsearch.xpack.sql.proto.ColumnInfo;
@@ -63,8 +63,8 @@ Cursor query(String sql, List<SqlTypedParamValue> params, RequestMeta meta) thro
         SqlQueryRequest sqlRequest = new SqlQueryRequest(sql, params, conCfg.zoneId(),
                 jdbcConn.getCatalog(),
                 fetch,
-                TimeValue.timeValueMillis(meta.timeoutInMs()),
                 TimeValue.timeValueMillis(meta.queryTimeoutInMs()),
+                TimeValue.timeValueMillis(meta.pageTimeoutInMs()),
                 null,
                 Boolean.FALSE,
                 null,
@@ -82,8 +82,13 @@ Cursor query(String sql, List<SqlTypedParamValue> params, RequestMeta meta) thro
      * the scroll id to use to fetch the next page.
      */
     Tuple<String, List<List<Object>>> nextPage(String cursor, RequestMeta meta) throws SQLException {
-        SqlQueryRequest sqlRequest = new SqlQueryRequest(cursor, TimeValue.timeValueMillis(meta.timeoutInMs()),
-                TimeValue.timeValueMillis(meta.queryTimeoutInMs()), new RequestInfo(Mode.JDBC), conCfg.binaryCommunication());
+        SqlQueryRequest sqlRequest = new SqlQueryRequest(
+            cursor,
+            TimeValue.timeValueMillis(meta.queryTimeoutInMs()),
+            TimeValue.timeValueMillis(meta.pageTimeoutInMs()),
+            new RequestInfo(Mode.JDBC),
+            conCfg.binaryCommunication()
+        );
         SqlQueryResponse response = httpClient.query(sqlRequest);
         return new Tuple<>(response.cursor(), response.rows());
     }

diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/RequestMeta.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/RequestMeta.java
@@ -9,12 +9,12 @@
 class RequestMeta {
 
     private int fetchSize;
-    private long timeoutInMs;
+    private long pageTimeoutInMs;
     private long queryTimeoutInMs;
 
-    RequestMeta(int fetchSize, long timeout, long queryTimeoutInMs) {
+    RequestMeta(int fetchSize, long pageTimeoutInMs, long queryTimeoutInMs) {
         this.fetchSize = fetchSize;
-        this.timeoutInMs = timeout;
+        this.pageTimeoutInMs = pageTimeoutInMs;
         this.queryTimeoutInMs = queryTimeoutInMs;
     }
 
@@ -23,8 +23,8 @@ RequestMeta queryTimeout(long timeout) {
         return this;
     }
 
-    RequestMeta timeout(long timeout) {
-        this.timeoutInMs = timeout;
+    RequestMeta pageTimeout(long timeout) {
+        this.pageTimeoutInMs = timeout;
         return this;
     }
 
@@ -37,8 +37,8 @@ int fetchSize() {
         return fetchSize;
     }
 
-    long timeoutInMs() {
-        return timeoutInMs;
+    long pageTimeoutInMs() {
+        return pageTimeoutInMs;
     }
 
     long queryTimeoutInMs() {

diff --git a/...jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfigurationDataSourceTests.java b/...jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfigurationDataSourceTests.java
@@ -8,11 +8,15 @@
 package org.elasticsearch.xpack.sql.jdbc;
 
 import org.elasticsearch.common.xcontent.XContentHelper;
-import org.elasticsearch.xcontent.XContentType;
+import org.elasticsearch.test.http.MockRequest;
 import org.elasticsearch.test.http.MockResponse;
+import org.elasticsearch.xcontent.XContentType;
+import org.elasticsearch.xcontent.json.JsonXContent;
 
 import java.io.IOException;
 import java.net.URISyntaxException;
+import java.sql.Connection;
+import java.sql.PreparedStatement;
 import java.sql.SQLException;
 import java.util.Map;
 import java.util.Properties;
@@ -43,4 +47,36 @@ public void testDataSourceConfigurationWithSSLInURL() throws SQLException, URISy
         assertEquals(address, connection.getURL());
         JdbcConfigurationTests.assertSslConfig(allProps, connection.cfg.sslConfig());
     }
+
+    public void testTimeoutsInUrl() throws IOException, SQLException {
+        int queryTimeout = 10;
+        int pageTimeout = 20;
+
+        webServer().enqueue(
+            new MockResponse().setResponseCode(200)
+                .addHeader("Content-Type", "application/json")
+                .setBody(XContentHelper.toXContent(createCurrentVersionMainResponse(), XContentType.JSON, false).utf8ToString())
+        );
+
+        EsDataSource dataSource = new EsDataSource();
+        String address = "jdbc:es://" + webServerAddress()
+            + "/?binary.format=false&query.timeout=" + queryTimeout
+            + "&page.timeout=" + pageTimeout;
+        dataSource.setUrl(address);
+        Connection connection = dataSource.getConnection();
+        webServer().takeRequest();
+
+        webServer().enqueue(
+            new MockResponse().setResponseCode(200)
+                .addHeader("Content-Type", "application/json")
+                .setBody("{\"rows\":[],\"columns\":[]}")
+        );
+        PreparedStatement statement = connection.prepareStatement("SELECT 1");
+        statement.execute();
+        MockRequest request = webServer().takeRequest();
+
+        Map<String, Object> sqlQueryRequest = XContentHelper.convertToMap(JsonXContent.jsonXContent, request.getBody(), false);
+        assertEquals(queryTimeout + "ms", sqlQueryRequest.get("request_timeout"));
+        assertEquals(pageTimeout + "ms", sqlQueryRequest.get("page_timeout"));
+    }
 }