Skip to content

Explicitly close DB connections when discarding SQLAlchemy's Engine#6303

Merged
sawa3030 merged 1 commit intooptuna:masterfrom
not522:dispose-connection
Oct 17, 2025
Merged

Explicitly close DB connections when discarding SQLAlchemy's Engine#6303
sawa3030 merged 1 commit intooptuna:masterfrom
not522:dispose-connection

Conversation

@not522
Copy link
Copy Markdown
Member

@not522 not522 commented Oct 16, 2025

Motivation

Recently, random failures in CI have been occurring frequently (e.g., #5797 (comment)). This may be caused by connection objects created by SQLAlchemy not being properly released (see https://docs.sqlalchemy.org/en/20/core/connections.html#engine-disposal).
To call Engine.dispose() after each unit test will resolve this issue. However, modifying all relevant tests would be extensive work, so this PR limits its scope to modifications to the StorageSupplier. This should result in most tests being fixed, thereby reducing the likelihood of CI failures.

From my understanding, this issue only occurs when creating Engine objects frequently—like in CI scenarios. It doesn't happen in normal user usage.

The master branch ran the CI 10 times, with one failure (attempt 7 at https://github.com/not522/optuna/actions/runs/18522577119). Meanwhile, this PR never failed for this error (https://github.com/not522/optuna/actions/runs/18548728266; attempt 5 failed for a different reason).

Description of the changes

  • Explicitly close DB connections in StorageSupplier

@not522 not522 added the CI Continuous integration. label Oct 16, 2025
@y0z
Copy link
Copy Markdown
Member

y0z commented Oct 16, 2025

@c-bata @sawa3030 Could you review this PR?

@y0z y0z added this to the v4.6.0 milestone Oct 16, 2025
Copy link
Copy Markdown
Member

@c-bata c-bata left a comment

Choose a reason for hiding this comment

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

Changes themselves look good to me.

According to the Python 3.13 release note, the sqlite3 module started emitting ResourceWarning from version 3.13. Some failed tests such as this workflow run and this one are using Python 3.13.

A ResourceWarning is now emitted if a Connection object is not closed explicitly. (Contributed by Erlend E. Aasland in gh-105539.)
https://docs.python.org/3/whatsnew/3.13.html#sqlite3

However, following recent workflow runs uses Python 3.8 or 3.12, so I would say we still need to investigate those errors.

Anyway, thank you for your pull request. LGTM.

@c-bata c-bata removed their assignment Oct 17, 2025
Copy link
Copy Markdown
Collaborator

@sawa3030 sawa3030 left a comment

Choose a reason for hiding this comment

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

LGTM

@sawa3030 sawa3030 merged commit 4b8add0 into optuna:master Oct 17, 2025
13 checks passed
@not522 not522 deleted the dispose-connection branch October 17, 2025 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous integration.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix a fragile unit test, test_check_distribution_suggest_loguniform[sqlite3]

4 participants