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

Microsoft.Data.Sqlite: Pool Connections #13837

Closed
Tracked by #24110 ...
bricelam opened this issue May 15, 2018 · 33 comments · Fixed by #25018
Closed
Tracked by #24110 ...

Microsoft.Data.Sqlite: Pool Connections #13837

bricelam opened this issue May 15, 2018 · 33 comments · Fixed by #25018
Assignees
Labels
area-adonet-sqlite area-perf closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@bricelam
Copy link
Contributor

bricelam commented May 15, 2018

Pooling connections will improve performance--especially when using encryption which makes opening a connection more expensive. (Part of https://github.com/aspnet/Microsoft.Data.Sqlite/issues/412)

This would also allow us to make CreateFunction(), etc. calls persist between closing and re-opening a connection obviating the need for https://github.com/aspnet/Microsoft.Data.Sqlite/issues/354.

@divega
Copy link
Contributor

divega commented May 16, 2018

@bricelam would this also be an excuse to implement auto-prepared statements (assuming it isn’t implemented yet)?

@bricelam
Copy link
Contributor Author

We currently cache the compilation at the DbCommand level. We could lift it to an opened connection without this. With this we could do it across close and re-open.

@divega
Copy link
Contributor

divega commented May 16, 2018

Cool! Once we have that, I think implementing auto-prepared commands (like Npgsql) could give us another boost.

Cc @roji

@roji
Copy link
Member

roji commented May 16, 2018

Nice... If you work on prepared statements, consider also persisting them across pooled open/close, otherwise they're useless in short-lived connection scenarios.

@ErikEJ
Copy link
Contributor

ErikEJ commented May 16, 2018

@divega Is someone having a hard look a SqlClient for these performance improvements, would likely hit a wider audience?

@bricelam bricelam self-assigned this Jun 6, 2018
@ajcvickers ajcvickers transferred this issue from aspnet/Microsoft.Data.Sqlite Oct 31, 2018
@ajcvickers ajcvickers added this to the 3.0.0 milestone Oct 31, 2018
@ajcvickers ajcvickers changed the title Pool Connections Microsoft.Data.Sqlite: Pool Connections Oct 31, 2018
@bricelam
Copy link
Contributor Author

There are some PRAGMAs that change the functional behavior of the connection. If we add a pool, we'll need to consider how we handle these.

  • case_sensitive_like
  • foreign_keys
  • ignore_check_constraints
  • read_uncommitted
  • recursive_triggers
  • reverse_unordered_selects

There are other statements that alter the connection state, but they're primarily for performance tuning and don't affect functionality.

@bricelam
Copy link
Contributor Author

Also, extensions (e.g. SpatiaLite) are enabled per-connection. I don't think they can be disabled aside from closing and re-opening the connection.

@bricelam
Copy link
Contributor Author

I decided this probably isn't a breaking change since we can make the behavioral breaking changes as part of #13826, and add pooling later to improve performance.

@roji
Copy link
Member

roji commented Dec 7, 2018

@bricelam,

There are some PRAGMAs that change the functional behavior of the connection. If we add a pool, we'll need to consider how we handle these.

I have a similar situation with PostgreSQL, where various connection state can be changed with parameters. PostgreSQL does contain a RESET ALL mechanism mechanism which can be used to reset any changes, when returning it to the pool - I wonder if SQLite has anything similar.

(note that Npgsql has a No Reset On Close for extreme-perf-mode which does not do this)

@bricelam
Copy link
Contributor Author

bricelam commented Jan 23, 2020

SQLite 3.30.0 added support to unload extensions virtual table modules: sqlite3_drop_modules

bricelam added a commit to bricelam/efcore that referenced this issue Jun 18, 2021
bricelam added a commit to bricelam/efcore that referenced this issue Aug 9, 2021
bricelam added a commit to bricelam/efcore that referenced this issue Aug 11, 2021
@bricelam bricelam modified the milestones: 6.0.0, 6.0.0-rc1 Aug 12, 2021
bricelam added a commit to bricelam/efcore that referenced this issue Aug 16, 2021
Since merging, the CI fail rate increased significantly from ericsink/SQLitePCL.raw#430

Fixes dotnet#16202, unresolves dotnet#13837
@bricelam bricelam reopened this Aug 16, 2021
@bricelam bricelam removed the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 16, 2021
@bricelam bricelam removed this from the 6.0.0-rc1 milestone Aug 16, 2021
bricelam added a commit to bricelam/efcore that referenced this issue Aug 16, 2021
Since merging, the CI fail rate increased significantly from ericsink/SQLitePCL.raw#430

Fixes dotnet#16202, unresolves dotnet#13837
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 17, 2021
@ajcvickers ajcvickers added this to the 6.0.0-rc1 milestone Aug 17, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0-rc1, 6.0.0 Nov 8, 2021
@sjb-sjb
Copy link

sjb-sjb commented Mar 30, 2022

@ajcvickers: A simple question I'm sure but ... in the released version I am supposed to call SqliteConnection.ClearPool( SqliteConnection), where do I get the SqliteConnection from? DbContext.Database.GetDbConnection() returns a DbConnection instead.

Edit: the parameter type to InitializeDbConnection in SqliteRelationalConnection.cs would seem to suggest that the DbConnection can just be cast down to an SqliteConnection, is that correct? Would be nice to note this in the Breaking Changes document.

@ajcvickers
Copy link
Member

@sjb-sjb SqliteConnection inherits from DbConnetion. If you are using Sqlite, then your DbConnection is a SqliteConnection.

@ajcvickers
Copy link
Member

@sjb-sjb How is this a breaking change?

@sjb-sjb
Copy link

sjb-sjb commented Mar 30, 2022

The breaking change was that connection pooling results in the file being locked for longer than before. To unlock the file you need to call ClearPool. In my case when my UI program is in the background for a long time then I want to release the file so that OneDrive can copy it. The breaking change page talks about calling SqliteConnection.ClearPool but doesn't say how to get the SqliteConnection.

@ajcvickers
Copy link
Member

@sjb-sjb I'll bring it up with the team, but that seems like an extremely basic thing that anyone using C# should understand.

@sjb-sjb
Copy link

sjb-sjb commented Mar 30, 2022

Fair enough. I guess the reason I was confused was I assumed that SqliteConnection would be provided by the sqlite engine rather than by EF Core, so it wasn't a natural conclusion for me that it would be derived from an EF class.

@ajcvickers
Copy link
Member

ajcvickers commented Mar 30, 2022

DbConnection is not an EF class.

@bricelam
Copy link
Contributor Author

We did add a breaking change note about it here.

@sjb-sjb
Copy link

sjb-sjb commented Mar 30, 2022

I'm not trying to be hard-headed here. I know there is a breaking change note for EF, thanks, that's great. I'm just giving you the feedback that it would be nice if the breaking change note said "The SqliteConnection can be obtained by casting DbContext.Database.GetDbConnection() to SqliteConnection". It would save time poking around in the source code and bugging you nice folks in an attempt to figure it out. I know it's obvious to you but it wasn't very obvious to me for whatever reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-adonet-sqlite area-perf closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants