Connection pool #411

kdubb · 2016-04-16T08:18:08Z

Implement simultaneous multi reader, single write connection pool. Uses WAL and exclusive VFS mode to achieve concurrency.

Uses WAL mode to support multiple reads and a single writer.

Allows customizing new connections once created

ConnectionType is now Connection and Connection is now DBConnection

vfs seems to be a more reliable handling of exclusive mode.

stephencelis · 2016-04-17T19:20:45Z

SQLite/Core/Connection.swift

+
+
+/// Protocol to an SQLite connection
+public protocol Connection {


Per the Swift naming guidelines can we make this ConnectionProtocol and retain the default Connection class name?

Wouldn't ConnectionType be more Switfy? (thinking of SequenceType, etc)

I haven't kept up on the Swift-Evo list as of late. I know they are renaming basically everything.

Also, I kind of liked having DBConnection, regardless of the protocol name, simply because it has more capabilities than the protocol does and the name kind of explains why in that case. Although, with a renamed protocol, simply Connection works as well.

Yep, this is the proposal: https://github.com/apple/swift-evolution/blob/master/proposals/0006-apply-api-guidelines-to-the-standard-library.md

Strip Type suffix from protocol names. In a few special cases this means adding a Protocol suffix to get out of the way of type names that are primary (though most of these we expect to be obsoleted by Swift 3 language features).

I need to go back and update the other Type-suffixed protocols, as well.

Given this I'd love to keep it as is. Connection, DBConnection & BorrowedConnection (although you never use it directly) are more descriptive and seem to be more in line with the eventual Swift naming schemes; the alternative of ConnectionProtocol, Connection & BorrowedConnection seems less desirable to me even if it means a bit of renaming in projects.

In reality, projects shouldn't have to do much renaming at all since Connection just becomes a protocol. Only during initialization & when using special functions would you need to use DBConnection.

I wonder if we can find another, happier medium. The reaction I had to the current naming direction is that it's a little harder to derive specific meaning on its own. I'm left wondering why only one of the types is prefixed with the abbreviation DB and what the significance of DB has over the other types. I also wonder if decorators like DB are slightly redundant in a SQLite library.

I actually prefer ConnectionType over ConnectionProtocol, but was trying to move in the general direction of Swift norms. I wonder if we could come up with a different protocol name that we like even better. Or maybe DBConnection could change to something else, like ConcreteConnection, though that descriptor still seems slightly confusing to me.

Any other ideas?

I agree that 'DB' wasn't the best. Prior to that I had almost settled on DirectConnection. Which may be more to your liking?

Yup! Definitely prefer that! Has very clear meaning on its own.

stephencelis · 2016-04-17T19:27:46Z

This is great! Naming/delegate question aside, I'd love to get this merged!

kdubb · 2016-04-18T21:42:52Z

@stephencelis Updated to use setup closures very close to your suggestion. Like it much better. Also renamed to DirectConnection

Was using incorrect dispatch_once token.

# Conflicts: # SQLiteTests/ConnectionPoolTests.swift

stephencelis · 2016-04-20T14:08:21Z

Looking good! Going to check it out and put it through some tests before merging.

robertjpayne · 2016-04-20T16:09:25Z

SQLite/Core/ConnectionPool.swift

+
+        var borrowed : BorrowedConnection!
+
+        repeat {


Isn't this a busy loop? If all the readers are checked out it would appear this method will just keep repeating busily until one is available. Would be a lot better to use a dispatch_semaphore or NSCondition to let it sleep and get a signal when a connection is actually available.

To be honest I need to look at this again... This seems to be a holdover from an earlier version/feature. This code should find an available connection, wait for one or fail. None of that requires a loop. Good catch!

Right now because it will always create a new reader it'll never busy loop, but if it didn't it would create a busy loop just need a signal of some sort…

You also want to make sure that signal isn't does block the lockQueue otherwise a connection wont be able to be checked in.

I did this in 960309e and would appreciate any feedback.

nickmshelley · 2016-06-01T21:54:10Z

@kdubb: Is there something I can do to help get this through? I don't want to step on any toes but I'd love to see this in and am willing to finish up whatever's remaining if you're too busy at the moment.

nickmshelley · 2016-06-01T22:02:31Z

SQLite/Core/Connection.swift

@@ -67,10 +268,32 @@ public final class Connection {
    ///     Default: `false`.
    ///
    /// - Returns: A new database connection.
-    public init(_ location: Location = .InMemory, readonly: Bool = false) throws {
+    public convenience init(_ location: Location = .InMemory, readonly: Bool = false, vfsName: String? = nil) throws {


Looks like the function docs above need to be updated to include the vfsName parameter.

nickmshelley · 2016-06-16T21:53:16Z

I opened #451 because this doesn't seem to be going anywhere. Let me know if there's a better way to get this through than opening a new PR.

kdubb · 2016-07-19T20:15:38Z

@nickmshelley My apologies I hadn't seen the resurrection of this PR until I was just notified about it being merged. All worked out it seems!

nickmshelley · 2016-07-19T20:46:40Z

@kdubb I hope so. Sorry if I stole any thunder.

nickmshelley · 2016-07-19T20:47:14Z

Also thanks for doing the bulk of the work @kdubb.

stephencelis · 2016-07-19T20:49:53Z

Thanks for shepherding this, @nickmshelley!

kdubb added 6 commits April 15, 2016 22:23

Add connection pool for concurrent access

6881560

Uses WAL mode to support multiple reads and a single writer.

Add delegate to connection pool

42ebd18

Allows customizing new connections once created

Better names for protocols and classes

68f46d5

ConnectionType is now Connection and Connection is now DBConnection

Use vfs for exclusivity

66b82b0

vfs seems to be a more reliable handling of exclusive mode.

Added alternate concurrency test

ccc20e5

Fix imports in ConnectionPool.swift

18f612f

stephencelis reviewed Apr 17, 2016
View reviewed changes

kdubb added 2 commits April 17, 2016 15:55

Rename DBConnection to DirectConnection

2a5908b

Replace pool delegate with setup closures

9548d17

kdubb added 2 commits April 18, 2016 18:02

Fix writable connection initialization

ffb9274

Was using incorrect dispatch_once token.

Cleanup connection pool tests

b7a0eb1

# Conflicts: # SQLiteTests/ConnectionPoolTests.swift

stephencelis mentioned this pull request Apr 20, 2016

Allow concurrent reads and writes (connection pool) #400

Open

robertjpayne reviewed Apr 20, 2016
View reviewed changes

nickmshelley mentioned this pull request Jun 1, 2016

Error handling #413

Closed

nickmshelley reviewed Jun 1, 2016
View reviewed changes

nickmshelley mentioned this pull request Jun 16, 2016

Implement connection pool. #451

Merged

nickmshelley merged commit b7a0eb1 into stephencelis:master Jul 19, 2016

kdubb deleted the connection_pool branch July 19, 2016 20:15

jberkel mentioned this pull request Dec 7, 2016

multi threading support? #365

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Connection pool #411

Connection pool #411

kdubb commented Apr 16, 2016

stephencelis Apr 17, 2016

kdubb Apr 17, 2016

kdubb Apr 17, 2016

kdubb Apr 17, 2016

stephencelis Apr 17, 2016

kdubb Apr 17, 2016 •

edited

Loading

stephencelis Apr 17, 2016 •

edited

Loading

kdubb Apr 17, 2016

stephencelis Apr 17, 2016

stephencelis commented Apr 17, 2016

kdubb commented Apr 18, 2016

stephencelis commented Apr 20, 2016

robertjpayne Apr 20, 2016

kdubb Apr 20, 2016

robertjpayne Apr 20, 2016

nickmshelley Jun 16, 2016

nickmshelley commented Jun 1, 2016

nickmshelley Jun 1, 2016

nickmshelley commented Jun 16, 2016

kdubb commented Jul 19, 2016

nickmshelley commented Jul 19, 2016

nickmshelley commented Jul 19, 2016

stephencelis commented Jul 19, 2016



		/// Protocol to an SQLite connection
		public protocol Connection {

Connection pool #411

Connection pool #411

Conversation

kdubb commented Apr 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kdubb Apr 17, 2016 • edited Loading

Choose a reason for hiding this comment

stephencelis Apr 17, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephencelis commented Apr 17, 2016

kdubb commented Apr 18, 2016

stephencelis commented Apr 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickmshelley commented Jun 1, 2016

Choose a reason for hiding this comment

nickmshelley commented Jun 16, 2016

kdubb commented Jul 19, 2016

nickmshelley commented Jul 19, 2016

nickmshelley commented Jul 19, 2016

stephencelis commented Jul 19, 2016

kdubb Apr 17, 2016 •

edited

Loading

stephencelis Apr 17, 2016 •

edited

Loading