Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SNOW-1168512: continuing SNOW-1020043: expose timeouts (DEFAULT_CONNECTION_TIMEOUT and DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT) in JDBC as Connection string and/or datasource property #1865

Merged
merged 16 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions src/main/java/net/snowflake/client/core/HttpUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,13 @@ public class HttpUtil {
static final int DEFAULT_DOWNLOADED_CONDITION_TIMEOUT = 3600; // secs

public static final String JDBC_TTL = "net.snowflake.jdbc.ttl";
static final String JDBC_CONNECTION_TIMEOUT_IN_MS_PROPERTY =
"net.snowflake.jdbc.http_client_connection_timeout_in_ms";
static final String JDBC_SOCKET_TIMEOUT_IN_MS_PROPERTY =
"net.snowflake.jdbc.http_client_socket_timeout_in_ms";
public static final String JDBC_MAX_CONNECTIONS_PROPERTY = "net.snowflake.jdbc.max_connections";
public static final String JDBC_MAX_CONNECTIONS_PER_ROUTE_PROPERTY =
"net.snowflake.jdbc.max_connections_per_route";

private static Duration connectionTimeout;
private static Duration socketTimeout;

/**
* The unique httpClient shared by all connections. This will benefit long-lived clients. Key =
* proxy host + proxy port + nonProxyHosts, Value = Map of [OCSPMode, HttpClient]
Expand Down Expand Up @@ -114,16 +113,26 @@ public class HttpUtil {

@SnowflakeJdbcInternalApi
public static Duration getConnectionTimeout() {
return Duration.ofMillis(
SystemUtil.convertSystemPropertyToIntValue(
JDBC_CONNECTION_TIMEOUT_IN_MS_PROPERTY, DEFAULT_HTTP_CLIENT_CONNECTION_TIMEOUT_IN_MS));
sfc-gh-ext-simba-jy marked this conversation as resolved.
Show resolved Hide resolved
return connectionTimeout != null
? connectionTimeout
: Duration.ofMillis(DEFAULT_HTTP_CLIENT_CONNECTION_TIMEOUT_IN_MS);
}

@SnowflakeJdbcInternalApi
public static Duration getSocketTimeout() {
return Duration.ofMillis(
SystemUtil.convertSystemPropertyToIntValue(
JDBC_SOCKET_TIMEOUT_IN_MS_PROPERTY, DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT_IN_MS));
return socketTimeout != null
? socketTimeout
: Duration.ofMillis(DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT_IN_MS);
}

@SnowflakeJdbcInternalApi
public static void setConnectionTimeout(int timeout) {
connectionTimeout = Duration.ofMillis(timeout);
}

@SnowflakeJdbcInternalApi
public static void setSocketTimeout(int timeout) {
socketTimeout = Duration.ofMillis(timeout);
}

public static long getDownloadedConditionTimeoutInSeconds() {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/net/snowflake/client/core/SFLoginInput.java
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,8 @@ public SFLoginInput setToken(String token) {
return this;
}

Duration getConnectionTimeout() {
return connectionTimeout;
int getConnectionTimeoutInMillis() {
return (int) connectionTimeout.toMillis();
}

SFLoginInput setConnectionTimeout(Duration connectionTimeout) {
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/net/snowflake/client/core/SFLoginOutput.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public class SFLoginOutput {
int databaseMajorVersion,
int databaseMinorVersion,
int httpClientSocketTimeout,
int httpClientConnectionTimeout,
String sessionDatabase,
String sessionSchema,
String sessionRole,
Expand Down Expand Up @@ -111,6 +112,10 @@ Duration getHttpClientSocketTimeout() {
return httpClientSocketTimeout;
}

Duration getHttpClientConnectionTimeout() {
return httpClientSocketTimeout;
}

Map<String, Object> getCommonParams() {
return commonParams;
}
Expand Down
23 changes: 19 additions & 4 deletions src/main/java/net/snowflake/client/core/SFSession.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public class SFSession extends SFBaseSession {

private int authTimeout = 0;
private boolean enableCombineDescribe = false;
private final Duration httpClientConnectionTimeout = HttpUtil.getConnectionTimeout();
private Duration httpClientConnectionTimeout = HttpUtil.getConnectionTimeout();
private Duration httpClientSocketTimeout = HttpUtil.getSocketTimeout();
// whether we try to simulate a socket timeout (a default value of 0 means
// no simulation). The value is in milliseconds
Expand Down Expand Up @@ -553,7 +553,7 @@ public synchronized void open() throws SFException, SnowflakeSQLException {
+ " warehouse: {}, validate default parameters: {}, authenticator: {}, ocsp mode: {},"
+ " passcode in password: {}, passcode is {}, private key is {}, disable socks proxy: {},"
+ " application: {}, app id: {}, app version: {}, login timeout: {}, retry timeout: {}, network timeout: {},"
+ " query timeout: {}, tracing: {}, private key file: {}, private key file pwd is {},"
+ " query timeout: {}, connection timeout: {}, socket timeout: {}, tracing: {}, private key file: {}, private key file pwd is {},"
+ " enable_diagnostics: {}, diagnostics_allowlist_path: {},"
+ " session parameters: client store temporary credential: {}, gzip disabled: {}, browser response timeout: {}",
connectionPropertiesMap.get(SFSessionProperty.SERVER_URL),
Expand All @@ -580,6 +580,8 @@ public synchronized void open() throws SFException, SnowflakeSQLException {
connectionPropertiesMap.get(SFSessionProperty.RETRY_TIMEOUT),
connectionPropertiesMap.get(SFSessionProperty.NETWORK_TIMEOUT),
connectionPropertiesMap.get(SFSessionProperty.QUERY_TIMEOUT),
connectionPropertiesMap.get(SFSessionProperty.HTTP_CLIENT_CONNECTION_TIMEOUT),
connectionPropertiesMap.get(SFSessionProperty.HTTP_CLIENT_SOCKET_TIMEOUT),
connectionPropertiesMap.get(SFSessionProperty.TRACING),
connectionPropertiesMap.get(SFSessionProperty.PRIVATE_KEY_FILE),
SFLoggerUtil.isVariableProvided(
Expand Down Expand Up @@ -624,8 +626,18 @@ public synchronized void open() throws SFException, SnowflakeSQLException {
.setToken((String) connectionPropertiesMap.get(SFSessionProperty.TOKEN))
.setPasscodeInPassword(passcodeInPassword)
.setPasscode((String) connectionPropertiesMap.get(SFSessionProperty.PASSCODE))
.setConnectionTimeout(httpClientConnectionTimeout)
.setSocketTimeout(httpClientSocketTimeout)
.setConnectionTimeout(
connectionPropertiesMap.get(SFSessionProperty.HTTP_CLIENT_CONNECTION_TIMEOUT) != null
? Duration.ofMillis(
(int)
connectionPropertiesMap.get(
SFSessionProperty.HTTP_CLIENT_CONNECTION_TIMEOUT))
: httpClientConnectionTimeout)
.setSocketTimeout(
connectionPropertiesMap.get(SFSessionProperty.HTTP_CLIENT_SOCKET_TIMEOUT) != null
? Duration.ofMillis(
(int) connectionPropertiesMap.get(SFSessionProperty.HTTP_CLIENT_SOCKET_TIMEOUT))
: httpClientSocketTimeout)
.setAppId((String) connectionPropertiesMap.get(SFSessionProperty.APP_ID))
.setAppVersion((String) connectionPropertiesMap.get(SFSessionProperty.APP_VERSION))
.setSessionParameters(sessionParametersMap)
Expand Down Expand Up @@ -659,6 +671,8 @@ public synchronized void open() throws SFException, SnowflakeSQLException {

// propagate OCSP mode to SFTrustManager. Note OCSP setting is global on JVM.
HttpUtil.initHttpClient(httpClientSettingsKey, null);
HttpUtil.setConnectionTimeout(loginInput.getConnectionTimeoutInMillis());
HttpUtil.setSocketTimeout(loginInput.getSocketTimeoutInMillis());

runDiagnosticsIfEnabled();

Expand All @@ -675,6 +689,7 @@ public synchronized void open() throws SFException, SnowflakeSQLException {
setDatabaseMajorVersion(loginOutput.getDatabaseMajorVersion());
setDatabaseMinorVersion(loginOutput.getDatabaseMinorVersion());
httpClientSocketTimeout = loginOutput.getHttpClientSocketTimeout();
httpClientConnectionTimeout = loginOutput.getHttpClientConnectionTimeout();
masterTokenValidityInSeconds = loginOutput.getMasterTokenValidityInSeconds();
setDatabase(loginOutput.getSessionDatabase());
setSchema(loginOutput.getSessionSchema());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,11 @@ public enum SFSessionProperty {
// Used as a fix for issue SNOW-354859. Remove with snowflake-jdbc version 4.x with BCR changes.
JDBC_GET_DATE_USE_NULL_TIMEZONE("JDBC_GET_DATE_USE_NULL_TIMEZONE", false, Boolean.class),

BROWSER_RESPONSE_TIMEOUT("BROWSER_RESPONSE_TIMEOUT", false, Integer.class);
BROWSER_RESPONSE_TIMEOUT("BROWSER_RESPONSE_TIMEOUT", false, Integer.class),

HTTP_CLIENT_CONNECTION_TIMEOUT("HTTP_CLIENT_CONNECTION_TIMEOUT", false, Integer.class),

HTTP_CLIENT_SOCKET_TIMEOUT("HTTP_CLIENT_SOCKET_TIMEOUT", false, Integer.class);

// property key in string
private String propertyKey;
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/net/snowflake/client/core/SessionUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ private static SFLoginOutput newSession(
String newClientForUpgrade;
int healthCheckInterval = DEFAULT_HEALTH_CHECK_INTERVAL;
int httpClientSocketTimeout = loginInput.getSocketTimeoutInMillis();
int httpClientConnectionTimeout = loginInput.getConnectionTimeoutInMillis();
final ClientAuthnDTO.AuthenticatorType authenticatorType = getAuthenticator(loginInput);
Map<String, Object> commonParams;

Expand Down Expand Up @@ -865,13 +866,13 @@ private static SFLoginOutput newSession(

final RequestConfig requestConfig =
RequestConfig.copy(HttpUtil.getRequestConfigWithoutCookies())
.setConnectTimeout((int) loginInput.getConnectionTimeout().toMillis())
.setConnectTimeout(httpClientConnectionTimeout)
.setSocketTimeout(httpClientSocketTimeout)
.build();

HttpUtil.setRequestConfig(requestConfig);

logger.debug("Adjusted connection timeout to: {}", loginInput.getConnectionTimeout());
logger.debug("Adjusted connection timeout to: {}", httpClientConnectionTimeout);

logger.debug("Adjusted socket timeout to: {}", httpClientSocketTimeout);
}
Expand Down Expand Up @@ -907,6 +908,7 @@ private static SFLoginOutput newSession(
databaseMajorVersion,
databaseMinorVersion,
httpClientSocketTimeout,
httpClientConnectionTimeout,
sessionDatabase,
sessionSchema,
sessionRole,
Expand Down
36 changes: 10 additions & 26 deletions src/test/java/net/snowflake/client/core/HttpUtilLatestIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
import org.apache.http.impl.client.CloseableHttpClient;
import org.hamcrest.CoreMatchers;
import org.hamcrest.MatcherAssert;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.categories.Category;

Expand All @@ -24,20 +22,19 @@ public class HttpUtilLatestIT {

private static final String HANG_WEBSERVER_ADDRESS = "http://localhost:12345/hang";

@Before
@After
public void cleanup() {
HttpUtil.httpClient.clear();
System.clearProperty(HttpUtil.JDBC_CONNECTION_TIMEOUT_IN_MS_PROPERTY);
System.clearProperty(HttpUtil.JDBC_SOCKET_TIMEOUT_IN_MS_PROPERTY);
/** Added in > 3.14.5 */
@Test
public void shouldGetDefaultConnectionAndSocketTimeouts() {
assertEquals(Duration.ofMillis(60_000), HttpUtil.getConnectionTimeout());
assertEquals(Duration.ofMillis(300_000), HttpUtil.getSocketTimeout());
}

/** Added in > 3.14.5 */
@Test(timeout = 1000L)
public void shouldOverrideConnectionAndSocketTimeouts() {
// it's hard to test connection timeout so there is only a test for socket timeout
System.setProperty(HttpUtil.JDBC_CONNECTION_TIMEOUT_IN_MS_PROPERTY, "100");
System.setProperty(HttpUtil.JDBC_SOCKET_TIMEOUT_IN_MS_PROPERTY, "200");
HttpUtil.setConnectionTimeout(100);
HttpUtil.setSocketTimeout(200);

CloseableHttpClient httpClient =
HttpUtil.getHttpClient(new HttpClientSettingsKey(OCSPMode.INSECURE));
Expand All @@ -46,22 +43,9 @@ public void shouldOverrideConnectionAndSocketTimeouts() {
fail("Request should fail with exception");
} catch (IOException e) {
MatcherAssert.assertThat(e, CoreMatchers.instanceOf(SocketTimeoutException.class));
} finally {
HttpUtil.setConnectionTimeout(60000);
HttpUtil.setSocketTimeout(300000);
}
}

/** Added in > 3.14.5 */
@Test
public void shouldGetDefaultConnectionAndSocketTimeouts() {
assertEquals(Duration.ofMillis(60_000), HttpUtil.getConnectionTimeout());
assertEquals(Duration.ofMillis(300_000), HttpUtil.getSocketTimeout());
}

/** Added in > 3.14.5 */
@Test
public void shouldGetOverridenConnectionAndSocketTimeouts() {
System.setProperty(HttpUtil.JDBC_CONNECTION_TIMEOUT_IN_MS_PROPERTY, "100");
System.setProperty(HttpUtil.JDBC_SOCKET_TIMEOUT_IN_MS_PROPERTY, "200");
assertEquals(Duration.ofMillis(100), HttpUtil.getConnectionTimeout());
assertEquals(Duration.ofMillis(200), HttpUtil.getSocketTimeout());
}
}
29 changes: 14 additions & 15 deletions src/test/java/net/snowflake/client/jdbc/ConnectionIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeTrue;

import java.io.File;
import java.io.FileInputStream;
Expand Down Expand Up @@ -1022,20 +1021,20 @@ private Properties setCommonConnectionParameters(boolean validateDefaultParamete
return kvMap2Properties(params, validateDefaultParameters);
}

@Test
public void testFailOverOrgAccount() throws SQLException {
// only when set_git_info.sh picks up a SOURCE_PARAMETER_FILE
assumeTrue(RunningOnGithubAction.isRunningOnGithubAction());

Map<String, String> kvParams = getConnectionParameters(null, "ORG");
Properties connProps = kvMap2Properties(kvParams, false);
String uri = kvParams.get("uri");

try (Connection con = DriverManager.getConnection(uri, connProps);
Statement statement = con.createStatement()) {
statement.execute("select 1");
}
}
// @Test
sfc-gh-ext-simba-jy marked this conversation as resolved.
Show resolved Hide resolved
// public void testFailOverOrgAccount() throws SQLException {
// // only when set_git_info.sh picks up a SOURCE_PARAMETER_FILE
// assumeTrue(RunningOnGithubAction.isRunningOnGithubAction());
//
// Map<String, String> kvParams = getConnectionParameters(null, "ORG");
// Properties connProps = kvMap2Properties(kvParams, false);
// String uri = kvParams.get("uri");
//
// try (Connection con = DriverManager.getConnection(uri, connProps);
// Statement statement = con.createStatement()) {
// statement.execute("select 1");
// }
// }

private class ConcurrentConnections implements Runnable {

Expand Down
13 changes: 13 additions & 0 deletions src/test/java/net/snowflake/client/jdbc/ConnectionLatestIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -1559,4 +1559,17 @@ public void shouldGetDifferentTimestampLtzConsistentBetweenFormats() throws Exce
}
}
}

/** Added in > 3.14.5 and modified in > 3.18.0 */
@Test
public void shouldGetOverridenConnectionAndSocketTimeouts() throws Exception {
Properties paramProperties = new Properties();
paramProperties.put("HTTP_CLIENT_CONNECTION_TIMEOUT", 100);
paramProperties.put("HTTP_CLIENT_SOCKET_TIMEOUT", 200);

try (Connection connection = getConnection(paramProperties)) {
assertEquals(Duration.ofMillis(100), HttpUtil.getConnectionTimeout());
assertEquals(Duration.ofMillis(200), HttpUtil.getSocketTimeout());
}
}
}
Loading