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

database/sql: close driver.Connector if it implements io.Closer #41710

Closed
wants to merge 4 commits into from

Conversation

tie
Copy link
Contributor

@tie tie commented Sep 30, 2020

This change allows driver implementations to manage resources in
driver.Connector, e.g. to share the same underlying database handle
between multiple connections. That is, it allows embedded databases
with in-memory backends like SQLite and Genji to safely release the
resources once the sql.DB is closed.

This makes it possible to address oddities with in-memory stores in
SQLite and Genji drivers without introducing too much complexity in
the driver implementations.

See also:

Fixes #41790

@googlebot googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Sep 30, 2020
@gopherbot
Copy link

This PR (HEAD: 04bfcb1) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/258360 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

This PR (HEAD: edd35d7) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/258360 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

Message from Emmanuel Odeke:

Patch Set 2: Run-TryBot+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gobot Gobot:

Patch Set 2:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=54ecb348


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gobot Gobot:

Patch Set 2: TryBot-Result+1

TryBots are happy.


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Ivan Trubach:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Go Bot:

Patch Set 2:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=54ecb348


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Go Bot:

Patch Set 2: TryBot-Result+1

TryBots are happy.


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Ivan Trubach:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Daniel Theophanes:

Patch Set 2: Code-Review+2


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Emmanuel Odeke:

Patch Set 3: Run-TryBot+1 Code-Review+2 Trust+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Go Bot:

Patch Set 3:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=61d06b53


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Go Bot:

Patch Set 3:

Build is still in progress...
This change failed on (x/tools) linux-amd64:
See https://storage.googleapis.com/go-build-log/61d06b53/linux-amd64_8f7dbd55.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Go Bot:

Patch Set 3: TryBot-Result-1

1 of 21 TryBots failed:
Failed on (x/tools) linux-amd64: https://storage.googleapis.com/go-build-log/61d06b53/linux-amd64_8f7dbd55.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

This change allows driver implementations to manage resources in
driver.Connector, e.g. to share the same underlying database handle
between multiple connections. That is, it allows embedded databases
with in-memory backends like SQLite and Genji to safely release the
resources once the sql.DB is closed.

This makes it possible to address oddities with in-memory stores in
SQLite and Genji drivers without introducing too much complexity in
the driver implementations.

See also:
- mattn/go-sqlite3#204
- mattn/go-sqlite3#511
- chaisql/chai#210
@gopherbot
Copy link

This PR (HEAD: 962c785) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/258360 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

Message from Ivan Trubach:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@odeke-em odeke-em changed the title database/sql: close driver.Connector if it implements an io.Closer database/sql: close driver.Connector if it implements io.Closer Feb 25, 2021
@gopherbot
Copy link

Message from Emmanuel Odeke:

Patch Set 4: Run-TryBot+1 Code-Review+2 Trust+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Go Bot:

Patch Set 4:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=162dfdf5


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Emmanuel Odeke:

Patch Set 5: Run-TryBot+1 Code-Review+2 Trust+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Go Bot:

Patch Set 5:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=99666900


Please don’t reply on this GitHub thread. Visit golang.org/cl/258360.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Feb 25, 2021
This change allows driver implementations to manage resources in
driver.Connector, e.g. to share the same underlying database handle
between multiple connections. That is, it allows embedded databases
with in-memory backends like SQLite and Genji to safely release the
resources once the sql.DB is closed.

This makes it possible to address oddities with in-memory stores in
SQLite and Genji drivers without introducing too much complexity in
the driver implementations.

See also:
- mattn/go-sqlite3#204
- mattn/go-sqlite3#511
- chaisql/chai#210

Fixes #41790

Change-Id: Idbd19763134438ed38288b9d44f16608e4e97fd7
GitHub-Last-Rev: 962c785
GitHub-Pull-Request: #41710
Reviewed-on: https://go-review.googlesource.com/c/go/+/258360
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
@gopherbot
Copy link

This PR is being closed because golang.org/cl/258360 has been merged.

@gopherbot gopherbot closed this Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

database/sql: close driver.Connector if it implements an io.Closer
3 participants