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.

Sign up for GitHub

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

fix(jdbc-sink): set network timeout #17244

Merged
merged 3 commits into from
Jun 13, 2024
Merged

fix(jdbc-sink): set network timeout #17244

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1fb723f
set network timeout
StrikeW Jun 13, 2024
55809f2
minor
StrikeW Jun 13, 2024
4af7455
fix format
StrikeW Jun 13, 2024
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
13 changes: 13 additions & 0 deletions ...connector-node/risingwave-sink-jdbc/src/main/java/com/risingwave/connector/JdbcUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@

public abstract class JdbcUtils {

static final int CONNECTION_TIMEOUT = 30;
static final int SOCKET_TIMEOUT = 300;

public static Optional<JdbcDialectFactory> getDialectFactory(String jdbcUrl) {
if (jdbcUrl.startsWith("jdbc:mysql")) {
return Optional.of(new MySqlDialectFactory());
Expand All @@ -43,6 +46,16 @@ public static Connection getConnection(String jdbcUrl) throws SQLException {
// https://jdbc.postgresql.org/documentation/use/
// https://dev.mysql.com/doc/connectors/en/connector-j-connp-props-networking.html#cj-conn-prop_tcpKeepAlive
props.setProperty("tcpKeepAlive", "true");

// default timeout in seconds
boolean isPg = jdbcUrl.startsWith("jdbc:postgresql");

// postgres use seconds and mysql use milliseconds
int connectTimeout = isPg ? CONNECTION_TIMEOUT : CONNECTION_TIMEOUT * 1000;
int socketTimeout = isPg ? SOCKET_TIMEOUT : SOCKET_TIMEOUT * 1000;
props.setProperty("connectTimeout", String.valueOf(connectTimeout));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing the value via props, I think we can create the connection with only jdbcUrl anyway and then get and reset the timeout by the methods provided in the Connection

https://docs.oracle.com/javase%2F8%2Fdocs%2Fapi%2F%2F/java/sql/Connection.html

It provides get/setNetworkTimeout method in the interface.

Copy link
Contributor Author

@StrikeW StrikeW Jun 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setNetworkTimeout(Executor executor, int milliseconds)

I inspected the interface. But the first arg is confusing. For PG we can pass null for the executor, but for MySQL it will throw an exception if first arg is null.
These properties are well documented in the driver's doc, so this way should be reliable to set the timeout.

props.setProperty("socketTimeout", String.valueOf(socketTimeout));

var conn = DriverManager.getConnection(jdbcUrl, props);
// disable auto commit can improve performance
conn.setAutoCommit(false);
Expand Down
Loading