Skip to content

Flink: Fix ResultSet resource leak in JdbcLockFactory.initializeLockTables().#13821

Merged
stevenzwu merged 4 commits into
apache:mainfrom
slfan1989:jdbc-lockfactory-resultset-leak
Aug 18, 2025
Merged

Flink: Fix ResultSet resource leak in JdbcLockFactory.initializeLockTables().#13821
stevenzwu merged 4 commits into
apache:mainfrom
slfan1989:jdbc-lockfactory-resultset-leak

Conversation

@slfan1989

Copy link
Copy Markdown
Contributor

Problem

In JdbcLockFactory#initializeLockTables(), the ResultSet returned by DatabaseMetaData#getTables(...) is not closed explicitly. This can lead to resource leaks (open cursors/handles) on certain JDBC drivers and databases.

Changes

  • Wrapped the ResultSet in a try-with-resources block to ensure it is always closed.
  • Minor code formatting adjustments for readability.

@github-actions github-actions Bot added the flink label Aug 15, 2025
@slfan1989

Copy link
Copy Markdown
Contributor Author

@pvary @Guosmilesmile @mxm This is a relatively minor change, but I believe it is still necessary, as the current implementation does not release resources after using the ResultSet object.

@Guosmilesmile Guosmilesmile left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

try (PreparedStatement ps = conn.prepareStatement(CREATE_LOCK_TABLE_SQL)) {
ps.execute();
}
return true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: newline

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you for helping review the code! I have already fixed this issue.

@pvary

pvary commented Aug 15, 2025

Copy link
Copy Markdown
Contributor

@slfan1989: good catch! Thanks for reporting and fixing

@stevenzwu stevenzwu added this to the Iceberg 1.10.0 milestone Aug 15, 2025
@slfan1989

Copy link
Copy Markdown
Contributor Author

@slfan1989: good catch! Thanks for reporting and fixing

@pvary @stevenzwu Thank you very much for reviewing this PR. I’ve re-triggered the build, and the results now look as expected. Could you please help review this PR again?

@stevenzwu stevenzwu merged commit a0ef463 into apache:main Aug 18, 2025
18 checks passed
@stevenzwu

Copy link
Copy Markdown
Contributor

thanks @slfan1989 for the fix and everyone for the review

@slfan1989

Copy link
Copy Markdown
Contributor Author

thanks @slfan1989 for the fix and everyone for the review

Thank you all for your reviews and feedback on this PR!

huaxingao pushed a commit that referenced this pull request Aug 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants