Skip to content

Commit

Permalink
SNOW-1901399: Let query timeout be server side or client side, not both
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-dprzybysz committed Feb 20, 2025
1 parent 7f998d9 commit ef8ddbd
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 23 deletions.
5 changes: 0 additions & 5 deletions src/main/java/net/snowflake/client/core/SFBaseStatement.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,7 @@ public void addProperty(String propertyName, Object propertyValue) throws SFExce
statementParametersMap.put(propertyName, propertyValue);

if ("query_timeout".equalsIgnoreCase(propertyName)) {
// Client side implementation
queryTimeout = (Integer) propertyValue;
if (this.getSFBaseSession().getImplicitServerSideQueryTimeout()) {
// Set server parameter for supporting query timeout on async queries
statementParametersMap.put("STATEMENT_TIMEOUT_IN_SECONDS", (Integer) propertyValue);
}
}

// check if the number of session properties exceed limit
Expand Down
10 changes: 8 additions & 2 deletions src/main/java/net/snowflake/client/core/SFStatement.java
Original file line number Diff line number Diff line change
Expand Up @@ -487,8 +487,14 @@ public Object executeHelper(
// if timeout is set, start a thread to cancel the request after timeout
// reached.
if (this.queryTimeout > 0) {
executor = Executors.newScheduledThreadPool(1);
setTimeBomb(executor);
if (session.getImplicitServerSideQueryTimeout()) {
// Server side only query timeout
statementParametersMap.put("STATEMENT_TIMEOUT_IN_SECONDS", this.queryTimeout);
} else {
// client side only query timeout
executor = Executors.newScheduledThreadPool(1);
setTimeBomb(executor);
}
}

StmtUtil.StmtOutput stmtOutput = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -893,10 +893,12 @@ public void setAsyncQueryTimeout(int seconds) throws SQLException {
try {
if (this.sfBaseStatement != null) {
this.sfBaseStatement.addProperty("STATEMENT_TIMEOUT_IN_SECONDS", seconds);
// disable statement level query timeout to avoid override by connection parameter
this.setQueryTimeout(0);
}
} catch (SFException ex) {
throw new SnowflakeSQLException(
ex.getCause(), ex.getSqlState(), ex.getVendorCode(), ex.getParams());
null, ex.getCause(), ex.getSqlState(), ex.getVendorCode(), ex.getParams());
}
}

Expand Down
123 changes: 108 additions & 15 deletions src/test/java/net/snowflake/client/jdbc/StatementLatestIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import static net.snowflake.client.jdbc.ErrorCode.ROW_DOES_NOT_EXIST;
import static org.awaitility.Awaitility.await;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand All @@ -21,10 +22,16 @@
import java.sql.SQLException;
import java.sql.Statement;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import net.snowflake.client.TestUtil;
import net.snowflake.client.annotations.DontRunOnGithubActions;
import net.snowflake.client.category.TestTags;
Expand Down Expand Up @@ -335,25 +342,80 @@ public void testSetQueryTimeoutForAsyncQueryUsingConnectionProperty() throws SQL

/**
* Test for setting query timeout on regular queries with the IMPLICIT_SERVER_SIDE_QUERY_TIMEOUT
* property set to true. Applicable to versions after 3.21.0.
* property set to true should rely on server only. Applicable to versions after 3.21.0. In
* version above 3.22.0 the error should be handled only on the server side.
*
* @throws SQLException if there is an error when executing
*/
@Test
public void testSetQueryTimeoutWhenAsyncConnectionPropertySet() throws SQLException {
public void testSetQueryTimeoutOnStatementWhenImplicitQueryTimeoutIsSet()
throws SQLException, InterruptedException, ExecutionException {
int threads = 20;
ExecutorService executor = Executors.newFixedThreadPool(threads);
List<Future<?>> futures = new ArrayList<>();
Properties p = new Properties();
p.put("IMPLICIT_SERVER_SIDE_QUERY_TIMEOUT", true);
try (Connection con = getConnection(p);
Statement statement = con.createStatement()) {
statement.setQueryTimeout(3);
try (Connection con = getConnection(p)) {

String sql = "select seq4() from table(generator(rowcount => 1000000000))";

try {
statement.executeQuery(sql);
fail("This query should fail.");
} catch (SQLException e) {
assertEquals(SqlState.QUERY_CANCELED, e.getSQLState());
for (int i = 0; i < threads; ++i) {
futures.add(
executor.submit(
() -> {
try (Statement statement = con.createStatement()) {
statement.setQueryTimeout(3);
statement.executeQuery(sql);
fail("This query should fail.");
} catch (SQLException e) {
assertEquals(SqlState.QUERY_CANCELED, e.getSQLState());
}
}));
}
executor.shutdown();
assertTrue(executor.awaitTermination(60, TimeUnit.SECONDS));
for (Future<?> future : futures) {
assertNull(future.get());
}
}
}

/**
* Test for setting connection level query timeout on regular queries with the
* IMPLICIT_SERVER_SIDE_QUERY_TIMEOUT property set to true should rely on server only. Applicable
* to versions after 3.22.0.
*
* @throws SQLException if there is an error when executing
*/
@Test
public void testSetQueryTimeoutOnConnectionWhenImplicitQueryTimeoutIsSet()
throws SQLException, InterruptedException, ExecutionException {
int threads = 20;
ExecutorService executor = Executors.newFixedThreadPool(threads);
List<Future<?>> futures = new ArrayList<>();
Properties p = new Properties();
p.put("IMPLICIT_SERVER_SIDE_QUERY_TIMEOUT", true);
p.put("queryTimeout", 3);
try (Connection con = getConnection(p)) {

String sql = "select seq4() from table(generator(rowcount => 1000000000))";

for (int i = 0; i < threads; ++i) {
futures.add(
executor.submit(
() -> {
try (Statement statement = con.createStatement()) {
statement.executeQuery(sql);
fail("This query should fail.");
} catch (SQLException e) {
assertEquals(SqlState.QUERY_CANCELED, e.getSQLState());
}
}));
}
executor.shutdown();
assertTrue(executor.awaitTermination(60, TimeUnit.SECONDS));
for (Future<?> future : futures) {
assertNull(future.get());
}
}
}
Expand All @@ -378,11 +440,42 @@ public void testSetQueryTimeoutForAsyncQuery() throws SQLException {
.atMost(Duration.ofSeconds(10))
.until(() -> sfrs.getStatusV2().getStatus() == QueryStatus.FAILED_WITH_ERROR);

assertTrue(
sfrs.getStatusV2()
.getErrorMessage()
.contains(
"Statement reached its statement or warehouse timeout of 3 second(s) and was canceled"));
assertThat(
sfrs.getStatusV2().getErrorMessage(),
containsString(
"Statement reached its statement or warehouse timeout of 3 second(s) and was canceled"));
}
}
}

/**
* Applicable to versions after 3.22.0.
*
* @throws SQLException if there is an error when executing
*/
@Test
public void testSetAsyncQueryTimeoutOverridesConnectionQueryTimeoutForAsyncQuery()
throws SQLException {
Properties p = new Properties();
p.put("IMPLICIT_SERVER_SIDE_QUERY_TIMEOUT", true);
p.put("queryTimeout", 1);
try (Connection con = getConnection(p);
Statement statement = con.createStatement()) {
SnowflakeStatement sfStmt = statement.unwrap(SnowflakeStatement.class);
sfStmt.setAsyncQueryTimeout(3);

String sql = "select seq4() from table(generator(rowcount => 1000000000))";

try (ResultSet resultSet = sfStmt.executeAsyncQuery(sql)) {
SnowflakeResultSet sfrs = resultSet.unwrap(SnowflakeResultSet.class);
await()
.atMost(Duration.ofSeconds(10))
.until(() -> sfrs.getStatusV2().getStatus() == QueryStatus.FAILED_WITH_ERROR);

assertThat(
sfrs.getStatusV2().getErrorMessage(),
containsString(
"Statement reached its statement or warehouse timeout of 3 second(s) and was canceled"));
}
}
}
Expand Down

0 comments on commit ef8ddbd

Please sign in to comment.