From bc893a6d7e8848d88f1c2511d52ac12620ca574a Mon Sep 17 00:00:00 2001 From: Dominik Przybysz <132913826+sfc-gh-dprzybysz@users.noreply.github.com> Date: Mon, 29 Jan 2024 07:18:09 +0100 Subject: [PATCH 1/8] SNOW-1020081: Do not run CodeCoverage jenkins jobs on Pull Request (#1618) --- Jenkinsfile | 1 - 1 file changed, 1 deletion(-) diff --git a/Jenkinsfile b/Jenkinsfile index 5c62291a8..5e62aab1b 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -76,7 +76,6 @@ timestamps { 'RT-LanguageJDBC2-PC' : "Test JDBC 2 - $jdk", 'RT-LanguageJDBC3-PC' : "Test JDBC 3 - $jdk", 'RT-LanguageJDBC4-PC' : "Test JDBC 4 - $jdk", - 'RT-LanguageJDBC-CodeCoverage-PC' : "CodeCoverage JDBC - $jdk" ].collect { jobToRun, runName -> return new JdbcJobDefinition( jdk: jdk, From 705fa4f0f4c445ac8d99d7b89de118156cea3acc Mon Sep 17 00:00:00 2001 From: Dominik Przybysz <132913826+sfc-gh-dprzybysz@users.noreply.github.com> Date: Mon, 29 Jan 2024 07:18:41 +0100 Subject: [PATCH 2/8] SNOW-1020059: Describe driver installation from source (#1617) --- README.rst | 59 ++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 15 deletions(-) diff --git a/README.rst b/README.rst index 790ddbaa0..ae38c2a67 100644 --- a/README.rst +++ b/README.rst @@ -25,7 +25,7 @@ Installation Maven ----- -Add following code block as a dependency +Add following dependency for fat-jar .. code-block:: xml @@ -35,6 +35,25 @@ Add following code block as a dependency {version} +or for FIPS compliant fat-jar + +.. code-block:: xml + + + net.snowflake + snowflake-jdbc-fips + {version} + + +or for experimental thin-jar + +.. code-block:: xml + + + net.snowflake + snowflake-jdbc-thin + {version} + Build from Source Code ---------------------- @@ -44,11 +63,33 @@ Build from Source Code git clone https://github.com/snowflakedb/snowflake-jdbc.git -2. Build the driver by running: +2. Build the fat-jar and install it in local maven repository by running: .. code-block:: bash - mvn install + ./mvnw clean verify + ./mvnw org.apache.maven.plugins:maven-install-plugin:3.1.1:install-file -Dfile=target/snowflake-jdbc.jar -DpomFile=./public_pom.xml + +3. Build the FIPS compliant fat-jar and install it in local maven repository by running: + +.. code-block:: bash + + cd FIPS + ../mvnw clean verify + ../mvnw org.apache.maven.plugins:maven-install-plugin:3.1.1:install-file -Dfile=target/snowflake-jdbc-fips.jar -DpomFile=./public_pom.xml + cd - + +4. Build the experimental thin-jar and install it in local maven repository by running: + +.. code-block:: bash + + ./mvnw clean verify -Dnot-self-contained-jar -Dthin-jar + ./mvnw org.apache.maven.plugins:maven-install-plugin:3.1.1:install-file -Dfile=target/snowflake-jdbc-thin.jar -DpomFile=./thin_public_pom.xml -Dnot-self-contained-jar -Dthin-jar + +- ``thin-jar`` enables thin jar profile +- ``not-self-contained-jar`` turns off fat jar profile (enabled by default) + +5. **Note that the built dependencies are installed with version 1.0-SNAPSHOT** Usage ===== @@ -111,18 +152,6 @@ You may import the coding style from IntelliJ so that the coding style can be ap - Enable `google-java-format` for the JDBC project. - In the source code window, select **Code** -> **Reformat** to apply the coding style. -Thin Jar -======== - -To build a thin jar run command: - -.. code-block:: bash - - mvn clean verify -Dthin-jar -Dnot-self-contained-jar - -- `thin-jar` enables thin jar profile -- `not-self-contained-jar` turns off fat jar profile (enabled by default) - Tests ===== From 15b3acaa115d99fbff306de77170e8cd3aeefd71 Mon Sep 17 00:00:00 2001 From: Dominik Przybysz <132913826+sfc-gh-dprzybysz@users.noreply.github.com> Date: Mon, 29 Jan 2024 13:24:35 +0100 Subject: [PATCH 3/8] SNOW-1021666: Remove plexus archiver from old driver tests (#1619) --- TestOnly/pom.xml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/TestOnly/pom.xml b/TestOnly/pom.xml index 0b194177b..46d45b1cf 100644 --- a/TestOnly/pom.xml +++ b/TestOnly/pom.xml @@ -120,10 +120,10 @@ test - org.codehaus.plexus - plexus-archiver - 4.9.0 - provided + org.apache.commons + commons-compress + 1.21 + test net.minidev From 65e40fb049baba5a7b763332c7b0f37567fb8916 Mon Sep 17 00:00:00 2001 From: Dominik Przybysz <132913826+sfc-gh-dprzybysz@users.noreply.github.com> Date: Mon, 29 Jan 2024 14:17:32 +0100 Subject: [PATCH 4/8] SNOW-1003125: Fix flaky test DatabaseMetaDataIT.testGetSchemas (#1614) --- .../java/net/snowflake/client/TestUtil.java | 17 +++++- .../client/jdbc/DatabaseMetaDataIT.java | 50 +++++++++------- .../client/jdbc/DatabaseMetaDataLatestIT.java | 58 +++++++++++++------ .../client/jdbc/StatementLatestIT.java | 4 +- 4 files changed, 89 insertions(+), 40 deletions(-) diff --git a/src/test/java/net/snowflake/client/TestUtil.java b/src/test/java/net/snowflake/client/TestUtil.java index da5271dc4..e33b3df3a 100644 --- a/src/test/java/net/snowflake/client/TestUtil.java +++ b/src/test/java/net/snowflake/client/TestUtil.java @@ -22,6 +22,20 @@ public class TestUtil { private static final Pattern QUERY_ID_REGEX = Pattern.compile("[a-z0-9]{8}-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{12}"); + public static final String GENERATED_SCHEMA_PREFIX = "GENERATED_"; + public static final String ESCAPED_GENERATED_SCHEMA_PREFIX = + GENERATED_SCHEMA_PREFIX.replaceAll("_", "\\\\_"); + private static final String GITHUB_SCHEMA_PREFIX = + "GITHUB_"; // created by JDBC CI jobs before tests + private static final String GITHUB_JOB_SCHEMA_PREFIX = + "GH_JOB_"; // created by other drivers e.g. Python driver + + public static boolean isSchemaGeneratedInTests(String schema) { + return schema.startsWith(TestUtil.GITHUB_SCHEMA_PREFIX) + || schema.startsWith(TestUtil.GITHUB_JOB_SCHEMA_PREFIX) + || schema.startsWith(TestUtil.GENERATED_SCHEMA_PREFIX); + } + /** * Util function to assert a piece will throw exception and assert on the error code * @@ -99,7 +113,8 @@ public static void withSchema(Statement statement, String schemaName, ThrowingRu */ public static void withRandomSchema(Statement statement, ThrowingConsumer action) throws Exception { - String customSchema = "TEST_SCHEMA_" + SnowflakeUtil.randomAlphaNumeric(5); + String customSchema = + GENERATED_SCHEMA_PREFIX + SnowflakeUtil.randomAlphaNumeric(5).toUpperCase(); try { statement.execute("CREATE OR REPLACE SCHEMA " + customSchema); action.call(customSchema); diff --git a/src/test/java/net/snowflake/client/jdbc/DatabaseMetaDataIT.java b/src/test/java/net/snowflake/client/jdbc/DatabaseMetaDataIT.java index f94272fee..81a63ba2d 100644 --- a/src/test/java/net/snowflake/client/jdbc/DatabaseMetaDataIT.java +++ b/src/test/java/net/snowflake/client/jdbc/DatabaseMetaDataIT.java @@ -7,6 +7,7 @@ import static java.sql.ResultSetMetaData.columnNullableUnknown; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.hasItem; import static org.junit.Assert.*; import com.google.common.base.Strings; @@ -139,41 +140,50 @@ public void testGetSchemas() throws Throwable { // CLIENT_METADATA_REQUEST_USE_CONNECTION_CTX = false try (Connection connection = getConnection()) { DatabaseMetaData metaData = connection.getMetaData(); + String currentSchema = connection.getSchema(); assertEquals("schema", metaData.getSchemaTerm()); - ResultSet resultSet = metaData.getSchemas(); - verifyResultSetMetaDataColumns(resultSet, DBMetadataResultSetMetadata.GET_SCHEMAS); - Set schemas = new HashSet<>(); - while (resultSet.next()) { - schemas.add(resultSet.getString(1)); + try (ResultSet resultSet = metaData.getSchemas()) { + verifyResultSetMetaDataColumns(resultSet, DBMetadataResultSetMetadata.GET_SCHEMAS); + while (resultSet.next()) { + String schema = resultSet.getString(1); + if (currentSchema.equals(schema) || !TestUtil.isSchemaGeneratedInTests(schema)) { + schemas.add(schema); + } + } } - resultSet.close(); - assertThat(schemas.size(), greaterThanOrEqualTo(1)); // one or more schemas + assertThat(schemas.size(), greaterThanOrEqualTo(1)); Set schemasInDb = new HashSet<>(); - resultSet = metaData.getSchemas(connection.getCatalog(), "%"); - while (resultSet.next()) { - schemasInDb.add(resultSet.getString(1)); + try (ResultSet resultSet = metaData.getSchemas(connection.getCatalog(), "%")) { + while (resultSet.next()) { + String schema = resultSet.getString(1); + if (currentSchema.equals(schema) || !TestUtil.isSchemaGeneratedInTests(schema)) { + schemasInDb.add(schema); + } + } } - assertThat(schemasInDb.size(), greaterThanOrEqualTo(1)); // one or more schemas + assertThat(schemasInDb.size(), greaterThanOrEqualTo(1)); assertThat(schemas.size(), greaterThanOrEqualTo(schemasInDb.size())); - assertTrue(schemas.containsAll(schemasInDb)); - assertTrue(schemas.contains(connection.getSchema())); + schemasInDb.forEach(schemaInDb -> assertThat(schemas, hasItem(schemaInDb))); + assertTrue(schemas.contains(currentSchema)); + assertTrue(schemasInDb.contains(currentSchema)); } // CLIENT_METADATA_REQUEST_USE_CONNECTION_CTX = true - try (Connection connection = getConnection()) { - Statement statement = connection.createStatement(); + try (Connection connection = getConnection(); + Statement statement = connection.createStatement()) { statement.execute("alter SESSION set CLIENT_METADATA_REQUEST_USE_CONNECTION_CTX=true"); DatabaseMetaData metaData = connection.getMetaData(); assertEquals("schema", metaData.getSchemaTerm()); - ResultSet resultSet = metaData.getSchemas(); - Set schemas = new HashSet<>(); - while (resultSet.next()) { - schemas.add(resultSet.getString(1)); + try (ResultSet resultSet = metaData.getSchemas()) { + Set schemas = new HashSet<>(); + while (resultSet.next()) { + schemas.add(resultSet.getString(1)); + } + assertThat(schemas.size(), equalTo(1)); } - assertThat(schemas.size(), equalTo(1)); // more than 1 schema } } diff --git a/src/test/java/net/snowflake/client/jdbc/DatabaseMetaDataLatestIT.java b/src/test/java/net/snowflake/client/jdbc/DatabaseMetaDataLatestIT.java index 6d22befed..9c5743c9d 100644 --- a/src/test/java/net/snowflake/client/jdbc/DatabaseMetaDataLatestIT.java +++ b/src/test/java/net/snowflake/client/jdbc/DatabaseMetaDataLatestIT.java @@ -171,11 +171,20 @@ public void testDoubleQuotedDatabaseAndSchema() throws Exception { // To query the schema and table, we can use a normal java escaped quote. Wildcards are also // escaped here String schemaRandomPart = SnowflakeUtil.randomAlphaNumeric(5); - String querySchema = "TEST\\_SCHEMA\\_\"WITH\\_QUOTES" + schemaRandomPart + "\""; + String querySchema = + TestUtil.ESCAPED_GENERATED_SCHEMA_PREFIX + + "TEST\\_SCHEMA\\_\"WITH\\_QUOTES" + + schemaRandomPart + + "\""; String queryTable = "TESTTABLE\\_\"WITH\\_QUOTES\""; // Create the schema and table. With SQL commands, double quotes must be escaped with another // quote - String schemaName = "\"TEST_SCHEMA_\"\"WITH_QUOTES" + schemaRandomPart + "\"\"\""; + String schemaName = + "\"" + + TestUtil.GENERATED_SCHEMA_PREFIX + + "TEST_SCHEMA_\"\"WITH_QUOTES" + + schemaRandomPart + + "\"\"\""; TestUtil.withSchema( statement, schemaName, @@ -367,24 +376,26 @@ public void testGetFunctionSqlInjectionProtection() throws Throwable { */ @Test public void testGetProcedureColumnsWildcards() throws SQLException { - try (Connection con = getConnection()) { - Statement statement = con.createStatement(); + try (Connection con = getConnection(); + Statement statement = con.createStatement()) { String database = con.getCatalog(); - String schema1 = "SCH1"; - String schema2 = "SCH2"; + String schemaPrefix = + TestUtil.GENERATED_SCHEMA_PREFIX + SnowflakeUtil.randomAlphaNumeric(5).toUpperCase(); + String schema1 = schemaPrefix + "SCH1"; + String schema2 = schemaPrefix + "SCH2"; // Create 2 schemas, each with the same stored procedure declared in them statement.execute("create or replace schema " + schema1); statement.execute(TEST_PROC); statement.execute("create or replace schema " + schema2); statement.execute(TEST_PROC); DatabaseMetaData metaData = con.getMetaData(); - ResultSet rs = metaData.getProcedureColumns(database, "SCH_", "TESTPROC", "PARAM1"); - // Assert 4 rows returned for the param PARAM1 that's present in each of the 2 identical - // stored procs in different schemas. A result row is returned for each procedure, making the - // total rowcount 4 - assertEquals(4, getSizeOfResultSet(rs)); - rs.close(); - statement.close(); + try (ResultSet rs = + metaData.getProcedureColumns(database, schemaPrefix + "SCH_", "TESTPROC", "PARAM1")) { + // Assert 4 rows returned for the param PARAM1 that's present in each of the 2 identical + // stored procs in different schemas. A result row is returned for each procedure, making + // the total rowcount 4 + assertEquals(4, getSizeOfResultSet(rs)); + } } } @@ -453,7 +464,7 @@ public void testGetStringValueFromColumnDef() throws SQLException { @Test public void testGetColumnsNullable() throws Throwable { - try (Connection connection = getConnection()) { + try (Connection connection = getConnection(); ) { String database = connection.getCatalog(); String schema = connection.getSchema(); final String targetTable = "T0"; @@ -484,8 +495,8 @@ public void testSessionDatabaseParameter() throws Throwable { String altdb = "ALTERNATEDB"; String altschema1 = "ALTERNATESCHEMA1"; String altschema2 = "ALTERNATESCHEMA2"; - try (Connection connection = getConnection()) { - Statement statement = connection.createStatement(); + try (Connection connection = getConnection(); + Statement statement = connection.createStatement()) { String catalog = connection.getCatalog(); String schema = connection.getSchema(); statement.execute("create or replace database " + altdb); @@ -905,7 +916,12 @@ public void testHandlingSpecialChars() throws Exception { statement.execute("INSERT INTO \"TEST\\1\\_1\" (\"C%1\",\"C\\1\\\\11\") VALUES (0,0)"); // test getColumns with escaped special characters in schema and table name String specialSchemaSuffix = SnowflakeUtil.randomAlphaNumeric(5); - String specialSchema = "\"SPECIAL%_\\SCHEMA" + specialSchemaSuffix + "\""; + String specialSchema = + "\"" + + TestUtil.GENERATED_SCHEMA_PREFIX + + "SPECIAL%_\\SCHEMA" + + specialSchemaSuffix + + "\""; TestUtil.withSchema( statement, specialSchema, @@ -939,7 +955,13 @@ public void testHandlingSpecialChars() throws Exception { String escapedTable2 = "TEST" + escapeChar + "_1" + escapeChar + "_1"; String escapedSchema = - "SPECIAL%" + escapeChar + "_" + escapeChar + "\\SCHEMA" + specialSchemaSuffix; + TestUtil.ESCAPED_GENERATED_SCHEMA_PREFIX + + "SPECIAL%" + + escapeChar + + "_" + + escapeChar + + "\\SCHEMA" + + specialSchemaSuffix; resultSet = metaData.getColumns(database, escapedSchema, escapedTable2, null); assertTrue(resultSet.next()); assertEquals("RNUM", resultSet.getString("COLUMN_NAME")); diff --git a/src/test/java/net/snowflake/client/jdbc/StatementLatestIT.java b/src/test/java/net/snowflake/client/jdbc/StatementLatestIT.java index 35d2a65d5..dfc585163 100644 --- a/src/test/java/net/snowflake/client/jdbc/StatementLatestIT.java +++ b/src/test/java/net/snowflake/client/jdbc/StatementLatestIT.java @@ -229,7 +229,9 @@ public void testPreparedStatementLogging() throws SQLException { @Test // SNOW-647217 public void testSchemaWith255CharactersDoesNotCauseException() throws SQLException { - String schemaName = "a" + SnowflakeUtil.randomAlphaNumeric(254); + String schemaName = + TestUtil.GENERATED_SCHEMA_PREFIX + + SnowflakeUtil.randomAlphaNumeric(255 - TestUtil.GENERATED_SCHEMA_PREFIX.length()); try (Connection con = getConnection()) { try (Statement stmt = con.createStatement()) { stmt.execute("create schema " + schemaName); From 455ba1753a5d456a263b066ead60b60b7f5fd5cd Mon Sep 17 00:00:00 2001 From: Harsh Chaturvedi Date: Tue, 30 Jan 2024 12:49:10 -0800 Subject: [PATCH 5/8] SNOW-1022736: Update TelemetryServiceIT.java (#1620) Disable the test that was enabled to prevent triggering the bug in log_analyzer --- .../snowflake/client/jdbc/telemetryOOB/TelemetryServiceIT.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/net/snowflake/client/jdbc/telemetryOOB/TelemetryServiceIT.java b/src/test/java/net/snowflake/client/jdbc/telemetryOOB/TelemetryServiceIT.java index 47e4e4dde..6aff9577f 100644 --- a/src/test/java/net/snowflake/client/jdbc/telemetryOOB/TelemetryServiceIT.java +++ b/src/test/java/net/snowflake/client/jdbc/telemetryOOB/TelemetryServiceIT.java @@ -56,6 +56,7 @@ public void tearDown() throws InterruptedException { } @SuppressWarnings("divzero") + @Ignore @Test public void testCreateException() { TelemetryService service = TelemetryService.getInstance(); From f922aa244983be16e33d33d8500caac5195288e0 Mon Sep 17 00:00:00 2001 From: Dominik Przybysz <132913826+sfc-gh-dprzybysz@users.noreply.github.com> Date: Thu, 1 Feb 2024 07:52:20 +0100 Subject: [PATCH 6/8] SNOW-1003783: Fix multi-release jar entries (#1621) --- FIPS/pom.xml | 36 ++++++++++++-- ci/scripts/check_content.sh | 5 ++ pom.xml | 97 ++++++++++++++++--------------------- 3 files changed, 78 insertions(+), 60 deletions(-) diff --git a/FIPS/pom.xml b/FIPS/pom.xml index 9071dd0bd..e7b0f6ccf 100644 --- a/FIPS/pom.xml +++ b/FIPS/pom.xml @@ -567,10 +567,7 @@ META-INF/DEPENDENCIES META-INF/maven/** META-INF/services/com.fasterxml.* - - - META-INF/versions/17/** - META-INF/versions/19/** + META-INF/versions/9/module-info.* META-INF/*.xml META-INF/*.SF META-INF/*.DSA @@ -618,6 +615,37 @@ + + + + org.apache.maven.plugins + maven-antrun-plugin + + + repack + + run + + package + + + + + + + + + + + + + + + + + + + diff --git a/ci/scripts/check_content.sh b/ci/scripts/check_content.sh index d2922fe7a..a9c0768b6 100755 --- a/ci/scripts/check_content.sh +++ b/ci/scripts/check_content.sh @@ -12,3 +12,8 @@ if jar tvf $DIR/../../target/snowflake-jdbc${package_modifier}.jar | awk '{prin echo "[ERROR] JDBC jar includes class not under the snowflake namespace" exit 1 fi + +if jar tvf $DIR/../../target/snowflake-jdbc${package_modifier}.jar | awk '{print $8}' | grep -E "^META-INF/versions/.*.class" | grep -v -E "^META-INF/versions/.*/(net|com)/snowflake"; then + echo "[ERROR] JDBC jar includes multi release classes not under the snowflake namespace" + exit 1 +fi diff --git a/pom.xml b/pom.xml index e28976451..146b1367f 100644 --- a/pom.xml +++ b/pom.xml @@ -167,7 +167,6 @@ groupId,artifactId true true - groupId,artifactId true stop strict @@ -609,33 +608,6 @@ - - - - org.apache.maven.plugins - maven-antrun-plugin - - - repack - - run - - package - - - - - - - - - - - - - - - org.apache.maven.plugins @@ -697,7 +669,6 @@ META-INF/NOTICE* META-INF/DEPENDENCIES META-INF/maven/** - META-INF/services/com.fasterxml.* META-INF/*.xml META-INF/*.SF META-INF/*.DSA @@ -781,33 +752,6 @@ - - - - org.apache.maven.plugins - maven-antrun-plugin - - - repack - - run - - package - - - - - - - - - - - - - - - org.apache.maven.plugins @@ -1010,6 +954,7 @@ META-INF/DEPENDENCIES META-INF/maven/** META-INF/services/com.fasterxml.* + META-INF/versions/9/module-info.* META-INF/*.xml META-INF/*.SF META-INF/*.DSA @@ -1059,6 +1004,46 @@ + + + + org.apache.maven.plugins + maven-antrun-plugin + + + repack + + run + + package + + + + + + + + + + + + + + + + + + + + + + + + + + + + org.codehaus.mojo buildnumber-maven-plugin From 35edeea1ca72fda7d848107998bf88d29a3f508b Mon Sep 17 00:00:00 2001 From: Dominik Przybysz <132913826+sfc-gh-dprzybysz@users.noreply.github.com> Date: Wed, 7 Feb 2024 07:02:39 +0100 Subject: [PATCH 7/8] SNOW-1044219: Add marker annotation for internal API (#1629) --- .../client/core/SnowflakeJdbcInternalApi.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 src/main/java/net/snowflake/client/core/SnowflakeJdbcInternalApi.java diff --git a/src/main/java/net/snowflake/client/core/SnowflakeJdbcInternalApi.java b/src/main/java/net/snowflake/client/core/SnowflakeJdbcInternalApi.java new file mode 100644 index 000000000..7d97df085 --- /dev/null +++ b/src/main/java/net/snowflake/client/core/SnowflakeJdbcInternalApi.java @@ -0,0 +1,22 @@ +/* + * Copyright (c) 2024 Snowflake Computing Inc. All right reserved. + */ +package net.snowflake.client.core; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Elements marked with this annotation should be considered as internal API even if they are public + */ +@Target({ + ElementType.PACKAGE, + ElementType.TYPE, + ElementType.FIELD, + ElementType.CONSTRUCTOR, + ElementType.METHOD +}) +@Retention(RetentionPolicy.RUNTIME) +public @interface SnowflakeJdbcInternalApi {} From 84b177708c8bc32a688bcece1ec293591d100bf4 Mon Sep 17 00:00:00 2001 From: Dominik Przybysz <132913826+sfc-gh-dprzybysz@users.noreply.github.com> Date: Wed, 7 Feb 2024 13:49:44 +0100 Subject: [PATCH 8/8] SNOW-1020043: Configure connection and socket timeout (#1628) --- .../net/snowflake/client/core/HttpUtil.java | 40 ++++++++--- .../snowflake/client/core/SFLoginInput.java | 18 +++-- .../snowflake/client/core/SFLoginOutput.java | 7 +- .../net/snowflake/client/core/SFSession.java | 22 ++++-- .../snowflake/client/core/SessionUtil.java | 22 +++--- .../core/SessionUtilExternalBrowser.java | 2 +- .../snowflake/client/jdbc/RestRequest.java | 3 +- .../cloud/storage/SnowflakeGCSClient.java | 2 +- .../jdbc/telemetry/TelemetryClient.java | 4 +- .../client/core/HttpUtilLatestIT.java | 67 +++++++++++++++++++ 10 files changed, 142 insertions(+), 45 deletions(-) create mode 100644 src/test/java/net/snowflake/client/core/HttpUtilLatestIT.java diff --git a/src/main/java/net/snowflake/client/core/HttpUtil.java b/src/main/java/net/snowflake/client/core/HttpUtil.java index 628d0e28d..eb2d3d6b6 100644 --- a/src/main/java/net/snowflake/client/core/HttpUtil.java +++ b/src/main/java/net/snowflake/client/core/HttpUtil.java @@ -20,6 +20,7 @@ import java.net.Socket; import java.security.KeyManagementException; import java.security.NoSuchAlgorithmException; +import java.time.Duration; import java.util.Map; import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; @@ -59,23 +60,27 @@ import org.apache.http.util.EntityUtils; public class HttpUtil { - static final SFLogger logger = SFLoggerFactory.getLogger(HttpUtil.class); + private static final SFLogger logger = SFLoggerFactory.getLogger(HttpUtil.class); static final int DEFAULT_MAX_CONNECTIONS = 300; static final int DEFAULT_MAX_CONNECTIONS_PER_ROUTE = 300; - static final int DEFAULT_CONNECTION_TIMEOUT = 60000; - static final int DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT = 300000; // ms + private static final int DEFAULT_HTTP_CLIENT_CONNECTION_TIMEOUT_IN_MS = 60000; + static final int DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT_IN_MS = 300000; // ms static final int DEFAULT_TTL = 60; // secs static final int DEFAULT_IDLE_CONNECTION_TIMEOUT = 5; // secs 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"; /** - * The unique httpClient shared by all connections. This will benefit long- lived clients. Key = + * The unique httpClient shared by all connections. This will benefit long-lived clients. Key = * proxy host + proxy port + nonProxyHosts, Value = Map of [OCSPMode, HttpClient] */ public static Map httpClient = @@ -101,6 +106,20 @@ public class HttpUtil { private static boolean socksProxyDisabled = false; + @SnowflakeJdbcInternalApi + public static Duration getConnectionTimeout() { + return Duration.ofMillis( + convertSystemPropertyToIntValue( + JDBC_CONNECTION_TIMEOUT_IN_MS_PROPERTY, DEFAULT_HTTP_CLIENT_CONNECTION_TIMEOUT_IN_MS)); + } + + @SnowflakeJdbcInternalApi + public static Duration getSocketTimeout() { + return Duration.ofMillis( + convertSystemPropertyToIntValue( + JDBC_SOCKET_TIMEOUT_IN_MS_PROPERTY, DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT_IN_MS)); + } + public static long getDownloadedConditionTimeoutInSeconds() { return DEFAULT_DOWNLOADED_CONDITION_TIMEOUT; } @@ -286,9 +305,14 @@ public static CloseableHttpClient buildHttpClient( @Nullable HttpClientSettingsKey key, File ocspCacheFile, boolean downloadUnCompressed) { // set timeout so that we don't wait forever. // Setup the default configuration for all requests on this client - int timeToLive = convertSystemPropertyToIntValue(JDBC_TTL, DEFAULT_TTL); logger.debug("time to live in connection pooling manager: {}", timeToLive); + long connectTimeout = getConnectionTimeout().toMillis(); + long socketTimeout = getSocketTimeout().toMillis(); + logger.debug( + "Connect timeout is {} ms and socket timeout is {} for connection pooling manager", + connectTimeout, + socketTimeout); // Set proxy settings for DefaultRequestConfig. If current proxy settings are the same as for // the last request, keep the current DefaultRequestConfig. If not, build a new @@ -306,9 +330,9 @@ public static CloseableHttpClient buildHttpClient( if (noDefaultRequestConfig || !DefaultRequestConfig.getProxy().equals(proxy)) { RequestConfig.Builder builder = RequestConfig.custom() - .setConnectTimeout(DEFAULT_CONNECTION_TIMEOUT) - .setConnectionRequestTimeout(DEFAULT_CONNECTION_TIMEOUT) - .setSocketTimeout(DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT); + .setConnectTimeout((int) connectTimeout) + .setConnectionRequestTimeout((int) connectTimeout) + .setSocketTimeout((int) socketTimeout); // only set the proxy settings if they are not null // but no value has been specified for nonProxyHosts // the route planner will determine whether to use a proxy based on nonProxyHosts value. diff --git a/src/main/java/net/snowflake/client/core/SFLoginInput.java b/src/main/java/net/snowflake/client/core/SFLoginInput.java index 927638f99..3d53bf104 100644 --- a/src/main/java/net/snowflake/client/core/SFLoginInput.java +++ b/src/main/java/net/snowflake/client/core/SFLoginInput.java @@ -7,14 +7,12 @@ import java.net.MalformedURLException; import java.net.URL; import java.security.PrivateKey; +import java.time.Duration; import java.util.Map; import net.snowflake.client.jdbc.ErrorCode; /** A class for holding all information required for login */ public class SFLoginInput { - private static int DEFAULT_HTTP_CLIENT_CONNECTION_TIMEOUT = 60000; // millisec - private static int DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT = 300000; // millisec - private String serverUrl; private String databaseName; private String schemaName; @@ -32,8 +30,8 @@ public class SFLoginInput { private boolean passcodeInPassword; private String passcode; private String token; - private int connectionTimeout = DEFAULT_HTTP_CLIENT_CONNECTION_TIMEOUT; - private int socketTimeout = DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT; + private Duration connectionTimeout = HttpUtil.getConnectionTimeout(); + private Duration socketTimeout = HttpUtil.getSocketTimeout(); private String appId; private String appVersion; private String sessionToken; @@ -216,20 +214,20 @@ public SFLoginInput setToken(String token) { return this; } - int getConnectionTimeout() { + Duration getConnectionTimeout() { return connectionTimeout; } - SFLoginInput setConnectionTimeout(int connectionTimeout) { + SFLoginInput setConnectionTimeout(Duration connectionTimeout) { this.connectionTimeout = connectionTimeout; return this; } - int getSocketTimeout() { - return socketTimeout; + int getSocketTimeoutInMillis() { + return (int) socketTimeout.toMillis(); } - SFLoginInput setSocketTimeout(int socketTimeout) { + SFLoginInput setSocketTimeout(Duration socketTimeout) { this.socketTimeout = socketTimeout; return this; } diff --git a/src/main/java/net/snowflake/client/core/SFLoginOutput.java b/src/main/java/net/snowflake/client/core/SFLoginOutput.java index da0e00a70..490184ec9 100644 --- a/src/main/java/net/snowflake/client/core/SFLoginOutput.java +++ b/src/main/java/net/snowflake/client/core/SFLoginOutput.java @@ -4,6 +4,7 @@ package net.snowflake.client.core; +import java.time.Duration; import java.util.Map; /** Login output information including session tokens, database versions */ @@ -16,7 +17,7 @@ public class SFLoginOutput { private String databaseVersion; private int databaseMajorVersion; private int databaseMinorVersion; - private int httpClientSocketTimeout; + private Duration httpClientSocketTimeout; private String sessionDatabase; private String sessionSchema; private String sessionRole; @@ -50,7 +51,7 @@ public class SFLoginOutput { this.databaseVersion = databaseVersion; this.databaseMajorVersion = databaseMajorVersion; this.databaseMinorVersion = databaseMinorVersion; - this.httpClientSocketTimeout = httpClientSocketTimeout; + this.httpClientSocketTimeout = Duration.ofMillis(httpClientSocketTimeout); this.sessionDatabase = sessionDatabase; this.sessionSchema = sessionSchema; this.sessionRole = sessionRole; @@ -106,7 +107,7 @@ int getDatabaseMinorVersion() { return databaseMinorVersion; } - int getHttpClientSocketTimeout() { + Duration getHttpClientSocketTimeout() { return httpClientSocketTimeout; } diff --git a/src/main/java/net/snowflake/client/core/SFSession.java b/src/main/java/net/snowflake/client/core/SFSession.java index fec471492..8ff722191 100644 --- a/src/main/java/net/snowflake/client/core/SFSession.java +++ b/src/main/java/net/snowflake/client/core/SFSession.java @@ -14,6 +14,7 @@ import java.security.PrivateKey; import java.sql.DriverPropertyInfo; import java.sql.SQLException; +import java.time.Duration; import java.util.*; import java.util.concurrent.*; import java.util.concurrent.atomic.AtomicInteger; @@ -48,7 +49,9 @@ public class SFSession extends SFBaseSession { "CLIENT_STORE_TEMPORARY_CREDENTIAL"; private static final ObjectMapper mapper = ObjectMapperFactory.getObjectMapper(); private static final int MAX_SESSION_PARAMETERS = 1000; - public static final int DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT = 300000; // millisec + // this constant was public - let's not change it + public static final int DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT = + HttpUtil.DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT_IN_MS; private final AtomicInteger sequenceId = new AtomicInteger(0); private final List missingProperties = new ArrayList<>(); // list of active asynchronous queries. Used to see if session should be closed when connection @@ -85,8 +88,8 @@ public class SFSession extends SFBaseSession { private int authTimeout = 0; private boolean enableCombineDescribe = false; - private int httpClientConnectionTimeout = 60000; // milliseconds - private int httpClientSocketTimeout = DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT; // milliseconds + private final 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 private int injectSocketTimeout = 0; @@ -188,7 +191,12 @@ private JsonNode getQueryMetadata(String queryID) throws SQLException { get.setHeader("Authorization", "Snowflake Token=\"" + this.sessionToken + "\""); response = HttpUtil.executeGeneralRequest( - get, loginTimeout, authTimeout, httpClientSocketTimeout, 0, getHttpClientKey()); + get, + loginTimeout, + authTimeout, + (int) httpClientSocketTimeout.toMillis(), + 0, + getHttpClientKey()); jsonNode = OBJECT_MAPPER.readTree(response); } catch (Exception e) { throw new SnowflakeSQLLoggedException( @@ -915,7 +923,7 @@ protected void heartbeat() throws SFException, SQLException { postRequest, SF_HEARTBEAT_TIMEOUT, authTimeout, - httpClientSocketTimeout, + (int) httpClientSocketTimeout.toMillis(), 0, getHttpClientKey()); @@ -985,11 +993,11 @@ public int getAuthTimeout() { } public int getHttpClientSocketTimeout() { - return httpClientSocketTimeout; + return (int) httpClientSocketTimeout.toMillis(); } public int getHttpClientConnectionTimeout() { - return httpClientConnectionTimeout; + return (int) httpClientConnectionTimeout.toMillis(); } public boolean isClosed() { diff --git a/src/main/java/net/snowflake/client/core/SessionUtil.java b/src/main/java/net/snowflake/client/core/SessionUtil.java index 3c416a8d4..e17459f7f 100644 --- a/src/main/java/net/snowflake/client/core/SessionUtil.java +++ b/src/main/java/net/snowflake/client/core/SessionUtil.java @@ -354,7 +354,7 @@ private static SFLoginOutput newSession( int databaseMinorVersion = 0; String newClientForUpgrade; int healthCheckInterval = DEFAULT_HEALTH_CHECK_INTERVAL; - int httpClientSocketTimeout = loginInput.getSocketTimeout(); + int httpClientSocketTimeout = loginInput.getSocketTimeoutInMillis(); final ClientAuthnDTO.AuthenticatorType authenticatorType = getAuthenticator(loginInput); Map commonParams; @@ -623,7 +623,7 @@ private static SFLoginOutput newSession( String theString = null; int leftRetryTimeout = loginInput.getLoginTimeout(); - int leftsocketTimeout = loginInput.getSocketTimeout(); + int leftsocketTimeout = loginInput.getSocketTimeoutInMillis(); int retryCount = 0; while (true) { @@ -664,7 +664,7 @@ private static SFLoginOutput newSession( // auth timeout within socket timeout is thrown without backoff, // and we need to update time remained in socket timeout here to control the // the actual socket timeout from customer setting. - if (loginInput.getSocketTimeout() > 0) { + if (loginInput.getSocketTimeoutInMillis() > 0) { if (ex.issocketTimeoutNoBackoff()) { if (leftsocketTimeout > elapsedSeconds) { leftsocketTimeout -= elapsedSeconds; @@ -673,7 +673,7 @@ private static SFLoginOutput newSession( } } else { // reset curl timeout for retry with backoff. - leftsocketTimeout = loginInput.getSocketTimeout(); + leftsocketTimeout = loginInput.getSocketTimeoutInMillis(); } } @@ -782,11 +782,11 @@ private static SFLoginOutput newSession( if (healthCheckIntervalFromGS > 0 && healthCheckIntervalFromGS != healthCheckInterval) { // add health check interval to socket timeout httpClientSocketTimeout = - loginInput.getSocketTimeout() + (healthCheckIntervalFromGS * 1000); + loginInput.getSocketTimeoutInMillis() + (healthCheckIntervalFromGS * 1000); final RequestConfig requestConfig = RequestConfig.copy(HttpUtil.getRequestConfigWithoutCookies()) - .setConnectTimeout(loginInput.getConnectionTimeout()) + .setConnectTimeout((int) loginInput.getConnectionTimeout().toMillis()) .setSocketTimeout(httpClientSocketTimeout) .build(); @@ -961,7 +961,7 @@ private static SFLoginOutput tokenRequest(SFLoginInput loginInput, TokenRequestT postRequest, loginInput.getLoginTimeout(), loginInput.getAuthTimeout(), - loginInput.getSocketTimeout(), + loginInput.getSocketTimeoutInMillis(), 0, loginInput.getHttpClientSettingsKey()); @@ -1054,7 +1054,7 @@ static void closeSession(SFLoginInput loginInput) throws SFException, SnowflakeS postRequest, loginInput.getLoginTimeout(), loginInput.getAuthTimeout(), - loginInput.getSocketTimeout(), + loginInput.getSocketTimeoutInMillis(), 0, loginInput.getHttpClientSettingsKey()); @@ -1120,7 +1120,7 @@ private static String federatedFlowStep4( httpGet, loginInput.getLoginTimeout(), loginInput.getAuthTimeout(), - loginInput.getSocketTimeout(), + loginInput.getSocketTimeoutInMillis(), 0, loginInput.getHttpClientSettingsKey()); @@ -1194,7 +1194,7 @@ private static String federatedFlowStep3(SFLoginInput loginInput, String tokenUr postRequest, loginInput.getLoginTimeout(), loginInput.getAuthTimeout(), - loginInput.getSocketTimeout(), + loginInput.getSocketTimeoutInMillis(), 0, 0, null, @@ -1285,7 +1285,7 @@ private static JsonNode federatedFlowStep1(SFLoginInput loginInput) throws Snowf postRequest, loginInput.getLoginTimeout(), loginInput.getAuthTimeout(), - loginInput.getSocketTimeout(), + loginInput.getSocketTimeoutInMillis(), 0, loginInput.getHttpClientSettingsKey()); logger.debug("authenticator-request response: {}", gsResponse); diff --git a/src/main/java/net/snowflake/client/core/SessionUtilExternalBrowser.java b/src/main/java/net/snowflake/client/core/SessionUtilExternalBrowser.java index f322e3b98..6e36f3adf 100644 --- a/src/main/java/net/snowflake/client/core/SessionUtilExternalBrowser.java +++ b/src/main/java/net/snowflake/client/core/SessionUtilExternalBrowser.java @@ -187,7 +187,7 @@ private String getSSOUrl(int port) throws SFException, SnowflakeSQLException { postRequest, loginInput.getLoginTimeout(), loginInput.getAuthTimeout(), - loginInput.getSocketTimeout(), + loginInput.getSocketTimeoutInMillis(), 0, loginInput.getHttpClientSettingsKey()); diff --git a/src/main/java/net/snowflake/client/jdbc/RestRequest.java b/src/main/java/net/snowflake/client/jdbc/RestRequest.java index b6f1f7780..3e57d2ff1 100644 --- a/src/main/java/net/snowflake/client/jdbc/RestRequest.java +++ b/src/main/java/net/snowflake/client/jdbc/RestRequest.java @@ -242,7 +242,8 @@ public static CloseableHttpResponse execute( savedEx = ex; // if the request took more than 5 min (socket timeout) log an error - if ((System.currentTimeMillis() - startTimePerRequest) > 300000) { + if ((System.currentTimeMillis() - startTimePerRequest) + > HttpUtil.getSocketTimeout().toMillis()) { logger.warn( "HTTP request took longer than 5 min: {} sec", (System.currentTimeMillis() - startTimePerRequest) / 1000); diff --git a/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java b/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java index ea629d7c0..b1acfd2bc 100644 --- a/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java +++ b/src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java @@ -584,7 +584,7 @@ public void uploadWithPresignedUrlWithoutConnection( uploadWithPresignedUrl( networkTimeoutInMilli, 0, // auth timeout - SFSession.DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT, // socket timeout + (int) HttpUtil.getSocketTimeout().toMillis(), meta.getContentEncoding(), meta.getUserMetadata(), uploadStreamInfo.left, diff --git a/src/main/java/net/snowflake/client/jdbc/telemetry/TelemetryClient.java b/src/main/java/net/snowflake/client/jdbc/telemetry/TelemetryClient.java index b302edb1c..e92a57524 100644 --- a/src/main/java/net/snowflake/client/jdbc/telemetry/TelemetryClient.java +++ b/src/main/java/net/snowflake/client/jdbc/telemetry/TelemetryClient.java @@ -3,8 +3,6 @@ */ package net.snowflake.client.jdbc.telemetry; -import static net.snowflake.client.core.SFSession.DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT; - import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; @@ -361,7 +359,7 @@ private boolean sendBatch() throws IOException { post, TELEMETRY_HTTP_RETRY_TIMEOUT_IN_SEC, 0, - DEFAULT_HTTP_CLIENT_SOCKET_TIMEOUT, + (int) HttpUtil.getSocketTimeout().toMillis(), 0, this.httpClient) : HttpUtil.executeGeneralRequest( diff --git a/src/test/java/net/snowflake/client/core/HttpUtilLatestIT.java b/src/test/java/net/snowflake/client/core/HttpUtilLatestIT.java new file mode 100644 index 000000000..a7a7aff87 --- /dev/null +++ b/src/test/java/net/snowflake/client/core/HttpUtilLatestIT.java @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2024 Snowflake Computing Inc. All right reserved. + */ +package net.snowflake.client.core; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import java.io.IOException; +import java.net.SocketTimeoutException; +import java.time.Duration; +import net.snowflake.client.category.TestCategoryCore; +import org.apache.http.client.methods.HttpGet; +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; + +@Category(TestCategoryCore.class) +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(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"); + + CloseableHttpClient httpClient = + HttpUtil.getHttpClient(new HttpClientSettingsKey(OCSPMode.INSECURE)); + try { + httpClient.execute(new HttpGet(HANG_WEBSERVER_ADDRESS)); + fail("Request should fail with exception"); + } catch (IOException e) { + MatcherAssert.assertThat(e, CoreMatchers.instanceOf(SocketTimeoutException.class)); + } + } + + /** 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()); + } +}