This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 9a72e5fe1b [#14694] fix(jdbc): prevent PostgreSQL timeout overflow
(#14695)
9a72e5fe1b is described below
commit 9a72e5fe1bfd57f81a299081e19c37f3e7561591
Author: Qi Yu <[email protected]>
AuthorDate: Fri Nov 6 15:57:01 2026 +0800
[#14694] fix(jdbc): prevent PostgreSQL timeout overflow (#14695)
### What changes were proposed in this pull request?
Cap derived PostgreSQL timeouts at 2147483 whole seconds so the JDBC
driver can convert them back to integer milliseconds safely. Add
boundary regressions for pooled connections and create-time probes, and
document the upper-bound rounding.
### Why are the changes needed?
The valid jdbc.pool.connect-timeout-ms=2147483647 setting rounds up to
2147484 seconds. PostgreSQL JDBC multiplies timeout seconds by 1000
using integer arithmetic, which overflows and can cause connection
verification to fail with "timeout can't be negative".
Fixes #14694
### Does this PR introduce _any_ user-facing change?
Values near the accepted millisecond upper bound now generate a safe
driver timeout. The cap shortens those values by at most 647
milliseconds. The accepted configuration range, normal rounding,
operator-supplied driver properties and other driver families are
unchanged.
### How was this patch tested?
- On current main, the new boundary regressions fail in three cases
before the fix. After the fix, 61 relevant tests across six JDBC common
classes pass with no failures, errors or skips.
- The prepared branch-1.3 backport applies without conflicts; all 57
relevant tests pass with no failures, errors or skips.
- A local TCP-listener probe using the actual translator and PostgreSQL
JDBC 42.7.11 reproduces the negative-timeout exception before the fix
and reaches protocol handling after the fix for 2147483001 and
2147483647 milliseconds. This is not a live-database or deployed REST
test.
- Scoped Spotless and git diff --check pass.
A branch-1.3 backport is prepared for submission after the main fix
merges.
---
.../gravitino/catalog/jdbc/config/JdbcConfig.java | 8 ++---
.../catalog/jdbc/utils/JdbcConnectionTimeouts.java | 12 ++++----
.../jdbc/utils/TestJdbcConnectionTimeouts.java | 35 +++++++++++++++-------
docs/jdbc-catalog-connection-validation.md | 4 ++-
4 files changed, 39 insertions(+), 20 deletions(-)
diff --git
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/config/JdbcConfig.java
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/config/JdbcConfig.java
index 5ef5348ab1..161152ec42 100644
---
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/config/JdbcConfig.java
+++
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/config/JdbcConfig.java
@@ -129,10 +129,10 @@ public class JdbcConfig extends Config {
* otherwise block the connect-only bound forever. An operator value set
directly on the
* connection or in the {@code jdbc-url} is never overridden.
*
- * <p>The value is bounded by {@link Integer#MAX_VALUE} milliseconds because
the MySQL and
- * PostgreSQL driver timeouts are {@code int}s (and the PostgreSQL
millisecond-to-seconds
- * conversion stays within {@code int} range); a larger value would be
accepted here and only fail
- * later when the driver parses it.
+ * <p>The value is bounded by {@link Integer#MAX_VALUE} milliseconds to fit
the drivers' integer
+ * millisecond socket timeouts. PostgreSQL's whole-second timeout is capped
before conversion back
+ * to milliseconds, shortening values near the upper bound by less than one
second to avoid
+ * overflow. Larger millisecond values are rejected here before reaching the
driver.
*/
public static final ConfigEntry<Long> CONNECT_TIMEOUT_MS =
new ConfigBuilder("jdbc.pool.connect-timeout-ms")
diff --git
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/JdbcConnectionTimeouts.java
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/JdbcConnectionTimeouts.java
index 88188551f0..a20d98164d 100644
---
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/JdbcConnectionTimeouts.java
+++
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/JdbcConnectionTimeouts.java
@@ -202,17 +202,19 @@ public final class JdbcConnectionTimeouts {
/**
* Converts milliseconds to seconds, rounded up with a minimum of 1 so a
sub-second bound never
- * collapses to 0 ("no timeout"). The arithmetic is overflow-safe for the
whole positive {@code
- * long} range (the input is validated positive and {@code <=
Integer.MAX_VALUE} by {@link
- * JdbcConfig}).
+ * collapses to 0 ("no timeout"). Cap the result at the largest whole-second
timeout that the
+ * PostgreSQL driver can multiply by 1000 without overflowing its integer
millisecond timeout. The
+ * input is validated positive and {@code <= Integer.MAX_VALUE} by {@link
JdbcConfig}.
*
* @param millis the timeout in milliseconds (positive)
- * @return the timeout in whole seconds, at least 1
+ * @return the timeout in whole seconds, from 1 through {@code
Integer.MAX_VALUE / 1000}
*/
private static long millisToSeconds(long millis) {
// (millis - 1) / 1000 + 1 is ceil(millis/1000) for positive millis
without the overflow that
// (millis + 999) would hit near Long.MAX_VALUE.
- return (millis - 1L) / 1000L + 1L;
+ // PostgreSQL converts seconds back to int milliseconds. At the upper
bound, rounding down
+ // by less than one second keeps the timeout positive instead of
overflowing or disabling it.
+ return Math.min((millis - 1L) / 1000L + 1L, Integer.MAX_VALUE / 1000L);
}
/**
diff --git
a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestJdbcConnectionTimeouts.java
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestJdbcConnectionTimeouts.java
index da3426ff26..ad80de7104 100644
---
a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestJdbcConnectionTimeouts.java
+++
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestJdbcConnectionTimeouts.java
@@ -24,6 +24,8 @@ import java.util.Map;
import org.apache.gravitino.catalog.jdbc.config.JdbcConfig;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
public class TestJdbcConnectionTimeouts {
@@ -128,16 +130,29 @@ public class TestJdbcConnectionTimeouts {
assertPostgresConnectSeconds("1999", "2");
}
- @Test
- void testPostgresSecondsConversionAtMaxAllowedValue() {
- // The config caps the value at Integer.MAX_VALUE ms; the ceiling
conversion stays correct and
- // within int range at that bound (and is overflow-safe for the whole
positive long range).
- long millis = Integer.MAX_VALUE;
- long expectedSeconds = (millis - 1L) / 1000L + 1L;
- assertPostgresConnectSeconds(Long.toString(millis),
Long.toString(expectedSeconds));
- Assertions.assertTrue(expectedSeconds > 1L, "conversion must not collapse
to 1 second");
- Assertions.assertTrue(
- expectedSeconds <= Integer.MAX_VALUE, "converted seconds must fit the
driver int range");
+ @ParameterizedTest
+ @ValueSource(longs = {2147482999L, 2147483000L, 2147483001L, 2147483646L,
2147483647L})
+ void testPostgresTimeoutsFitDriverMilliseconds(long millis) {
+ Map<String, String> props =
+ Map.of(
+ DRIVER_KEY, "org.postgresql.Driver",
+ URL_KEY, "jdbc:postgresql://host:5432/db",
+ CONNECT_TIMEOUT_MS_KEY, Long.toString(millis));
+ Map<String, String> pooled =
+ JdbcConnectionTimeouts.connectionProperties(config(props),
props.get(URL_KEY));
+ Map<String, String> probe =
+ JdbcConnectionTimeouts.probeConnectionProperties(config(props),
props.get(URL_KEY));
+
+ // PostgreSQL multiplies its integer timeout seconds by 1000 before
setting socket timeouts.
+ // A value that fits an int in seconds can still overflow in that
conversion.
+ for (String seconds : probe.values()) {
+ int driverMillis = Math.multiplyExact(Integer.parseInt(seconds), 1000);
+ Assertions.assertTrue(driverMillis > 0, "driver timeout must remain
positive");
+ }
+ Assertions.assertEquals(Map.of(CONNECT_TIMEOUT, "2147483"), pooled);
+ Assertions.assertEquals(
+ Map.of(CONNECT_TIMEOUT, "2147483", SOCKET_TIMEOUT, "2147483",
"loginTimeout", "2147483"),
+ probe);
}
private static void assertPostgresConnectSeconds(String millis, String
expectedSeconds) {
diff --git a/docs/jdbc-catalog-connection-validation.md
b/docs/jdbc-catalog-connection-validation.md
index f4aa15c3f3..e8e7a7d9d1 100644
--- a/docs/jdbc-catalog-connection-validation.md
+++ b/docs/jdbc-catalog-connection-validation.md
@@ -92,7 +92,9 @@ unit:
- MySQL Connector/J — also used by the Doris, StarRocks, and OceanBase
catalogs: `connectTimeout`
and `socketTimeout` in milliseconds.
- PostgreSQL — also used by the Hologres catalog: `connectTimeout`,
`socketTimeout` and
- `loginTimeout` in seconds (rounded up, minimum 1).
+ `loginTimeout` in seconds (rounded up, minimum 1, maximum `2147483`). The
upper cap avoids
+ overflow when the driver converts seconds back to integer milliseconds; near
the millisecond
+ upper bound, it shortens the configured timeout by less than one second.
- ClickHouse: `connect_timeout` and `socket_timeout` in milliseconds (note the
underscore names).
It is applied differently to the two kinds of connection:
