.Net: Change Sqlite connector to accept connection string instead of DbConnection#10550
.Net: Change Sqlite connector to accept connection string instead of DbConnection#10550roji wants to merge 1 commit intomicrosoft:feature-vector-data-preb1from
Conversation
| string? serviceId = default) | ||
| { | ||
| services.AddKeyedTransient<IVectorStore>( | ||
| => services.AddKeyedSingleton<IVectorStore>( |
There was a problem hiding this comment.
Note the change from transient to singleton lifecycle here - this aligns with other connectors.
adamsitnik
left a comment
There was a problem hiding this comment.
Please see my question, I would like to get a better understanding of the connection pooling first.
|
|
||
| private async ValueTask<SqliteConnection> GetConnectionAsync(CancellationToken cancellationToken = default) | ||
| { | ||
| var connection = new SqliteConnection(this._connectionString); |
There was a problem hiding this comment.
Does SqliteConnection type implements it's own private pool of connections? So we don't actually open a new connection every time this method is being called?
There was a problem hiding this comment.
Yep, that's exactly right - that's the pattern that all .NET database drivers follow in general.
Static/private/implicit pools are (IMHO) generally a bad idea, which is why we introduced the DbDataSource abstraction - that's basically meant to be a factory for connections, and can explicitly represent a connection pool; so it allows users to manage pools explicitly (plus other benefits). Unfortunately, for now only Npgsql properly implements this (and I think maybe mysqlconnector).
Data Source=:memory:cannot be used, since the databases only lives while the connection is open, and disappears immediately when closed.[Obsolete(error: true)], to have an informative error message for the user.Closes #10454
/cc @westey-m @dmytrostruk @adamsitnik