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.

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 SQL NPE after connection is closed #3829

Merged
merged 8 commits into from
Jan 25, 2023

Conversation

pdoerner
Copy link
Contributor

@pdoerner pdoerner commented Jan 23, 2023

Stopped setting factory.mainDBConn to null when the last connection is closed so that a NullPointerException is not generated if this connection is used again after close.

Added tests for MySQL and Postgresql to confirm that the underlying persistence layer library will return a useful error message.

Tests were not added for Cassandra because it uses a different factory that was not modified.
Tests were not added for SQLite because at least one connection is always open until the server is shut down.

Fixes #3708

Why?

How did you test it?

Potential risks

Is hotfix candidate?

@pdoerner pdoerner requested a review from a team as a code owner January 23, 2023 23:22
@CLAassistant
Copy link

CLAassistant commented Jan 23, 2023

CLA assistant check
All committers have signed the CLA.

@pdoerner pdoerner requested a review from yycptt January 23, 2023 23:23
Copy link
Member

@yycptt yycptt left a comment

Choose a reason for hiding this comment

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

LGTM

common/persistence/tests/persistence_connection_suite.go Outdated Show resolved Hide resolved
common/persistence/sql/factory.go Outdated Show resolved Hide resolved
@pdoerner pdoerner merged commit e6113da into temporalio:master Jan 25, 2023
@pdoerner pdoerner deleted the fix-sql-npe-after-close branch January 25, 2023 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix sql persistence NPE after closed
3 participants